| menn0 | davecheney: see cmd/jujud/agent_test.go:347 | 00:00 |
|---|---|---|
| davecheney | why is it writing out a file | 00:01 |
| davecheney | just to start a mock api server ? | 00:01 |
| menn0 | the jujud tests often fire up real machines | 00:01 |
| * davecheney puts head in hands | 00:01 | |
| * davecheney starts to sob | 00:02 | |
| menn0 | I'm just the messenger man :) | 00:02 |
| menn0 | davecheney: what we should probably do is use a port of 0 in the config, let the API server get a port itself and then find out what it was and use that for client connections | 00:03 |
| menn0 | (as long as we have tests that fire up real API servers) | 00:04 |
| menn0 | no idea how hard that will actually be in practice | 00:04 |
| menn0 | davecheney: getting back to your earlier point about two tests getting the same port... what about any other processes on the host? we're not just competing for ports against our own tests here. | 00:07 |
| menn0 | davecheney: also, you may be right about Close() leaving the port unavailable for a short time afterwards. I think I've seen that before even with SO_REUSEADDR being used (which it is). | 00:08 |
| davecheney | it's just a fragile pattenr | 00:08 |
| menn0 | davecheney: but... given that the apiserver retries the listen I don't think that's what's happening here | 00:09 |
| menn0 | davecheney: agreed. the pattern sucks. | 00:09 |
| menn0 | davecheney: we either need to expect and handle "socket already in use" or use port 0 when setting up the actual server (where possible) | 00:10 |
| menn0 | davecheney: shall I update the ticket with some of what we've discussed? | 00:13 |
| davecheney | sure | 00:14 |
| hatch | hey all - none of the mysql charms (precise/trusty) will deploy 1.20.6-trusty-amd64 | 00:26 |
| hatch | where should I be filing this bug? | 00:26 |
| sinzui | wallyworld__, thumper cross your eyes and toes and hope for a Heisenbug I am retesting dimitern's commit to add better logging. CI has 2 test to complete to pass master tip. Dare I say that the commit to add logging decreased the chances of the bug re-occuring | 00:27 |
| wallyworld__ | oh dear | 00:28 |
| hatch | lazyPower: marcoceppi you guys look like the last people to modify the mysql charms? | 00:28 |
| sinzui | hatch, did you try hp and aws? hp required more mem because the charm does bogus steps are setup | 00:28 |
| hatch | I'm on aws now on a 1.7GB ram | 00:29 |
| sinzui | yeah, that is too low | 00:29 |
| sinzui | hatch, 2G seems to be what the charm needs. QA learned that last year with juju 1.14 | 00:30 |
| sinzui | hatch, on hp the charm needs 4G | 00:30 |
| hatch | sinzui: that doesn't seem like the real issue https://gist.github.com/hatched/cff0bc4929b3b3201bd9 | 00:30 |
| sinzui | hatch, I am just repeating my experience with that charm in clouds with juju 1.14..1.18. It just doesn't run in under 2G anyware reliably | 00:31 |
| hatch | ok I can try to spin up a bigger one....I've never done that in the GUI...hmm | 00:31 |
| hatch | sinzui: so is it just the charm that requires so much? Or mysql? | 00:33 |
| sinzui | wallyworld__, 1 test to go... | 00:33 |
| wallyworld__ | sinzui: maybe i forgot - what's the agreed method to tells devs that ci is blocked? the #juju-dev topic? | 00:34 |
| sinzui | hatch, mysql can be configure to be greedy and fast, or modest and slow...the charm tuned mysql to need to much at the start | 00:34 |
| sinzui | wallyworld__, I have been using the topic | 00:34 |
| wallyworld__ | sinzui: ok, could we make the text then reflect that these are the critical bugs blocking ci? since not all criticals do | 00:35 |
| sinzui | wallyworld__, and I haven't updated the topic since this morning. | 00:35 |
| wallyworld__ | s/Open critical bugs/Critical bugs blocking CI | 00:35 |
| sinzui | wallyworld__, lets be honest, critical means do this now. | 00:36 |
| sinzui | wallyworld__, now we get to talk about what to do now | 00:36 |
| wallyworld__ | yes, but not all devs need to care abut all criticals, but they do need to know if CI is blocked | 00:36 |
| sinzui | wallyworld__, in a few minutes http://reports.vapour.ws/releases is going to list build 1785 of master 5ecf58fb as blessed. The only change is the api name changes and logging | 00:38 |
| hatch | sinzui: even using a massive instance i get the same error, looks like the mysql charms are just busted on ec2 and lxc | 00:39 |
| wallyworld__ | sinzui: that sucks in a way, shows just how fragile our tests are :-( | 00:39 |
| sinzui | wallyworld__, We can demote the remaining bugs to high, opening master to mass landings | 00:39 |
| wallyworld__ | that will make thumper happy | 00:40 |
| hatch | now where do I file bugs....man this must be super frustrating for new users, finding where the real source is, then trying to find somewhere to file bugs | 00:40 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Blocking bugs: None | ||
| sinzui | wallyworld__, http://reports.vapour.ws/releases/1785 | 00:43 |
| wallyworld__ | o/ | 00:44 |
| wallyworld__ | \o/ | 00:44 |
| * wallyworld__ merges katco's harvesting mode branch | 00:45 | |
| sinzui | wallyworld__, I think abentley can release 1.20.7 tomorrow while I prep for 1.21-alpha1 for the day after | 00:46 |
| wallyworld__ | sounds good | 00:46 |
| wallyworld__ | and 1.20.8 next week sometime | 00:46 |
| hatch | sinzui: so....do you have any idea where I'm supposed to file bugs? Best I can come up with is https://bugs.launchpad.net/charms/trusty/+source/mysql but I'm pretty sure that's not the real repository | 00:47 |
| wallyworld__ | axw: you online? | 00:48 |
| axw | wallyworld__: I am | 00:49 |
| axw | good morning | 00:49 |
| wallyworld__ | morning :-) | 00:49 |
| axw | hooray, CI is happy again | 00:50 |
| axw | hum | 00:50 |
| wallyworld__ | axw: with the branch to mock out the provisioner api call - there's code to do that for uniter and usermanager facades (but duplicated). can we look to set up some common, shared code to do this? | 00:50 |
| axw | wallyworld__: not sure it's worth it, it still requires in-package code because of the need to access private field | 00:51 |
| axw | s | 00:51 |
| wallyworld__ | fair enough. perhaps then we should make sure all the implementatons are the same | 00:52 |
| axw | I looked at the usermanager one and wasn't keen on it, I'll take a look at the uniter one | 00:53 |
| wallyworld__ | it's pretty much the same | 00:53 |
| wallyworld__ | whatever we decide, i think consistency is best | 00:54 |
| axw | spose so. I'll take another shot | 00:55 |
| menn0 | sinzui, wallyworld__ : I've updated bug 1364410 with some more details based on a discussion davecheney and I had earlier. We don't think it's PPC related. | 00:59 |
| mup | Bug #1364410: Timeout TestManageEnviron MachineSuite in ppc64el <ci> <intermittent-failure> <regression> <juju-core:Triaged> <https://launchpad.net/bugs/1364410> | 00:59 |
| wallyworld__ | menn0: i would be surprised if it were | 00:59 |
| wallyworld__ | menn0: i like option 1 as well. i wonder why the fark we didn't implement it that way to start with :-( | 01:01 |
| menn0 | wallyworld__, sinzui: tags and title updated | 01:01 |
| menn0 | wallyworld__: possibly because it's hard to pull the port out of the depths of the API server | 01:01 |
| wallyworld__ | menn0: well, that's something that should have been considered in the design | 01:02 |
| menn0 | wallyworld__: and possibly because it makes managing the API server configuration more difficult | 01:02 |
| wallyworld__ | IMO | 01:02 |
| menn0 | wallyworld__: agreed | 01:02 |
| sinzui | hatch, That is the real location of the bug tracker, and this shows the recommended and user branches that were converted to charms https://code.launchpad.net/charms/trusty/+source/mysql | 01:04 |
| wallyworld__ | menn0: sometimes a just despair of how stuff has been written without concurrency in mind. i mean, the port issue should have been clearly raising alarm bells when the code was developed | 01:05 |
| wallyworld__ | s/a/I | 01:05 |
| hatch | sinzui: yeah found it I filed a bug https://bugs.launchpad.net/charms/+source/mysql/+bug/1365205 | 01:06 |
| mup | Bug #1365205: Charm cannot be deployed, fails on install hook with gpg error <mysql (Juju Charms Collection):New> <https://launchpad.net/bugs/1365205> | 01:06 |
| menn0 | wallyworld__: the docstring for FindTCPPort even mentions that there is a race but that the probility of an actual problem should "hopefully" be small | 01:08 |
| menn0 | wallyworld__: that didn't work out :) | 01:08 |
| * wallyworld__ cries on the inside | 01:08 | |
| wallyworld__ | ffs | 01:08 |
| axw | menn0 wallyworld__: not sure if it's helpful or not, but I changed apiserver not too long ago to take a net.Listener as an arg, for the purpose of getting its port | 01:14 |
| axw | i.e. listen on port 0, get the port, then pass the listener to api server | 01:14 |
| wallyworld__ | hmmmm, that may be useful indeed | 01:14 |
| menn0 | axw: that sounds pretty helpful | 01:15 |
| menn0 | axw: the problem is the config is generated and then the server is started later... and I think that config is also used for establishing client connections | 01:16 |
| menn0 | so there will be a bit of faff to get the port actually used by the API server to where it's needed to allow clients to connect | 01:18 |
| axw | menn0: so I see. yuck | 01:18 |
| axw | menn0: one option is to store a net.Listener there (in primeStateAgent), then override net.Listen so that it returns that listener | 01:20 |
| axw | a bit horrible maybe | 01:20 |
| menn0 | axw: yeah a bit... but at least it stops the tests breaking every now again | 01:22 |
| thumper | wallyworld__, sinzui: there is a race condition in our tests... | 01:30 |
| thumper | no surprise there | 01:30 |
| thumper | but the location is interesting | 01:30 |
| wallyworld__ | a?? | 01:30 |
| wallyworld__ | several | 01:30 |
| wallyworld__ | many | 01:30 |
| wallyworld__ | lots | 01:30 |
| thumper | we open a port (:0) to figure out which port to use for mongo | 01:30 |
| thumper | we then close it | 01:30 |
| thumper | and pass it to mongo | 01:30 |
| thumper | and mongo uses it | 01:30 |
| thumper | sometimes it isn't fully closed before mongo tries | 01:31 |
| thumper | we could add a retry thingy | 01:31 |
| thumper | around mongo | 01:31 |
| thumper | checking for "port in use" | 01:31 |
| thumper | just a few times | 01:31 |
| thumper | should reduce the chance of it happening | 01:31 |
| wallyworld__ | we could | 01:32 |
| thumper | funnily enough, we can't test that the port is closed properly without opening it | 01:32 |
| thumper | can we? | 01:32 |
| wallyworld__ | or wait for the port to close before passing to mongo | 01:33 |
| menn0 | thumper: are you referring to bug 1364410 or something else? | 01:33 |
| mup | Bug #1364410: API server fails to start with "address already in use" in MachineSuite tests <ci> <intermittent-failure> <regression> <juju-core:Triaged> <https://launchpad.net/bugs/1364410> | 01:33 |
| wallyworld__ | not sure | 01:33 |
| thumper | menn0: yes, that one | 01:33 |
| menn0 | thumper: because that bug is about the API server, not mongo | 01:33 |
| thumper | same problem | 01:33 |
| menn0 | yes | 01:33 |
| thumper | we open a random port and pass it on | 01:33 |
| menn0 | yes | 01:33 |
| wallyworld__ | doesn't mongo support passing in 0 for the port? | 01:33 |
| menn0 | we've already been talking about it quite a bit here | 01:33 |
| thumper | menn0: oh, ok, I wasn't watching... | 01:34 |
| wallyworld__ | it chooses one and reports what it is listening on | 01:34 |
| thumper | I'll leave it to someone else :) | 01:34 |
| wallyworld__ | maybe it doesn't support that | 01:34 |
| menn0 | thumper: read scrollback and check the updates to the ticket :) | 01:34 |
| menn0 | for mongo we do actually retry a bunch of times as you suggest | 01:34 |
| menn0 | see juju/testing/mgo.go:167 | 01:35 |
| menn0 | it's a bit harder for the API server | 01:35 |
| menn0 | bbs | 01:35 |
| axw | wallyworld__: you can't use port 0 in mongo | 01:35 |
| wallyworld__ | i was wishing more than anything | 01:36 |
| axw | wallyworld__: we could maybe do something horrible like get it to use a unix socket and redirect a socket to that | 01:36 |
| axw | but it would be better just to not use mongo in the unit tests :) | 01:36 |
| wallyworld__ | you think? :-) | 01:37 |
| axw | wallyworld__: I updated the api test mocking PR | 01:37 |
| wallyworld__ | you mean "unit" tests right? | 01:37 |
| wallyworld__ | ok | 01:37 |
| axw | unified them around a common patching thingy | 01:37 |
| wallyworld__ | \o/ | 01:37 |
| wallyworld__ | i'm still looking at 663 | 01:37 |
| axw | okey dokey | 01:37 |
| axw | I'll go back to fixing the tools URL stuff I emailed about | 01:38 |
| wallyworld__ | ok | 01:38 |
| perrito666 | uh am I late for bashing our tests for using mongo? | 01:38 |
| perrito666 | ahh I am | 01:38 |
| davecheney | has anyone tried the new side by side diffing on github yet ? | 01:41 |
| axw | what, this is news | 01:41 |
| perrito666 | oh I just did | 01:41 |
| axw | yay | 01:41 |
| perrito666 | its glorious | 01:41 |
| perrito666 | and it remembers my choice | 01:42 |
| perrito666 | sweeeeet | 01:42 |
| * axw removes octosplit | 01:42 | |
| perrito666 | and it highlights the diffed parts | 01:42 |
| * perrito666 cries | 01:42 | |
| perrito666 | our whining has been heard | 01:42 |
| davecheney | thumper: menn0 https://github.com/juju/juju/pull/670 | 01:52 |
| davecheney | a slightly contraversial one | 01:52 |
| axw | wallyworld__: that comment about failing is wrong, it just overwrites now | 01:53 |
| axw | sorry | 01:53 |
| axw | I had originally intended it to, but changed my mind | 01:54 |
| thumper | davecheney: is it possible to just change it in state, but leave it with the api? | 01:54 |
| axw | this is why the hash is encoded in the path, so if the put to the blobstore OR to mongo fails, we don't end up with inconsistent blob/metadata | 01:55 |
| davecheney | thumper: nope | 01:55 |
| davecheney | thumper: do you want a long discussion or a short discussion about why this type doesnt' add anything | 01:55 |
| thumper | short | 01:56 |
| thumper | the only benefit I see is that it is shorter to type | 01:56 |
| davecheney | thumper: http://play.golang.org/p/4KQ6WrhFGf | 01:57 |
| davecheney | there are no methods on StatusData | 01:57 |
| davecheney | all it does is obscure what is actually being stored in the StatusData field | 01:57 |
| * thumper ndos | 01:57 | |
| davecheney | most of the code churn is in the tests | 01:58 |
| davecheney | as usual | 01:58 |
| davecheney | the actual api code, apart from the method sigs | 01:58 |
| davecheney | is unaware of the change | 01:58 |
| davecheney | thumper: thanks | 01:59 |
| davecheney | this will probably be the most contraversial change | 01:59 |
| davecheney | the rest are more mechainical | 01:59 |
| menn0 | davecheney, thumper: FWIW, I'm -0 on this. I prefer the type aliases sometimes. They provide documentation and in this case, repeatedly spelling out of an otherwise awkward type. | 02:02 |
| menn0 | map[string]interface{} is ugly to look at | 02:03 |
| menn0 | especially when it's used a lot | 02:03 |
| davecheney | menn0: i dunno how to incorprate signed zero feedback | 02:04 |
| davecheney | either your +1 or -1 | 02:04 |
| menn0 | it means, I don't like it, but I don't feel strongly enough about it that I'm going to ask you to stop | 02:04 |
| davecheney | fair enough | 02:05 |
| menn0 | too late now anyway | 02:05 |
| davecheney | noted | 02:05 |
| davecheney | i don't think there will be many more cases like this | 02:05 |
| lazyPower | hatch: bug tracker for the charm is on the GUI - http://i.imgur.com/MW05eAO.png, it should be in the README though. | 02:09 |
| lazyPower | hatch: what's going on with MYSQL? did you get a bug filed that I can look at? | 02:10 |
| === menn0 is now known as menn0-afk | ||
| thumper | got this interesting intermittent failure: http://paste.ubuntu.com/8228881/ | 02:42 |
| thumper | why the 2 minute waits? | 02:42 |
| thumper | anyone got any ideas? | 02:42 |
| wallyworld__ | thumper: i *think* the waits are added to allow mongo replicaset to come up | 02:47 |
| wallyworld__ | from memory | 02:48 |
| thumper | ugh | 02:48 |
| wallyworld__ | which sucks. it's bad enough we have "unit" tests with mongo required, but to also turn on the replicaset stuff for *all* tests is even worse | 02:48 |
| wallyworld__ | if we must use mongo for now for tests, why have the replicaset stuff enabled unnecessarily | 02:49 |
| davecheney | wallyworld__: didn't someone note that recently | 02:51 |
| davecheney | i have a memory there was a note in the weekly minutes to stop doing that to ourseles | 02:51 |
| wallyworld__ | davecheney: probably, but it hasn't been done i don't think | 02:52 |
| wallyworld__ | i was hoping nate would be doing it since he's looking at the replicaset tests | 02:52 |
| wallyworld__ | i'll ask him | 02:53 |
| davecheney | ta | 02:54 |
| axw | wallyworld__: I don't understand your comment about Tools tests & primitives | 03:02 |
| axw | what do you want to see? | 03:02 |
| wallyworld__ | axw: it was a suggestion that we use things other than the component under test to test itself. so for Tools(), we would get a blobstore, write some data manually, and check that Tools() can load it. ie a simplified bit of code without error checking and with hard coded values or whatever | 03:04 |
| wallyworld__ | AddTools() can still use Tools() in its test because Tools() has been separately tested | 03:04 |
| axw | I see | 03:05 |
| axw | ok | 03:05 |
| wallyworld__ | does that make sense? do you agree? | 03:05 |
| davecheney | thumper: menn0-afk https://github.com/juju/juju/pull/671 | 03:05 |
| davecheney | one more, less contentious this time | 03:05 |
| === Ursinha is now known as Ursinha-afk | ||
| wallyworld__ | otherwise i could write an implementation that passes the tests but which doesn't work | 03:05 |
| === menn0-afk is now known as menn0 | ||
| === Ursinha-afk is now known as Ursinha | ||
| * thumper fears the rebase | 03:17 | |
| * thumper takes a deep breath | 03:17 | |
| davecheney | thumper: never go the full rebase | 03:18 |
| thumper | why? | 03:19 |
| davecheney | thumper: yes, agent/bootstrap.go depends on state | 03:19 |
| * davecheney really wishes our package had package comments | 03:19 | |
| davecheney | that way I wouldn't have to guess what the agent package did | 03:19 |
| davecheney | thumper: i agree, to swap one dependency for another would not be a win | 03:21 |
| davecheney | but agent already depends on state | 03:21 |
| thumper | yep | 03:21 |
| davecheney | now, i'm not sure if that makes sense | 03:21 |
| thumper | that is why I'm fine with this | 03:21 |
| davecheney | but i don't really know what the agent package is | 03:21 |
| davecheney | i'm guessing it's helpers for jujud agent processes | 03:21 |
| thumper | yeah | 03:22 |
| thumper | ideally it wouldn't depend on state | 03:22 |
| davecheney | no | 03:22 |
| davecheney | i can put that on the end of the list if you like | 03:22 |
| davecheney | thumper: that might be it for the changes today | 03:23 |
| davecheney | need to for arm64 paperwork this afternoon | 03:23 |
| thumper | kk | 03:23 |
| davecheney | and figure out how to come up with a plan for how to schedule 5 bits of concurrent work | 03:24 |
| menn0 | davecheney: that PR you just submitted has 7 commits, 4 of which have a commit message of just "wip" | 03:24 |
| davecheney | menn0: yup | 03:24 |
| davecheney | i don't rebase | 03:24 |
| davecheney | i can't make it work | 03:24 |
| menn0 | davecheney: that's awful | 03:24 |
| davecheney | yup | 03:24 |
| davecheney | we've discussed this before | 03:24 |
| menn0 | git rebase -i HEAD~<number of revs back> | 03:24 |
| menn0 | easy | 03:24 |
| davecheney | there has been no firm guidance here | 03:24 |
| menn0 | but you have to admit that a commit with a message of "wip" is pretty useless | 03:25 |
| davecheney | yup | 03:25 |
| davecheney | it's my old process from bzr | 03:25 |
| davecheney | menn0: i'm hoping that the bot will grow the ability to squash commits | 03:26 |
| menn0 | davecheney: but squashing commits or even just rewording the commit messages is ridiculously easy | 03:26 |
| menn0 | davecheney: nevermind.... I doubt I'm going to change your mind | 03:28 |
| thumper | menn0: I have a conflict during rebase | 03:31 |
| thumper | and I want to do a three way merge to see | 03:31 |
| thumper | any idea how? | 03:31 |
| menn0 | do you have a favourite merge tool? (e.g. meld or kdiff3) | 03:32 |
| thumper | yes | 03:32 |
| thumper | either of those is fine | 03:32 |
| menn0 | if so, type "git mergetool" at the conflict | 03:33 |
| menn0 | select the tool you want to use | 03:33 |
| menn0 | and it launches so that you can do the merge | 03:33 |
| menn0 | that's what I do | 03:33 |
| thumper | what do you mean "at the conflict" ? | 03:33 |
| menn0 | when the rebase stops due to the conflict | 03:34 |
| thumper | yes... | 03:34 |
| thumper | anything else? | 03:34 |
| menn0 | then you type "git mergetool" | 03:34 |
| menn0 | does that work for you? | 03:34 |
| menn0 | thumper: ^^^? | 03:37 |
| thumper | kinda | 03:37 |
| * thumper is in meld hoping he is doing the right thing | 03:37 | |
| menn0 | with meld I usually use "Merge All" and then check how well it did | 03:38 |
| menn0 | it normally does a pretty good job | 03:38 |
| menn0 | but for really tricky conflicts you sometimes need to do some hand editing or manual merges | 03:38 |
| menn0 | I didn't realise for sometime after I started using meld that you can edit in the middle pane | 03:39 |
| menn0 | :) | 03:39 |
| menn0 | thumper: ^^ | 03:39 |
| * thumper runs make to test | 03:47 | |
| thumper | haha | 03:49 |
| thumper | everything fails | 03:49 |
| thumper | fixed the two trivials | 03:50 |
| * thumper waits to try again | 03:50 | |
| thumper | coffee machine is calling | 03:50 |
| wwitzel3 | is there an easy way to push a new build to an existing juju environment? | 04:00 |
| wallyworld__ | wwitzel3: update-juju --upload-tools | 04:02 |
| wwitzel3 | wallyworld__: thanks | 04:02 |
| wallyworld__ | i've not used it myself :-) | 04:02 |
| wwitzel3 | I'm just peppering the code with Debugf statements and it gets old rebuilding the environment each time | 04:03 |
| wallyworld__ | yup | 04:05 |
| thumper | wallyworld__: or axw: https://github.com/juju/juju/pull/673 - trivial move | 04:17 |
| wallyworld__ | ok | 04:17 |
| thumper | gah... | 04:24 |
| * thumper rebased the wrong branch | 04:24 | |
| thumper | FFS | 04:24 |
| thumper | how? | 04:24 |
| thumper | nope | 04:25 |
| thumper | did the right branch | 04:25 |
| thumper | tried to merge the wrong branch | 04:25 |
| thumper | d'uh | 04:25 |
| hatch | hey anyone here know anything about haproxy? I'm trying to find documentation, release notes, etc, but the 'official' site hasn't been updated since 2013 | 04:37 |
| thumper | ugh... | 04:41 |
| thumper | I need to apply a reverse merge for just one file from a previous commit | 04:41 |
| thumper | git master help needed | 04:41 |
| thumper | just hit: [LOG] 0:00.293 ERROR juju.worker exited "apiserver": listen tcp :60622: bind: address already in use | 04:44 |
| hatch | thumper: so you want to remove one file from a previous commit? | 04:47 |
| thumper | hatch: remove the changes to one file from a previous commit, yes | 04:47 |
| hatch | thumper: is it a recent commit? | 04:49 |
| thumper | two back | 04:49 |
| hatch | git rebase -i HEAD~2 then change 'pick' for the commit in question to 'edit' | 04:49 |
| hatch | if I am remembering correctly | 04:50 |
| hatch | I believe edit will allow you to modify the commit | 04:50 |
| thumper | hmm... ok | 04:50 |
| hatch | then once you do the changes you do `git commit --ammend` I believe | 04:52 |
| hatch | sorry I don't have a local repo to test with atm :) | 04:52 |
| hatch | thumper: work? | 04:57 |
| thumper | yeah | 04:57 |
| hatch | excellent | 04:57 |
| hatch | thumper: rebase has a lot of really awesome functionality, if you ever get some free time it's definitely worth reading the docs on it | 04:58 |
| wallyworld__ | axw: whenever you're free, i've updated blobstore to just use sha384 checksums https://github.com/juju/blobstore/pull/14 | 05:05 |
| axw | looking | 05:08 |
| === uru-bot is now known as urulama | ||
| wallyworld__ | axw: ty, i've merged as we don't have a landing bot yet | 05:18 |
| axw | cool | 05:18 |
| davecheney | menn0: i'm sorry about the ugly commit | 05:26 |
| davecheney | i'll try to produce better PR's int he future | 05:26 |
| menn0 | davecheney: :) | 05:27 |
| davecheney | menn0: thumper | 05:32 |
| davecheney | i'm at the stage where I have a bunch of enumeration types | 05:33 |
| davecheney | lets say params.Life | 05:33 |
| davecheney | that's an easy one | 05:33 |
| davecheney | they are defined in params | 05:33 |
| davecheney | but used in state as they are stored in the database | 05:33 |
| davecheney | now, the first suggestion would be to move them into state | 05:33 |
| davecheney | which makes sense for the apiserver | 05:33 |
| davecheney | but that means api clients would also be importing state | 05:33 |
| davecheney | and that feels wrong | 05:34 |
| thumper | yes... | 05:34 |
| davecheney | thoughts ? | 05:34 |
| thumper | this is why we want a general separate package that handles that :) | 05:34 |
| davecheney | i think there is general agreements that the api/ packages should know nothing about state | 05:34 |
| * thumper is being called for dinner | 05:34 | |
| thumper | agreed | 05:34 |
| davecheney | thumper: yes, but then both state and the apiserver and api clietns have to import those | 05:34 |
| davecheney | and that feels like odd coupling | 05:34 |
| davecheney | i'm thinking about using raw types in state, ie strings | 05:35 |
| davecheney | then having parse and tostring() in the params pacakge | 05:35 |
| davecheney | so the api and apiserver deal with enumerated types | 05:35 |
| davecheney | and we convert them to strings to be stored in the databse | 05:35 |
| davecheney | or something | 05:35 |
| davecheney | similar to the method I wrote that converrts | 05:35 |
| davecheney | state.StateServingInfo -> params.StateServingInfo | 05:35 |
| menn0 | davecheney: seems ok except we then don't have nice friendly constants to use in state code | 05:37 |
| davecheney | menn0: i agree | 05:37 |
| menn0 | which kinda sucks | 05:38 |
| davecheney | and, taking params.Life as an example again | 05:38 |
| davecheney | it does have helper methods on it that make it more than just an alias, liek StatusData was | 05:38 |
| davecheney | menn0: hang on, give me a sec to try something | 05:38 |
| davecheney | hmm, what about | 05:39 |
| davecheney | package state; type Life int const ( LifeAlive << iota .. ) | 05:39 |
| davecheney | ^ this is what we have in params at the moment | 05:40 |
| davecheney | then in apiserver/params | 05:40 |
| davecheney | type Life state.Life | 05:40 |
| davecheney | these should still marshal across the wire | 05:40 |
| davecheney | and callers of the api won't be able to tell | 05:40 |
| davecheney | there will be a transitive dependency from api -> apiserver -> state | 05:40 |
| davecheney | but not a direct one | 05:40 |
| davecheney | it will also prevent anyone using api/ types in state | 05:41 |
| menn0 | that seems pretty good to me | 05:41 |
| davecheney | let me try a PR | 05:41 |
| davecheney | see how it looks | 05:41 |
| davecheney | that means in the apiserver | 05:41 |
| davecheney | you do | 05:41 |
| davecheney | f(life params.Life) { st.Something(state.Life(life)) } | 05:41 |
| davecheney | which is probably acceptable, given the constraints | 05:41 |
| menn0 | and if we decide that there needs to be a difference between the constants in the apiserver vs state, it could be unpicked fairly easily | 05:42 |
| * menn0 is EOD... | 05:43 | |
| davecheney | kk | 05:44 |
| fwereade | wallyworld__, I have to go into town to see the dentist shortly; I think there's just one major unresolved thing in the qos/status stuff, but it's a biggie: it's the per-relation statuses | 05:45 |
| fwereade | wallyworld__, I can't see a way to do health well without relation granularity at least | 05:46 |
| wallyworld__ | fwereade: ok, i'm updating some comments in the doc now, yet to get to that bit | 05:46 |
| wallyworld__ | but i will | 05:46 |
| fwereade | wallyworld__, and I can't see a way to do relation-granular statuses easily and comprehensibly either | 05:46 |
| wallyworld__ | fwereade: i have soccer in an hour or so, perhaps we can talk after the TL meeting? | 05:46 |
| fwereade | wallyworld__, yeah sgtm | 05:46 |
| wallyworld__ | fwereade: what time is it? | 05:47 |
| fwereade | wallyworld__, FWIW it's not really about stuff in the impl doc, it's unresolved questions in the reqs doc | 05:47 |
| wallyworld__ | tooth-hurty :-D | 05:47 |
| fwereade | wallyworld__, haha | 05:47 |
| wallyworld__ | thought you'd "appeciate" that | 05:47 |
| fwereade | wallyworld__, oh, I do, I had a 2:30 dentist appointment when I was small and told teh dentist that exact joke | 05:48 |
| wallyworld__ | \o/ | 05:48 |
| fwereade | wallyworld__, he was very nice and pretended he'd never heard it before | 05:48 |
| wallyworld__ | lol | 05:48 |
| * fwereade has to go | 05:49 | |
| wallyworld__ | axw: do we need to, or should we be, including the uuid in the tools url? are tools something that we care about segregating per environment? | 05:58 |
| axw | wallyworld__: they are in the URL | 05:59 |
| wallyworld__ | axw: yes, that's what i'm referring to | 05:59 |
| axw | https://<apiaddr>/environment/<uuid>/tools/<version> | 05:59 |
| axw | wallyworld__: ah, sorry thought you thought they weren't and should be | 06:00 |
| axw | um | 06:00 |
| wallyworld__ | one sec, someone at door | 06:00 |
| axw | wallyworld__: I think one user should not affect another user's env by manipulating tools | 06:01 |
| wallyworld__ | axw: that's a fair point | 06:01 |
| wallyworld__ | and the data is de-deuped anyway | 06:01 |
| axw | they're going to be deduped in the blobstore | 06:01 |
| axw | yeah | 06:01 |
| wallyworld__ | yup :-) | 06:01 |
| axw | wallyworld__: we need to support 1.18 upgrading directly to 1.21? my understanding was that the client needs to be compatible, but upgrade still need to go through the hops | 06:10 |
| wallyworld__ | axw: yes, correct, upgrade steps are run one version at a time. i was worried if older clients attempted to use that attribute if it is taken away, i could be wrong | 06:11 |
| axw | wallyworld__: I mean, I thought the client still was required to upgrade to 1.20 first, and then to 1.21 | 06:12 |
| wallyworld__ | axw: we need to support people using juju 1.18 clients "forever" | 06:12 |
| wallyworld__ | even with the back end upgraded | 06:12 |
| axw | wallyworld__: everything except upgrading directly to 1.21 will work still | 06:13 |
| axw | if you want to upgrade you still need to go to 1.20, then to 1.21 (this is my assumption) | 06:13 |
| wallyworld__ | axw: sure, but i might upgrade to 1.22 and you may still want to use an older 1.18 client | 06:13 |
| wallyworld__ | we need to always ensure 1.18 clients can be used with any 1.20, 1.22 etc | 06:14 |
| axw | wallyworld__: that'll be fine. the bit of code in question is only relevant to the upgrader | 06:14 |
| wallyworld__ | axw: ok, np. just being doubly sure by asking the question | 06:15 |
| axw | sure | 06:15 |
| wallyworld__ | we'd be in the shit if stuff broke :-) | 06:15 |
| thumper | wallyworld__, davecheney, axw, someone: https://github.com/juju/juju/pull/675 | 06:20 |
| wallyworld__ | thumper: currently reviewing, will look after i finish unless i have to bail for soccer | 06:21 |
| thumper | wallyworld__: ack | 06:21 |
| davecheney | menn0: state already has a Life type | 06:21 |
| davecheney | and it's defined as a uint8 ... | 06:21 |
| davecheney | so we have params.Life, a string | 06:22 |
| davecheney | and state.Life, an uint8 | 06:22 |
| thumper | ick | 06:23 |
| thumper | wallyworld__: if you are happy with my branch, please add the merge flags | 06:24 |
| wallyworld__ | will do | 06:24 |
| thumper | wallyworld__: I'm off for the evening until meeting time | 06:24 |
| thumper | cheers | 06:24 |
| mattyw | morning all | 06:24 |
| wallyworld__ | o/ | 06:24 |
| mattyw | wallyworld__, thanks for merging my branch - it makes me very happy | 06:24 |
| wallyworld__ | mattyw: i landed your branch for you :-) | 06:24 |
| wallyworld__ | np | 06:24 |
| wallyworld__ | mattyw: i wanted to get in in case CI was blocked again before your SOD :-) | 06:25 |
| mattyw | wallyworld__, I appreciate it, thanks very mcuh | 06:26 |
| wallyworld__ | anytime | 06:26 |
| dimitern | morning all | 06:53 |
| wallyworld__ | axw: off to soccer, i lgtm'ed your pr with a suggested change | 07:07 |
| axw | wallyworld__: thanks | 07:07 |
| axw | enjoy | 07:07 |
| tasdomas | morning | 07:31 |
| tasdomas | could somebody take a look at https://github.com/juju/juju/pull/667 ? | 07:31 |
| dimitern | tasdomas, morning, and looking :) | 07:39 |
| tasdomas | dimitern, thanks | 07:39 |
| dimitern | tasdomas, reviewed, i'm afraid it looks like it needs a bit more | 08:13 |
| tasdomas | dimitern, thanks | 08:16 |
| TheMue | morning btw | 08:37 |
| voidspace | TheMue: morning | 08:37 |
| TheMue | voidspace: ah, good to see you. sharing room in Brussels? | 08:38 |
| * TheMue btw fight with code that yesterday before merging worked but not now anymore *wonder* | 08:38 | |
| voidspace | TheMue: I have a roomie TheMue | 08:45 |
| voidspace | TheMue: if that was an offer, which I assume it was, thank you | 08:46 |
| * voidspace is also wrestling, but with lxc not code | 08:48 | |
| TheMue | voidspace: ok, thx for info | 08:51 |
| gsamfira | if anyone has a few minutes to spare, can I have a review on: https://github.com/juju/utils/pull/27 ? | 09:18 |
| jam2 | morning TheMue | 09:30 |
| TheMue | jam2: hi jam | 09:30 |
| jam2 | voidspace: I read that for a second as wrestling with your roomie. | 09:30 |
| jam2 | TheMue: I missed you yesterday, was I just not around when you were, or did I miss something else? | 09:31 |
| TheMue | jam2: I thought I told on Tuesday that I had a doc appointment | 09:31 |
| jam2 | TheMue: it is certainly possible that you did and I just forgot. No problem. Just turned out that Voidspace had a doc appointment, and dimiter had a power outage all during standup time. | 09:32 |
| jam2 | I was so lonely. | 09:32 |
| TheMue | jam2: my whole working day moved in the afternoon and evening, at about 11 I finished with merging and fixing conflicts :) | 09:32 |
| TheMue | jam2: hehe, next time I will be with you using my mobile hangout :D | 09:33 |
| TheMue | jam2: I even had to break my work in the afternoon for a short while, I got an urgent call by my daughter | 09:33 |
| TheMue | jam2: she and her friend bought furnitures and wanted to rent a larger transporter. but they are too young | 09:34 |
| TheMue | jam2: so they needed daddy as driver | 09:34 |
| jam2 | natefinch: I commented on https://github.com/juju/juju/pull/512 | 09:35 |
| voidspace | jam2: heh, not yet wrestling with a roomie | 09:40 |
| voidspace | jam2: that comes soon | 09:40 |
| voidspace | jam2: I finally have juju running on an lxc under trickle | 09:40 |
| jam2 | voidspace: beware of thumper, he has a history there | 09:40 |
| voidspace | hah, I can believe it | 09:40 |
| voidspace | jam2: the fact that replicasets are taking *an age* to complete imply that it's working | 09:40 |
| voidspace | caching makes it slightly hard to tell | 09:40 |
| jam2 | voidspace: do you know about drop cache ? | 09:41 |
| voidspace | jam2: I don't, but I will shortly... thanks | 09:41 |
| jam2 | voidspace: http://www.linuxinsight.com/proc_sys_vm_drop_caches.html | 09:41 |
| voidspace | I didn't rate limit it too much whilst I was installing packages and fetching the juju dependencies | 09:41 |
| voidspace | 2MB/s | 09:41 |
| voidspace | I completely failed to get nbd-server to read a config file and actually start | 09:42 |
| voidspace | I ended up forcing it to skip the config file and specifying the parameters at the command line, which is actually a deprecated way of starting it | 09:42 |
| voidspace | https://plus.google.com/114852031032123777881/posts/LgLST5uPHsJ | 09:42 |
| stub | Interesting... my agent died while in debug-hooks. Anything I can do to diagnose? Its lxc. | 09:43 |
| voidspace | and as a testimony to my experimentation I now have an lxc container I can't destroy (as it's filesystem is on a disk that no-longer exists) | 09:43 |
| jam2 | voidspace: and inode handles keep them all alive? | 09:46 |
| jam2 | stub: so you can pastebin the panic | 09:46 |
| stub | http://pastebin.ubuntu.com/8231693/ | 09:47 |
| stub | jam2: this chan prob better ;) | 09:47 |
| jam2 | stub: it looks cut off at the start, though I'm not sure | 09:48 |
| jam2 | I only see from goroutine 28 | 09:48 |
| stub | jam2: yeah, hang on. paste fail. | 09:49 |
| * stub looks to see if it logged locally, outside of debug hooks terminal | 09:50 | |
| stub | http://pastebin.ubuntu.com/8231715/ | 09:51 |
| stub | jam2: So, yeah. I typed 'relation-get db' or something and killed it :) | 09:51 |
| jam2 | stub: so yeah... if a Charm can Panic Jujud, that's *bad*. Can you file a bug with just that first part of the traceback? | 09:52 |
| jam2 | I can understand how that code could be triggered | 09:52 |
| jam2 | (something assumes that it is getting a valid object, and then turns it into a Tag that assumes the input is well formatted and panics if it isn't) | 09:53 |
| jam2 | davecheney: ^^ | 09:53 |
| jam2 | looks like a case where we might be trying to validate input | 09:53 |
| jam2 | but it is getting turned into a Tag before we query the DB | 09:53 |
| * davecheney looks | 09:53 | |
| jam2 | but that panic() rather than giving a "no such thing" error | 09:53 |
| stub | relation-get - db | 09:53 |
| stub | That was what I typed | 09:54 |
| davecheney | yup, the bug is github.com/juju/juju/state/api/uniter.(*RelationUnit).ReadSettings | 09:54 |
| davecheney | is using NewUnitTag | 09:54 |
| jam2 | stub: sure, it probably is supposed to be "relation-get unit-db-0" | 09:54 |
| davecheney | not ParseUnitTag | 09:54 |
| stub | yeah, it was my fat fingers | 09:54 |
| jam2 | stub: but regardless, it is a bug in Jujud code | 09:54 |
| davecheney | jam2: hmm, not sure | 09:54 |
| jam2 | that we would panic() because of bad user input | 09:54 |
| davecheney | i think it should be db/0 | 09:54 |
| davecheney | that is the name of the unt | 09:54 |
| davecheney | unit | 09:54 |
| jam2 | davecheney: sure, I'm not quibbling on what Stub should type, but *jujud* should not panic because of a typo in a charm | 09:55 |
| davecheney | jam2: no argument there | 09:55 |
| jam2 | stub: I'd consider this a Critical bug, fwiw, since it means a bad charm makes your environment unreliable | 09:55 |
| jam2 | stub: or davecheney: can you file a bug to track this? I'm currently in 2 other conversations | 09:56 |
| stub | https://bugs.launchpad.net/juju-core/+bug/1365412 | 09:56 |
| mup | Bug #1365412: relation-get with invalid relation name panics agent <juju-core:New> <https://launchpad.net/bugs/1365412> | 09:56 |
| TheMue | aaaargh, found why it's not working anymore | 09:56 |
| jam2 | stub: is this with 1.20 or only with juju trunk? | 09:59 |
| stub | jam2: 1.20 | 09:59 |
| tasdomas | dimitern, thanks for the review - I've responded to the comments | 10:00 |
| voidspace | jam2: I think it's lxc configuration that prevents it from destroying the container | 10:10 |
| voidspace | jam2: it shows up under lxc-ls but lxc-destroy reports that the container doesn't exist | 10:10 |
| davecheney | jam2: stub i have a fix | 10:10 |
| davecheney | PR coming RSN | 10:10 |
| jam2 | great davecheney ! | 10:10 |
| dimitern | tasdomas, cheers, will look in a bit | 10:11 |
| davecheney | jam2: stub https://github.com/juju/juju/pull/677 | 10:21 |
| jam2 | davecheney: is there a function like NewUnitTag that would do the check for us rather than doing IsValidUnit call? | 10:23 |
| davecheney | jam2: nope | 10:23 |
| davecheney | the value we're handling is not a tag | 10:23 |
| davecheney | so names.ParseUnitTag() is not the right hammer either | 10:24 |
| stub | My, that looks like parameter validation ;) | 10:24 |
| davecheney | but, there is a simpler fix | 10:24 |
| perrito666 | good morning | 10:24 |
| jam2 | davecheney: ah right, it is the name not the tag | 10:24 |
| davecheney | if you're prepared to send this to the API server | 10:24 |
| * perrito666 sees a heavy storm outside and announces that he might be suddenly disconnected | 10:24 | |
| davecheney | note that we're turing the uname into a tag, then calling tag.String() on it to pass it to the api | 10:24 |
| davecheney | why both ? | 10:24 |
| davecheney | why bother | 10:24 |
| davecheney | actually, not | 10:25 |
| davecheney | we need to | 10:25 |
| davecheney | beause the api only accepts tags | 10:25 |
| davecheney | this fix is correct | 10:25 |
| jam2 | davecheney: is there a reason NewUnitTag should panic() rather than having error like other things? | 10:25 |
| davecheney | yes, it's a testing function | 10:25 |
| davecheney | it's use in prod code is a smell | 10:25 |
| davecheney | i discourage anyone from calling it | 10:25 |
| jam2 | davecheney: how do you turn user input into name.Tag objects then? | 10:26 |
| davecheney | depends | 10:26 |
| davecheney | if the input is a string form of a tag with ParseTag | 10:26 |
| davecheney | if not, we need to validate it first | 10:26 |
| davecheney | then handle it with care | 10:26 |
| TheMue | *iiirks* | 10:27 |
| jam2 | davecheney: it sounds like we want something similar to ParseTag (ParseName) ? | 10:27 |
| jam2 | davecheney: it certainly seems like validating user input for these things should be common code in the names package | 10:27 |
| jam2 | anyway, your fix seems fine for now, but it doesn't seem "ideal" in the design sense. | 10:27 |
| davecheney | jam2: i agree | 10:28 |
| davecheney | the use of NewXXXTag is very dangrous | 10:28 |
| davecheney | i discourage everyone from using it | 10:28 |
| davecheney | my use of it in this case was a mistake because I think at the time i started this work I didn't understand the scope of the problem | 10:28 |
| TheMue | jam2: is it intended that once the client requests a higher version of an API that doesn't exist on the server even if the API and the called function exist in a lower valid version, it fails (means: returns an error)? | 10:29 |
| jam2 | TheMue: it is intended, yes | 10:30 |
| jam2 | the client is responsible for noticing that the higher version is not available | 10:30 |
| TheMue | jam2: so once introducing a new version maybe the whole client has to changed, because now an error is returned and an explicit call to a lower version (where the code intenionally has been written for) has to be done | 10:32 |
| jam2 | TheMue: BestAPIVersion has already worked out what version is available and makes calls against that version | 10:32 |
| jam2 | the point is that api/client.go should know about the available versions and give the compatible internal API | 10:33 |
| jam2 | TheMue: so you would have Client.DoSomething(), which internally might be calling "Client", 1, "DoSomething", args | 10:33 |
| jam2 | or might be calling "Client", 0, "DoSomethingElse", otherargs | 10:33 |
| TheMue | jam2: it does it when starting once by retrieving them from the server, doesn't it? | 10:34 |
| jam2 | TheMue: at Login time we find out what facade versions the server has available | 10:34 |
| TheMue | jam2: then here maybe my troubles are | 10:34 |
| jam2 | and when instantiating the api.Client object it determines (FacadeCaller) what the matching version is | 10:34 |
| voidspace | jam2: running the AddRemoveSet tests on my machine (normally) takes 60 seconds | 10:34 |
| voidspace | jam2: running them on an lxc rate-limited to 2MB/s up/down takes 4 minutes | 10:35 |
| voidspace | jam2: limited to 1MB/s dies with unreachable servers | 10:35 |
| TheMue | jam2: the code for temporarilly discarding a version has also to do this on the already retrieved versions | 10:35 |
| voidspace | jam2: (this is with the extraneous Remove removed) | 10:35 |
| jam2 | voidspace: sounds like a good start, you can probably increase some timouts to see if unreachable servers can be recovered from. | 10:35 |
| voidspace | jam2: ok, I'll look at that - I might try with an intermediate value as well | 10:36 |
| voidspace | jam2: I'd like to get a good few runs with a slow disk showing that we don't die in test cleanup when we don't have the Remove | 10:36 |
| jam2 | voidspace: well for our tests to be reliable more consistenty, I'm think we probably need to change the timeouts, and move it to CI | 10:36 |
| jam2 | voidspace: well, first I'd like you to see that we *do* die in cleanup with Remove there | 10:37 |
| jam2 | voidspace: always have the test fail first | 10:37 |
| jam2 | before you fix it | 10:37 |
| voidspace | ah, ok - good call | 10:37 |
| jam2 | voidspace: standard bugfixing, you have to know that you're reproducing the bug before you know that your fix is actually fixing it :). | 10:39 |
| voidspace | jam2: sure :-) | 10:39 |
| voidspace | that's why changing existing tests is dangerous | 10:39 |
| voidspace | unless you also make sure you see the *modified test* fail | 10:39 |
| voidspace | it's easy to change a good test into a no-op without noticing... | 10:39 |
| voidspace | of course, testing replicasets on a *deliberately slow* system is even more painful than normal | 10:40 |
| jam2 | voidspace: dimitern: standup? | 10:46 |
| jam2 | voidspace: yeah, replication is painful | 10:46 |
| voidspace | omw | 10:46 |
| dimitern | jam2, omw, sorry | 10:47 |
| voidspace | jam2: dimitern: TheMue: you can tell the next door neighbours kids have gone back to school - my internet connection is now good enough for google hangouts... | 11:10 |
| jam2 | voidspace: :) | 11:10 |
| TheMue | voidspace: *lol* | 11:14 |
| dimitern | :) | 11:21 |
| dimitern | tasdomas, you've got another review with some suggestions | 11:21 |
| thumper | is anyone else having trouble pushing master to github? | 11:40 |
| thumper | if not, are you using the pre-push hook? | 11:40 |
| mgz_ | thumper: I can try now | 11:41 |
| thumper | mgz_: ta | 11:41 |
| * mgz_ suspects windows syslog changes | 11:44 | |
| mgz_ | and indeed, seems to be | 11:44 |
| mgz_ | lets just double check... | 11:45 |
| mgz_ | hm, happy after I did a dep upgrade | 11:46 |
| mgz_ | thumper: works for me | 11:46 |
| mgz_ | on trusty, after running godeps -u dependencies.tsv | 11:47 |
| mgz_ | $ git push origin master | 11:47 |
| mgz_ | Everything up-to-date | 11:47 |
| thumper | state/metrics_test.go:42:11: (*github.com/juju/juju/state.Metric).Key -> <nil> | 11:47 |
| thumper | MethodSet {} | 11:47 |
| thumper | panic: method sets and lookup don't agree [recovered] | 11:47 |
| thumper | panic: method sets and lookup don't agree [recovered] | 11:47 |
| thumper | panic: method sets and lookup don't agree | 11:47 |
| jam2 | voidspace: so it sounds like you're really close to the "land the code" state, is that true? | 11:47 |
| thumper | that is what I get | 11:47 |
| jam2 | voidspace: because I have other work for you to be looking into if you do. | 11:47 |
| mgz_ | thumper: I have that line, but not the panic. go 1.2.1 | 11:49 |
| jam2 | TheMue: voidspace: dimitern: we're going to be picking up the PortRange changes from domas, so it would be good for a couple of you to read over the patch so far | 11:49 |
| jam2 | tasdomas: can you work with those guys ^^ to help share knowledge of what is going on? | 11:49 |
| mgz_ | thumper: I guess if you just do `./scripts/pre-push.bash` you get that panic? | 11:50 |
| dimitern | jam2, i've reviewed it twice and am familiar with the previous work | 11:50 |
| voidspace | jam2: well - Remove the Remove is ready to land just need to create the PR | 11:50 |
| jam2 | voidspace: yep. | 11:50 |
| voidspace | jam2: the replicaset CheckHealth function (name?) is done but needs a test / logging / deciding where to actually use it | 11:50 |
| jam2 | voidspace: sounds reasonable | 11:50 |
| voidspace | jam2: if we just use it in the tests instead of sleeps then I can land that today too | 11:50 |
| jam2 | (name wise) | 11:51 |
| voidspace | jam2: and it probably doesn't need its own tests if we just use it in the tests | 11:51 |
| jam2 | voidspace: think carefully about it, and I'll let your educated thoughts drive where we put it :) | 11:51 |
| === allenap_ is now known as allenap | ||
| voidspace | jam2: heh, ok | 11:51 |
| jam2 | voidspace: The only good way to test it is by mocking out mongo | 11:51 |
| jam2 | IMO | 11:51 |
| tasdomas | jam2, TheMue, voidspace: I still need to update the patch w.r.t fwereade's comments and I have a day off tomorrow - could we set aside time to go over this on Monday? | 11:51 |
| jam2 | voidspace: I liked your ideas about having Add wait until things should be reliable | 11:51 |
| voidspace | jam2: yep | 11:52 |
| voidspace | jam2: ah, ok - I thought you *weren't* keen on that | 11:52 |
| jam2 | voidspace: but I'm not deep in the code to have a good feeling for how it all hangs together | 11:52 |
| jam2 | voidspace: sorry, it sounds like we must have been talking past eachother. I think haiing things transition from a known good state to another known good (enough) state is worthy | 11:53 |
| jam2 | and Add() seems like the place where you might do that. | 11:53 |
| voidspace | jam2: let me get the function usable, after I PR Remove the Remove, and look at plugging it into Add / Remove | 11:53 |
| voidspace | jam2: ok, cool - what about Remove? | 11:53 |
| voidspace | which is already slow | 11:53 |
| jam2 | The issue in the code is whether it expects things to be more asynchronous and would work poorly if we waited here and tehre. | 11:54 |
| voidspace | right | 11:54 |
| jam2 | voidspace: changing the replicaset should be really infrequent | 11:54 |
| jam2 | so waiting seems great | 11:54 |
| voidspace | we have code (in tests) that expect stuff to be unstable and just retries the next operations until they succeed | 11:54 |
| voidspace | which is horrible | 11:54 |
| jam2 | Again, we can make "waiting" be until quorum | 11:54 |
| voidspace | jam2: I'll look around it | 11:55 |
| jam2 | or maybe "wait X for everything stable, then back off to Y for quorum" | 11:55 |
| voidspace | yep, with logging | 11:55 |
| jam2 | voidspace: I like the idea with you can spend a bit of time for full stability as long as it doesn't really impact | 11:55 |
| voidspace | I'll see how they are used in the code and the wait strategies around them | 11:55 |
| jam2 | better than always returning immediately because you always have quorum | 11:55 |
| voidspace | and as the current operations leave the state unstable, we're either *already* waiting | 11:56 |
| voidspace | or we have code that can sometimes fail horribly | 11:56 |
| voidspace | so we're either replacing one wait with a better wait, or fixing a bug | 11:56 |
| voidspace | so it sounds like a win | 11:56 |
| jam2 | voidspace: right, and it is certainly possible that in your slow-case testing you'll find we can't reliably work with the system in a pre-commit test timeout | 11:57 |
| jam2 | (which IMO, real unit tests should be milliseconds....) | 11:57 |
| wallyworld_ | fwereade: i'm just about to do our standup, can i ping you after that, soon? | 11:57 |
| voidspace | jam2: I didn't quite parse that - you mean that waiting for stable should help with the slow system timeouts | 11:58 |
| voidspace | because we're waiting for stable state | 11:58 |
| jam2 | voidspace: I mean that waiting-for-stable-state may make the test unsuitable for the pre-commit suite and it needs to go into CI | 11:59 |
| voidspace | (specifically I couldn't parse "with the system in a pre-commit test timeout") | 11:59 |
| jam2 | and while I don't want to lose you to it, we need people who know how to add stuff to CI, so I'm willing for you to spend the time there. | 11:59 |
| voidspace | jam2: ah | 11:59 |
| voidspace | jam2: yep, parsed it now | 11:59 |
| voidspace | jam2: I thought "pre-commit" was mongo terminolody | 11:59 |
| voidspace | *terminology | 11:59 |
| voidspace | jam2: agreed | 12:00 |
| jam2 | voidspace: pre-landing-on-trunk | 12:00 |
| voidspace | yep | 12:00 |
| voidspace | But first, coffee | 12:00 |
| jam2 | :) | 12:01 |
| mgz_ | wait, what's this pre... I see | 12:01 |
| mgz_ | you're suggesting voidspaces adds this as a ci job rather than a unit test | 12:01 |
| jam2 | voidspace: I like the idea of having a "ci/" subdirectory that only runs in CI fwiw | 12:01 |
| mgz_ | ...why did I plural... | 12:02 |
| jam2 | mgz: so TestAddRemoveSet is currently in github-merge-juju | 12:02 |
| jam2 | mgz: we think that it needs to just be slower, which means it would be better as "run-trusty-tests" or whatever there | 12:02 |
| mgz_ | jam2: yeah, it sounds like a reasonable idea to me | 12:02 |
| mgz_ | the alternative is leave it in unit test form, but off unless --flag, and switch that on in the post-gating jobs... but that's pretty ugly | 12:03 |
| jam2 | mgz_: FWIW I like ENV_VAR more than --flag | 12:03 |
| jam2 | just because you can't pass --flag to "go test" reliabl | 12:03 |
| jam2 | reliably | 12:03 |
| mgz_ | yeah, that's a pain | 12:04 |
| jam2 | because it passes it to all packages | 12:04 |
| jam2 | not just the one you want | 12:04 |
| jam2 | otherwise I'd prefer flag | 12:04 |
| jam2 | I'm not a big fan on ENV magic, but go test seems close to requiring it | 12:04 |
| thumper | mgz_: yeah... I have ./scripts/pre-push.bash hooked up as the git pre-push hook | 12:05 |
| thumper | makes pushes take a lot longer | 12:05 |
| mgz_ | thumper: me too | 12:05 |
| thumper | but it was in the docs | 12:05 |
| thumper | mgz_: so why is mine panicing? | 12:05 |
| mgz_ | that's what I want to know :) | 12:05 |
| * thumper is too tired to work it out | 12:07 | |
| * thumper goes to bed | 12:07 | |
| thumper | night all | 12:07 |
| voidspace | I made a radical decision, I went for tea instead of coffee | 12:11 |
| jam2 | voidspace: omg.... | 12:12 |
| voidspace | jam2: yes, I like having CI tests in trunk - so developers can write new ones easily and have a sense of ownership over them | 12:12 |
| voidspace | heh, indeed | 12:12 |
| jam2 | voidspace: that's crazy talk | 12:12 |
| jam2 | go back to coffee before you have any more | 12:12 |
| jam2 | ownership over tests.... NOOOOO! | 12:12 |
| voidspace | :-) | 12:12 |
| jam2 | then we're responsible when they fail | 12:12 |
| voidspace | it would be nice to reduce sinzui's blood pressure at least a *bit* | 12:12 |
| jam2 | voidspace: we need a lot more ownership of CI stability, so I'm very happy to hear it. | 12:13 |
| voidspace | yep | 12:13 |
| voidspace | jam2: https://github.com/juju/juju/pull/679 | 12:15 |
| jam2 | voidspace: just to confirm, you saw reliable failures under slow disk with the Remove in, and not with it out? | 12:16 |
| voidspace | jam2: reliable as in about 3 runs out of 4 | 12:16 |
| voidspace | jam2: but yes | 12:16 |
| jam2 | voidspace: that is good reproducibility and good reduction without them, right? | 12:17 |
| jam2 | great | 12:17 |
| voidspace | jam2: yep | 12:17 |
| jam2 | voidspace: LGTM | 12:17 |
| voidspace | jam2: thanks | 12:18 |
| === ChanServ changed the topic of #juju-dev to: https://juju.ubuntu.com | On-call reviewer: see calendar | Blocking bugs: 1365480 | ||
| perrito666 | mattyw: ping without hurry | 12:44 |
| voidspace | natefinch: ping | 13:07 |
| jam2 | TheMue: can you have a look at https://github.com/juju/juju/pull/392 ? It is doing versioning of the root Login function, and I think it would be worthwhile to have you think about it, too. | 13:08 |
| jam2 | I'm mostly happy, though I had been hoping we wouldn't have to version the *entry point* into the system. | 13:08 |
| jam2 | We can only do that *today* because 1.18 would let you Login even if you pass a version, and 1.20 will give you CodeNotImplemented if you did so. | 13:08 |
| === wwitzel3_ is now known as wwitzel3 | ||
| * voidspace lunch | 13:15 | |
| TheMue | jam2: yep, will do | 13:16 |
| mattyw | perrito666, ping | 13:55 |
| mattyw | perrito666, pong, even | 13:55 |
| natefinch | wwitzel3: standup? | 14:05 |
| wwitzel3 | yep, sorry | 14:06 |
| fwereade | axw, ping? must be late for you | 14:22 |
| fwereade | axw, wallyworld_, if either of you are awake: in the context of "drop provider storage", how far are we from the "can do HA manual provider" subgoal? | 14:23 |
| fwereade | axw, wallyworld_, wwitzel3: because the cloudsigma thing will be incoming, and last I looked that was still using the manual storage method, which means that won't be HA-capable either | 14:24 |
| bodie_ | I thought we were avoiding gopkg.in | 14:35 |
| fwereade | bodie_, I didn't think so? | 14:38 |
| === urulama is now known as urulama_afk | ||
| bodie_ | hum, I got some pushback about it a while back, just didn't know whether I should worry about it | 14:39 |
| voidspace | natefinch: I've submitted a new expenses claim I would appreciate you approving | 14:45 |
| voidspace | natefinch: travel for Lexington sprint | 14:46 |
| natefinch | voidspace: ok sure | 14:48 |
| === benji_ is now known as benji | ||
| perrito666 | natefinch: ping me when you have a slot available | 15:06 |
| natefinch | perrito666: will do | 15:06 |
| === jog_ is now known as jog | ||
| katco` | hey, any opinions on how to name functions stuffed into var's for patching in tests? | 15:17 |
| katco` | i have environForName as the function now, i'd like to expose it with a var | 15:18 |
| === katco` is now known as katco | ||
| natefinch | katco: tricky | 15:19 |
| katco | natefinch: i really dislike this pattern =| | 15:20 |
| katco | natefinch: i'm thinking the var should be the name that makes sense since it should be the thing used everywhere, and then the func shouild be called *forPatching or something ugly to disuade use | 15:21 |
| natefinch | katco: yeah, that's a good idea | 15:21 |
| natefinch | katco: or *ProductionFunc or something | 15:21 |
| katco | natefinch: i like that a bit better | 15:21 |
| katco | natefinch: we'll see how that looks, thanks dude | 15:22 |
| natefinch | katco: welcome. I'm pretty good at coming up with ugly names ;) | 15:22 |
| katco | natefinch: haha aren't we all | 15:22 |
| bodie_ | WouldBeUglierIfJava | 15:34 |
| perrito666 | ouch is mattyw gone? | 15:34 |
| tasdomas | perrito666, mattyw is probably just having connection problems | 15:40 |
| perrito666 | * mattyw has quit (Max SendQ exceeded) | 15:41 |
| perrito666 | sounds like flooding | 15:41 |
| katco | evilnickveitch: ping | 15:55 |
| TheMue | *yeeeeeehaw* | 16:05 |
| TheMue | sorry for being loud, but this crazy versioning test now works as it should | 16:06 |
| jcw4 | TheMue: yay! | 16:07 |
| wwitzel3 | is there a way to upload new tools without re-bootstrapping? I tried juju update-tools but that isn't a command. | 16:07 |
| katco | wwitzel3: juju upgrade-juju --upload-tools maybe? | 16:11 |
| === hatch__ is now known as hatch | ||
| wwitzel3 | katco: yeah, tried that, but it just says they are the same version, which is true | 16:12 |
| wwitzel3 | :) | 16:12 |
| wwitzel3 | wondering if I hacked the version and incremented it, if it would work then | 16:12 |
| katco | wwitzel3: ah. look at the status, it should be incrementing the version. i think that error is... well erroneous ;) | 16:12 |
| katco | wwitzel3: yeah, before: agent-version: 1.21-alpha1.1; after: agent-version: 1.21-alpha1.3 | 16:15 |
| katco | wwitzel3: i remember reading up on this; there is a bug or something out there to simplify all this behavior. it's a very confusing command. | 16:15 |
| katco | wwitzel3: also check your path to make sure you're not using the one from apt | 16:16 |
| perrito666 | lunch brb | 16:48 |
| === uru_ is now known as urulama_afk | ||
| wwitzel3 | katco: sorry, forgot to say thanks :) | 17:54 |
| katco | wwitzel3: haha no worries. although i _was_ wondering if it worked! | 17:55 |
| katco | wwitzel3: btw i tried some bourbon the other day... maker's mark? i think? it wasn't as good as the stuff you bought in boston. but i'm a bourbon newb. | 17:57 |
| wwitzel3 | katco: I wish markers was a good as the stuff I buy, well actually, Jessa wishes it more than me :) | 17:58 |
| katco | wwitzel3: haha | 17:58 |
| katco | wwitzel3: i'm guessing the stuff you're into is rather pricey | 17:59 |
| perrito666 | natefinch: team meeting? | 18:06 |
| perrito666 | anyone else? | 18:06 |
| wwitzel3 | ericsnow: ping | 18:09 |
| ericsnow | wwitzel3: coming | 18:09 |
| perrito666 | anyone else aroudn here that feels should be on this meeting? | 18:16 |
| jcw4 | perrito666: sure ! :) | 18:22 |
| wwitzel3 | anyone know what line 50 of worker/uniter/runlistener.go is actually doing? | 18:39 |
| mattyw | night all | 18:39 |
| natefinch | wwitzel3: setting the value of result to the value of runResult | 18:41 |
| natefinch | wwitzel3: er vice versa | 18:41 |
| natefinch | wwitzel3: it's like an out parameter | 18:41 |
| natefinch | wwitzel3: I have no idea why they're not just returning the result and the error | 18:42 |
| wwitzel3 | natefinch: yeah, that's what I was wondering, if there was a reason to not just return them there | 18:44 |
| natefinch | wwitzel3: probably... maybe an interface that's being fulfilled or something (though why that interface itself wouldn't allow the result to get returned, I don't know) | 18:45 |
| wwitzel3 | the problem is, that if that RunCommands call actually does return nil for the result (as it does in all its error cases), that line throws a NPE | 18:45 |
| natefinch | wwitzel3: heh uh yeah, that's bad. | 18:45 |
| katco | wwitzel3: that code isn't run from within a closure by chance is it? | 18:46 |
| wwitzel3 | katco: yes, I think so, right | 18:47 |
| katco | wwitzel3: funny, i recently did this in a personal project. at the time of the creation of the closure, it needed the variable to assign to; i.e.: the closure was being run in such a way that it couldn't return a value due to an interface it had to conform to | 18:48 |
| katco | wwitzel3: and so i did a pointer dereference | 18:49 |
| natefinch | wwitzel3, katco: what's funny is that on line 24 there's an interface that is written in the right way, which returns results and the error | 18:49 |
| katco | natefinch: ok i call shenanigans on that code then :p | 18:50 |
| wwitzel3 | hrmm | 18:50 |
| natefinch | katco, wwitzel3: JujuRunServer is passed to an rpc server... that's the trick | 18:50 |
| katco | natefinch: so it can't return a result? | 18:51 |
| natefinch | katco: http://golang.org/pkg/net/rpc/#Server.Register | 18:52 |
| wwitzel3 | natefinch: thanks | 18:52 |
| wwitzel3 | katco: thanks | 18:52 |
| katco | natefinch: ah there we are. good find natefinch :) | 18:53 |
| === arosales_ is now known as arosales | ||
| wwitzel3 | well it still doesn't change the fact that if the results from runner.RunCommand are nil that it panics with a NPE | 19:02 |
| wwitzel3 | so I should probably fix that while I'm in here | 19:02 |
| katco | wwitzel3: quit being so proactive. | 19:03 |
| natefinch | wwitzel3: absolutely fix it. If err != nil, we shouldn't look at the other value at all | 19:04 |
| natefinch | ericsnow: what PR did you want me to review? | 19:13 |
| ericsnow | https://github.com/juju/juju/pull/606 | 19:13 |
| ericsnow | natefinch: thanks! | 19:13 |
| natefinch | ericsnow: I think you're trying too hard to hide stuff in that backups code | 19:51 |
| ericsnow | natefinch: I guess I'm trying to avoid the way we leak stuff all over the rest of our code base :) | 19:52 |
| ericsnow | natefinch: anything in particular? | 19:52 |
| natefinch | ericsnow: db.connInfo ... you should pretty much never have exported functions returning non-exported types | 19:53 |
| ericsnow | natefinch: that's a pretty crucial rule-of-thumb I wasn't aware of :( | 19:54 |
| ericsnow | natefinch: though it certainly makes sense | 19:54 |
| natefinch | ericsnow: well for one thing, it means you can't see documentation about the type in godoc. It also means you can't write a function that takes that type | 19:55 |
| natefinch | ericsnow: that's a really good example of a struct that really should just have all public fields. It doesn't need any methods at all | 19:55 |
| natefinch | ericsnow: if you really want the Check function, you can make it a standalone function that takes a ConnInfo and does basically what it does now. | 19:56 |
| ericsnow | natefinch: doesn't that put more burden on the caller unnecessarily? | 19:57 |
| natefinch | ericsnow: but honestly, the check function seems spurious. It might be useful as an internal helper function.. but ConnInfo is just a data bucket. Whether or not the address being empty is a bad thing depends on the function using it | 19:57 |
| ericsnow | natefinch: considering that the context is backups (specifically dumping the DB), I think the constraint is appropriate | 19:58 |
| natefinch | ericsnow: It ties the logic of what makes a valid conn info to the type, rather than to the function.... and it's only the function that cares. | 19:59 |
| ericsnow | natefinch: thanks for this feedback, by the way :) | 19:59 |
| natefinch | ericsnow: welcome :) | 20:00 |
| ericsnow | natefinch: but the point is that the type has a very specific use that is tied to that logic | 20:00 |
| ericsnow | natefinch: I'd expect a more general-purpose type to live in a more general-purpose package | 20:01 |
| natefinch | ericsnow: I like that Go makes it easy to separate data from logic. You don't need to have types and methods for everything. You can have a struct with just some fields and a standalone function that verifies the struct has the right things set. It would be a lot less code. I *do* think that NewMongoConnInfo is useful. That's some pretty specific logic there. The rest is just boilerplate stuff because you're hiding | 20:05 |
| natefinch | the fields in ConnInfo (and hiding ConnInfo itself). | 20:05 |
| natefinch | ericsnow: I know we were all taught that public variables are the devil. It turns out, well, sometimes they're really not... especially if it's just data, and not internal state. | 20:06 |
| ericsnow | natefinch: hey, I'm a Python guy, so the whole public/private thing is kind of new to me...I guess I've been feeling it out | 20:09 |
| natefinch | ericsnow: heh.... it;'s funny, because I come from C# / C++ etc where you have to think about that stuff all the time, and people are much more rigorous about never exposing anything... | 20:10 |
| ericsnow | natefinch: being able to syntactically express expectations like that is the one thing I like about static type systems | 20:10 |
| natefinch | ericsnow: I like being able to statically express the type of thing I'm expecting to get.... it's good for me and it's good for the person calling my code :) | 20:11 |
| natefinch | ericsnow: certainly, with Go's interfaces, it makes it a lot less burdensome on the caller than in other languages. "Give me something with a Read() method, I don't care what" is a lot nicer than "Give me exactly a ByteRead type, and if you don't have one, guess you're gonna have to make one and wrap your thing in it" | 20:12 |
| ericsnow | natefinch: type inference helps a lot too | 20:18 |
| natefinch | ericsnow: yep, definitely. | 20:18 |
| perrito666 | davecheney: if you have time, I LGTMd this https://github.com/juju/juju/pull/681 and therefore need your validation to assert my self worth | 21:06 |
| perrito666 | :p | 21:06 |
| davecheney | perrito666: sure | 21:13 |
| davecheney | looking | 21:13 |
| davecheney | i like it | 21:14 |
| davecheney | if we on't write our own logic | 21:14 |
| davecheney | we don't have to test it | 21:15 |
| davecheney | win/win! | 21:15 |
| davecheney | hmm | 21:15 |
| davecheney | i dunno how to review this | 21:15 |
| davecheney | i don't know if c.Remove respects the transaction system | 21:16 |
| davecheney | and if it doesn't what the results will be | 21:16 |
| davecheney | i'll write as such on the PR | 21:16 |
| perrito666 | davecheney: you are right, I somehow just assumed that it was not a problem since we are deleting and not changing (and therefore we do not worry about the main benefit of always work on an updated copy) | 21:18 |
| * perrito666 automatically looses all sense of self worth and crawls under his desk | 21:19 | |
| perrito666 | wallyworld_: say my name when you are available plz | 21:21 |
| wallyworld_ | perrito666: sure, on a call, so soon | 21:21 |
| * perrito666 actually needs to start using world clocks to know when people are here | 21:21 | |
| perrito666 | wallyworld_: davecheney what are your tzs? | 21:22 |
| davecheney | perrito666: ausstralian eastern time | 21:22 |
| davecheney | +10 (+11) I never remeber which | 21:23 |
| davecheney | you're in the same tz as US west coast right | 21:23 |
| perrito666 | davecheney: gmt -3 which I believe has 1h diff with west coast | 21:24 |
| perrito666 | well my clock puts you both tomorrow | 21:25 |
| davecheney | that is true, ian is from the future | 21:26 |
| perrito666 | that explains a lot | 21:26 |
| === dpb2 is now known as dpb1 | ||
| wallyworld_ | perrito666: hi, what's happening | 21:39 |
| perrito666 | wallyworld_: priv | 21:40 |
| === pjdc_ is now known as pjdc | ||
| thumper | morning folks | 22:17 |
| rick_h_ | thumper! | 22:17 |
| thumper | wallyworld_: check with gccgo before caring about power | 22:18 |
| menn0 | thumper: good moaning :) | 22:36 |
| thumper | menn0: 'ello 'ello | 22:36 |
| wallyworld_ | thumper: so just finished eating and looking at this bug, the log attached shows compile errors for power which makes me sad | 22:50 |
| wallyworld_ | the version on the test machine is 2 ahead of my gccgo which does compile it ok | 22:53 |
| wallyworld_ | ffs | 22:53 |
| wallyworld_ | awesome, i upgrade my compiler and it still works locally | 22:57 |
| waigani | menn0: standup? | 23:01 |
| wallyworld_ | \o/ lots of seg faults running tests wil gccgo | 23:19 |
| wallyworld_ | with | 23:19 |
| waigani | menn0: where were you going to make that one char change? | 23:47 |
| menn0 | waigani: cmd/jujud/machine.go:546 | 23:47 |
| waigani | cheers | 23:48 |
| menn0 | the call to net.Listen | 23:48 |
| wallyworld_ | davecheney: i think gccgo is broken again - it seems it no longer registers the hooks used in some of the tests, hence the code required to make the tests pass is never run | 23:53 |
| wallyworld_ | i think this may have come up before? | 23:54 |
| wallyworld_ | i have no idea what to do about it - i can't see that this should be a blocker that stops commits to trunk as it's not our code that is broken | 23:55 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!
