| davecheney | m_3: ec2 is having a less pissy day | 00:16 |
|---|---|---|
| davecheney | precise-ec2-charm-alice-irc passed ! | 00:16 |
| m_3 | davecheney: nice! | 00:21 |
| davecheney | two for two | 00:25 |
| davecheney | going to try with juju-core 1.9.12 | 00:25 |
| davecheney | m_3: and now ec2 is having a sad period | 01:14 |
| davecheney | 3 failures in a row | 01:14 |
| davecheney | m_3: aww crap | 01:28 |
| davecheney | status: error | 01:28 |
| davecheney | status-info: 'hook failed: "install"' | 01:28 |
| davecheney | + grep -v -q error | 01:28 |
| davecheney | ^ not spotting the problem | 01:28 |
| * davecheney steps out for lunch | 01:37 | |
| m_3 | davecheney: ack | 02:33 |
| m_3 | davecheney: gonna go get dinner | 02:33 |
| m_3 | davecheney: see you on the flipside | 02:34 |
| thumper | davecheney: review done | 02:57 |
| thumper | davecheney: I think the version tests should move into the cmd package | 02:58 |
| thumper | that's about it | 02:58 |
| davecheney | thumper: thanks mate | 03:18 |
| davecheney | will take another crack now | 03:18 |
| * thumper is done for today | 04:10 | |
| thumper | caio | 04:10 |
| wallyworld_ | davecheney: hi, i'm half way through fixing the raring tests and i noticed you have the card assigned to you. have you started on it yet? | 05:18 |
| davecheney | nah, grab it | 05:19 |
| wallyworld_ | awesome thans | 05:19 |
| davecheney | 4 | 05:19 |
| wallyworld_ | davecheney: it turns out the git issues can be fixed by setting env vars in setup test. but if we ever deploy on that later version, we will need to ensure the git config is correctly written | 05:20 |
| davecheney | wallyworld_: i could not figure out what was wrong with tims' setup | 05:20 |
| wallyworld_ | also, the later version of git in raring has different error messages, and we are doing exact string matches in the tests | 05:20 |
| davecheney | i tried in a fresh raring vm | 05:20 |
| davecheney | and couldn't replicate the problem | 05:21 |
| davecheney | wallyworld_: ahh, i have a fix for that | 05:21 |
| wallyworld_ | did you have a .gitconfig already? | 05:21 |
| * davecheney searches | 05:21 | |
| davecheney | not the .gitconfig one, i couldn't replicate the problem | 05:21 |
| wallyworld_ | i just changed the tests to use a regexp | 05:21 |
| davecheney | that'll do | 05:21 |
| davecheney | that is what I did too | 05:21 |
| wallyworld_ | i have never used git before, and i can replicate it | 05:21 |
| davecheney | go with your version | 05:21 |
| davecheney | mine didn't work :) | 05:21 |
| wallyworld_ | i think git has a few ways of knowing your email address | 05:21 |
| davecheney | wallyworld_: thing is | 05:22 |
| wallyworld_ | and since tim nor i have any of those, the test sfail | 05:22 |
| davecheney | in a fresh raring vm | 05:22 |
| davecheney | ther ewas not problem | 05:22 |
| wallyworld_ | hmmm, ok | 05:22 |
| davecheney | i cannot explain what thumper did to his machine | 05:22 |
| davecheney | and so was unable to fix it | 05:22 |
| wallyworld_ | i did a dist upgrade from quantal | 05:22 |
| wallyworld_ | anyways, adding set envs in the test setup fixes it | 05:22 |
| davecheney | brave man | 05:23 |
| wallyworld_ | i like to live on the edge | 05:23 |
| dimitern | morning all | 08:01 |
| fwereade | dimitern, mgz, jtv1: mornings | 08:45 |
| dimitern | fwereade: hey | 08:46 |
| fwereade | dimitern, thanks for the 1152717 review, I've loosened it up a little with a more detailed explanation; about to repropose, would be grateful if you would think of ways to break it ;p | 08:46 |
| jtv1 | Hi fwereade | 08:46 |
| === jtv1 is now known as jtv | ||
| fwereade | dimitern, how's the upgrade-charm stuff going? | 08:47 |
| dimitern | fwereade: will do :) | 08:47 |
| * jtv keeps misreading that nick as "dim intern" :( | 08:47 | |
| fwereade | haha | 08:47 |
| dimitern | fwereade: i'll finish the dreaded 010-upgade-charm-... branch today I hope - still fiddling with tests | 08:48 |
| dimitern | jtv: lol we're the first definitely | 08:48 |
| dimitern | jtv: s/we/you/ | 08:48 |
| jtv | I find that hard to believe! | 08:48 |
| jtv | Maybe I'm just the first who dared to say it. | 08:48 |
| dimitern | :D | 08:48 |
| fwereade | dimitern, cool -- if the morning goes well I might be free to pair after lunch, if that would be useful to you | 08:49 |
| * jtv may be going just a little bit dyslexic. | 08:49 | |
| dimitern | fwereade: that will be best and fastest actually | 08:50 |
| fwereade | dimitern, ok, great | 08:50 |
| dimitern | fwereade: although it's still a bit of a mess at home, so maybe I can come to yours? | 08:50 |
| fwereade | dimitern, surely, cath and laura will be out most of the afternoon I think | 08:51 |
| dimitern | fwereade: sweet, just give me a shout then - we can probably have lunch @cuba or something as well | 08:52 |
| mgz | ...seems like a long way to go for lunch... | 08:53 |
| fwereade | dimitern, when's your standup again? | 08:53 |
| fwereade | mgz, ;p | 08:53 |
| dimitern | fwereade: 12:30 | 08:53 |
| fwereade | dimitern, ok, lunch at 1:30 then? | 08:53 |
| dimitern | mgz: oh, it's maltaspace you know - everything it's but a wormhole away :D | 08:53 |
| dimitern | fwereade: sgtm | 08:53 |
| fwereade | dimitern, reproposed https://codereview.appspot.com/7591044 | 08:56 |
| dimitern | fwereade: already looking | 08:56 |
| fwereade | <3 | 08:56 |
| dimitern | jtv: sorry about my bitching about using lbox propose - i didn't realise you forked the maas provider and you're working on it separately | 08:57 |
| dimitern | fwereade: so if we have the same situation s.doc.UnitCount ==1 in local (stale) state, then $gt will fail and we'll retry | 09:02 |
| fwereade | dimitern, yep | 09:02 |
| dimitern | fwereade: looks ok to me | 09:02 |
| fwereade | dimitern, cool | 09:02 |
| rogpeppe | mornin' all | 09:15 |
| jam | so, does anyone know what writes "/var/lib/juju/agents/bootstrap/agent.conf" ? I'm playing with the Windows port, and the bootstrap node fails to find that file. | 09:19 |
| jam | I'm guessing it is a bootstrap => cloud-init issue. | 09:20 |
| jam | fwereade: as part of the "pass series into environ", is Arch on the table for poking at? As it seems to only use the juju client Arch (amd64/i386) but I'm not sure that actually makes sense. | 09:23 |
| rogpeppe | jam: yes, the cloudinit code should write that file | 09:23 |
| jam | (we only have 64-bit tools available, so why not start a 64-bit instance always) | 09:23 |
| fwereade | jam, arch will be incorporated alongside constraints | 09:23 |
| rogpeppe | fwereade: can i presume that https://codereview.appspot.com/7809043 was instigated by you? | 09:23 |
| fwereade | rogpeppe, well, we kinda agreed the business value was insufficient to make it a focus now, but thumper only remembered that when he'd done it | 09:24 |
| rogpeppe | fwereade: the problem is that it means that it's a right pain if you want to bootstrap from Go code now | 09:25 |
| rogpeppe | fwereade: for instance it breaks builddb | 09:25 |
| fwereade | rogpeppe, and I'm +1 on little steps that make environs clearer and less weirdly coupled, so I think it's still a good move | 09:25 |
| rogpeppe | fwereade: that was the main consideration behind the current design | 09:25 |
| fwereade | rogpeppe, hmm | 09:25 |
| fwereade | rogpeppe, builddb can't create a cert independently? | 09:26 |
| rogpeppe | fwereade: it *could*, but then it would be duplicating the code in cmd/juju/bootstrap | 09:26 |
| rogpeppe | fwereade: if nothing else, the code to generate certs appropriately for an environment should be factored out into environs, so it's at least available to other Go code that wants to bootstrap. | 09:27 |
| rogpeppe | fwereade: FWIW i hammered this out for ages with gustavo | 09:28 |
| fwereade | rogpeppe, I agree the cert generation should not be inside cmd/juju, but I think it's the appropriate place to invoke it | 09:29 |
| jam | rogpeppe: so the environs.agent code appears to use "path.Join()" to do these sorts of operations, which means it is trying to shell out to stuff like "bootstrap\agents.conf" rather than "bootstrap/agents.conf" | 09:30 |
| fwereade | rogpeppe, so keeping the functionality in environs sgtm, but I remain -1 on keeping it *inside* environs.Bootstrap | 09:30 |
| rogpeppe | fwereade: yeah, i'm not intrinsically opposed to needing two steps to bootstrap (i think that was my favoured choice originally) | 09:30 |
| jam | so.... sigh, lots of code that would act differently on Windows for bad reasons. | 09:30 |
| rogpeppe | jam: you mean filepath.Join ? | 09:30 |
| jtv | dimitern: I guessed. :) We don't think of it as forking, really, more as a feature branch. Eventually we'll submit the whole thing for review. (We'll probably chop it into smaller pieces for easier reviewing) | 09:30 |
| jam | rogpeppe: well explicitly path.Join | 09:31 |
| jam | maybe that one is ok? | 09:31 |
| fwereade | rogpeppe, incidentally, do you recall why builddb lacks tests? | 09:31 |
| rogpeppe | jam: path.Join should never produce \ | 09:31 |
| rogpeppe | fwereade: because it's a quick hack command that gustavo wrote | 09:31 |
| dimitern | jtv: :) I see | 09:31 |
| fwereade | rogpeppe, that's an important part of our infrastructure?part of our | 09:32 |
| * fwereade sighs gently | 09:32 | |
| rogpeppe | jam: i was originally trying to be very careful about path hygiene (path vs filepath) but got told that it wasn't worth it - we'd need another pass later, so, yes, everything will break under windows. | 09:32 |
| rogpeppe | fwereade: it is? | 09:32 |
| rogpeppe | fwereade: it just compiles mongo. | 09:32 |
| rogpeppe | fwereade: tbh, it could be a regular charm. | 09:32 |
| fwereade | rogpeppe, doesn't that qualify? | 09:32 |
| fwereade | rogpeppe, true | 09:33 |
| rogpeppe | fwereade: the only real test is running it. there's no input and only one possible output. | 09:33 |
| rogpeppe | jam: your best bet is to grep for all occurrences of (path|filepath)\.Join and inspect on a case-by-case basis. | 09:34 |
| fwereade | rogpeppe, weeeell... that'd be a test for the charm, right? | 09:34 |
| jam | rogpeppe: well as a start, I need to figure out if this is exactly why it is failing | 09:34 |
| rogpeppe | fwereade: yup. but it's not in the charm store. when would that test ever have run? | 09:35 |
| rogpeppe | jam: and it's quite an expensive test (n hours on an ec2 instance) | 09:35 |
| jam | rogpeppe: I think you meant fwereade :) | 09:35 |
| rogpeppe | jam: i did :- | 09:35 |
| rogpeppe | ) | 09:35 |
| fwereade | rogpeppe, surely there are plenty of things that could be tested without actually running the charm? eg that the code compiles, that the charm is actually a valid charm, etc? | 09:35 |
| rogpeppe | fwereade: possibly. i think there are more important considerations. if that command fails, it can be dealt with elsewhere. | 09:36 |
| rogpeppe | jam: what failure are you currently seeing? | 09:37 |
| fwereade | rogpeppe, if it's not important enough to test it's probably not important enough to exist then? | 09:38 |
| rogpeppe | fwereade: probably | 09:38 |
| rogpeppe | fwereade: i think gustavo wrote it as more of a proof of concept than anything else. | 09:39 |
| jam | rogpeppe: looks like agent.tools is ok, but trivial.ShQuote is not | 09:39 |
| jam | > '\var\lib\juju\agents\machine-0\agent.conf' | 09:40 |
| jam | not sure what file that will actually create :) | 09:40 |
| fwereade | rogpeppe, if you're reviewing that branch, maybe tell thumper to correspond with davecheney and figure out if it's ok to drop it? | 09:40 |
| rogpeppe | fwereade: ok | 09:40 |
| fwereade | rogpeppe, ideally mongossl ends up in cloudarchive anyway | 09:40 |
| jam | nm, not ShQuote, but c.Dir() | 09:41 |
| rogpeppe | jam: that's not a ShQuote issue. | 09:41 |
| jam | maybe?? | 09:41 |
| jam | rogpeppe: that is using path.Join() as near as I can tell | 09:41 |
| rogpeppe | jam: | 09:44 |
| rogpeppe | func (c *Conf) File(name string) string { | 09:44 |
| rogpeppe | return filepath.Join(c.Dir(), name) | 09:44 |
| rogpeppe | } | 09:44 |
| rogpeppe | jam: to be honest, i think if we changed all filepath imports to use path, everything would probably work | 09:45 |
| jam | rogpeppe: I haven't found a 'filepath' yet | 09:46 |
| jam | I can see that c.Dir() is returning '/var/lib'... | 09:46 |
| rogpeppe | jam: in environs/agent/agent.go | 09:46 |
| jam | but by the time it gets put together into WriteCommands it is \var... | 09:46 |
| jam | right, I guess that code is mixing "where will I read" with "where will I write" and not paying attention to the fact that "where will I write" is running on another OS | 09:47 |
| jam | but as you say, / works on Windows anyway, and for now, the 'where will I read' is always *nix anyway | 09:48 |
| jam | well prob always Ubuntu even | 09:48 |
| rogpeppe | jam: for the time being, yes | 09:49 |
| jam | rogpeppe: so changing that looks good so far, I have to wait for the instance to start up | 09:49 |
| dimitern | how can I get the caller of a function at run time? | 09:55 |
| jam | rogpeppe: and step 2, mongo doesn't start because the upstart file that gets written was using 'filepath' as well. :) | 09:55 |
| rogpeppe | jam: just change "filepath" to "path" throughout the code | 09:56 |
| dimitern | rogpeppe: ^^ ? | 09:57 |
| jam | yeah, that's what I'm thinking as well | 09:57 |
| rogpeppe | jam: there will be one or two places where it uses filepath.Walk, and you'll have to fix those, but i think everything else should probably just work | 09:57 |
| jam | dimitern: we used that in the Hooks code for goose | 09:57 |
| rogpeppe | dimitern: ? | 09:57 |
| jam | dimitern: http://golang.org/pkg/runtime/ | 09:57 |
| jam | runtime.Caller() | 09:57 |
| dimitern | jam: oh, cool 10x | 09:57 |
| rogpeppe | ah | 09:57 |
| rogpeppe | dimitern: i use a little helper function that gives me the source locations of all callers, all formatted on a single line | 09:58 |
| dimitern | rogpeppe: great! can I have this pls? | 09:58 |
| rogpeppe | dimitern: yeah, one mo | 09:58 |
| dimitern | I'm getting an obscure panic and trying to see which call caused it | 09:59 |
| rogpeppe | dimitern: import "code.google.com/p/rog-go/exp/runtime/debug" | 10:00 |
| rogpeppe | // Callers returns the stack trace of the goroutine that called it, | 10:00 |
| rogpeppe | // starting n entries above the caller of Callers, as a space-separated list | 10:00 |
| rogpeppe | // of filename:line-number pairs with no new lines. | 10:00 |
| rogpeppe | func Callers(n, max int) []byte { | 10:00 |
| dimitern | rogpeppe: tyvm | 10:00 |
| rogpeppe | dimitern: BTW doesn't the panic give you a stack trace? | 10:01 |
| dimitern | rogpeppe: it does, but it's in another goroutine and it's not helpful | 10:01 |
| rogpeppe | dimitern: ah yes | 10:01 |
| rogpeppe | dimitern: don't you get *all* stack traces? | 10:01 |
| dimitern | rogpeppe: I do but it's still confusing - there are like 80 goroutines | 10:02 |
| rogpeppe | dimitern: what i tend to do in that case is search for functions i'm interested in | 10:03 |
| rogpeppe | dimitern: in acme (you can probably do something similar in your editor) i do ,x/(.+\n)*/v/function-i'm-interested-in/d | 10:04 |
| rogpeppe | dimitern: which removes all stack traces that don't mention the given function name | 10:04 |
| dimitern | rogpeppe: aha, I can try this | 10:04 |
| dimitern | rogpeppe: actually it's still weird - it's something to do with the watchers - wanna take a look? | 10:06 |
| rogpeppe | dimitern: sure - paste away. is this against trunk? | 10:06 |
| dimitern | rogpeppe: no, against the branch I'm working on, but I merge trunk regularly (just did this morning) - http://paste.ubuntu.com/5613199/ | 10:07 |
| rogpeppe | dimitern: can you reproduce the problem in trunk? | 10:08 |
| dimitern | rogpeppe: no | 10:08 |
| dimitern | rogpeppe: but then again the tests in the uniter/filter will be different | 10:08 |
| dimitern | anyway I still don't get why "watcher was stopped cleanly" is a panic? | 10:09 |
| rogpeppe | dimitern: if you get eof from a watcher, it should be because it stopped because of an error. | 10:10 |
| dimitern | rogpeppe: istm there's no way to stop a watcher without a panic | 10:10 |
| rogpeppe | dimitern: the only time it there's no error is when the watcher was stopped deliberately | 10:10 |
| dimitern | rogpeppe: that is, if you then call musterr | 10:11 |
| dimitern | and I still don't get how this the the only place where err == nil is a panic | 10:11 |
| rogpeppe | dimitern: musterr is used when we know that the watcher is never stopped. | 10:11 |
| rogpeppe | dimitern: can you push your branch? | 10:12 |
| dimitern | rogpeppe: ok | 10:12 |
| dimitern | rogpeppe: lp:~dimitern/juju-core/010-uniter-handle-config-upgrades | 10:13 |
| rogpeppe | dimitern: from the look of those stack traces, *loads* of watchers have been stopped unexpectedly. | 10:14 |
| rogpeppe | dimitern: (look at all the calls to MustErr that are in progress) | 10:14 |
| dimitern | rogpeppe: I saw that, but still no clue what i did wrong | 10:14 |
| rogpeppe | dimitern: looks like you might not be cleaning up properly or something | 10:14 |
| rogpeppe | dimitern: which test fails? | 10:15 |
| dimitern | rogpeppe: not, sure let me run it with -vv | 10:15 |
| dimitern | rogpeppe: START: context.go:0: FilterSuite.TearDownTest, just after that is the panic | 10:16 |
| rogpeppe | dimitern: ah yes, i just worked that out | 10:16 |
| rogpeppe | dimitern: ah, i see the problem | 10:17 |
| rogpeppe | dimitern: you're not deferring f.Stop | 10:18 |
| dimitern | rogpeppe: I can't see the difference, since it's near the end anyway | 10:18 |
| dimitern | rogpeppe: no, what - which file/line? | 10:19 |
| rogpeppe | dimitern: well, it fixes your panic | 10:19 |
| dimitern | rogpeppe: filter_test.go:300 ? | 10:20 |
| rogpeppe | dimitern: no :243 | 10:20 |
| rogpeppe | dimitern: just after newFilter, in TestConfigEvents | 10:20 |
| dimitern | rogpeppe: I'm not, but I'm calling f.Stop() explicitly later | 10:21 |
| rogpeppe | dimitern: yes, but you're getting a test failure which means it never gets called | 10:21 |
| dimitern | rogpeppe: right! | 10:21 |
| dimitern | rogpeppe: I was thinking something like that | 10:21 |
| dimitern | rogpeppe: anyway, 10x :) | 10:21 |
| rogpeppe | dimitern: np. BTW, i'm not entirely sure why we get the error we do (i'd have thought that we'd get an error when the state is shut down underneath us) | 10:22 |
| dimitern | rogpeppe: yeah, the fix is easy, but understanding the error is hard :) | 10:23 |
| rogpeppe | dimitern: feel free to dig in and improve it. you'll get a better understanding of what's going on too if you do :-) | 10:24 |
| dimitern | rogpeppe: once I get it I will :) | 10:25 |
| rogpeppe | dimitern: perhaps try to write a little piece of code that reproduces the problem | 10:25 |
| dimitern | rogpeppe: yeah, i have something in mind already | 10:27 |
| dimitern | fwereade: can you take a look please - https://codereview.appspot.com/7425044 - the tests now pass, but I'm surely missing something? | 10:30 |
| fwereade | dimitern, looking | 10:31 |
| fwereade | dimitern, looking pretty good, sent some more comments -- should be able to polish it off today | 10:54 |
| dimitern | fwereade: great to hear! | 10:54 |
| dimitern | mgz: standup? | 11:31 |
| mgz | ta | 11:36 |
| rogpeppe | system hung up on me | 12:05 |
| rogpeppe | fwereade: any chance you could have a look at these CLs? https://codereview.appspot.com/7727044/ https://codereview.appspot.com/7727045/ | 12:06 |
| rogpeppe | (or anyone else, for that matter (thanks already dimitern!)) | 12:08 |
| dimitern | rogpeppe: if you stop the watcher, won't you get this when trying to read on the channel? | 12:08 |
| rogpeppe | dimitern: get what? | 12:09 |
| dimitern | rogpeppe: https://codereview.appspot.com/7727045/diff/5001/state/megawatcher.go#newcode105 | 12:09 |
| dimitern | rogpeppe: detect the watcher was stopped | 12:09 |
| rogpeppe | dimitern: sorry, i'm not sure i understand the question | 12:09 |
| dimitern | rogpeppe: I mean having reply chan struct{} instead | 12:10 |
| fwereade | rogpeppe, looking | 12:10 |
| dimitern | rogpeppe: you mean having a bool instead gives you the possibility to send 2 distinct msgs: watcher closed and ? | 12:11 |
| rogpeppe | dimitern: the allWatcher replies with either true (request accepted) or false (the StateWatcher has been stopped) | 12:11 |
| dimitern | rogpeppe: ah, I see | 12:12 |
| rogpeppe | dimitern: actually true means "request has been processed, and now contains the reply" | 12:12 |
| dimitern | rogpeppe: thanks for clarifying this | 12:12 |
| rogpeppe | dimitern: np. i'll try to clarify the comments appropriately. | 12:12 |
| rogpeppe | fwereade: thanks | 12:12 |
| fwereade | rogpeppe, sorry I didn't finish those yesterday -- both LGTM | 12:22 |
| rogpeppe | fwereade: cool, thanks | 12:22 |
| * dimitern lunch | 12:23 | |
| fwereade | rogpeppe, if you have a mo I have https://codereview.appspot.com/7591044/ and https://codereview.appspot.com/7715046/ out for review | 12:26 |
| * fwereade lunch | 12:26 | |
| rogpeppe | fwereade: ok, thanks, will have a look in a bit | 12:26 |
| === benji___ is now known as benji | ||
| benji | When you guys get a chance I have a small branch up for review: https://codereview.appspot.com/7554046/ | 12:54 |
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | fwereade, mramm: hangout? | 14:02 |
| mramm | rogpeppe: sorry, did not wake up for the alarm | 14:16 |
| fwereade | mramm, we're pretty much done actually | 14:17 |
| rogpeppe | mramm: we're just about done, but still there if you want to join | 14:17 |
| mramm | seems like you finished before I got there... :) | 14:21 |
| === teknico_ is now known as teknico | ||
| fwereade | rogpeppe, https://codereview.appspot.com/7755045 might be trivial -- it's certainly small | 14:37 |
| rogpeppe | fwereade: not trivial, i think - i'm having to think a bit | 14:39 |
| fwereade | rogpeppe, ok, definitely not a priority then, I'll leve it lying around unreferenced in kanban and just drop it in a few days unless someone really likes it | 14:40 |
| rogpeppe | fwereade: i *think* it's right, but there might have been a good reason i didn't do that before (rather than just not thinking of it) | 14:41 |
| fwereade | rogpeppe, one thing that crosses my mind is that the highest-version behaviour in environs is incompatible -- the highest version (as deployed) will (surely?) be immediately downgraded to agent-version once an agent starts running | 14:42 |
| rogpeppe | fwereade: i don't think so | 14:44 |
| rogpeppe | fwereade: note line 177 | 14:44 |
| fwereade | rogpeppe, not following -- `binary.Number = vers`? | 14:45 |
| rogpeppe | fwereade: yeah | 14:45 |
| fwereade | rogpeppe, I'm probably being thick, please explain | 14:45 |
| rogpeppe | fwereade: ah, i might have been misreading your remark | 14:46 |
| fwereade | rogpeppe, ISTM that environs gets the highest compatible tools and deploys with those | 14:46 |
| fwereade | rogpeppe, but that the agent that then runs checks for the agent-version in env config | 14:46 |
| rogpeppe | fwereade: i can't remember where agent-version gets set originally. | 14:47 |
| fwereade | rogpeppe, and up/down/whatever-grades to that immediately | 14:47 |
| fwereade | rogpeppe, hmm, good question | 14:47 |
| rogpeppe | fwereade: in general, if agent version is set, we want to use that version. | 14:47 |
| fwereade | rogpeppe, tools we bootstrp with, or defaults to version.Current | 14:48 |
| fwereade | rogpeppe, I think :) | 14:48 |
| rogpeppe | fwereade: agent-version is important - that's how we have any control over what version the agents are running | 14:49 |
| fwereade | rogpeppe, indeed | 14:49 |
| fwereade | rogpeppe, I think environs should be a bit more careful with it | 14:50 |
| rogpeppe | fwereade: i'm thinking that the tools selection logic ... yeah | 14:50 |
| fwereade | rogpeppe, but I think thumper's heading in that direction anyway | 14:50 |
| rogpeppe | fwereade: AgentVersion defaults to the current version, BTW | 14:52 |
| fwereade | rogpeppe, I think I said that | 14:53 |
| * rogpeppe continues to study the logic | 14:59 | |
| rogpeppe | fwereade: i *think* the provisioner should pass the current agent version into StartInstance | 15:06 |
| rogpeppe | fwereade: (optionally) | 15:06 |
| fwereade | rogpeppe, +-1, I'm not sure any of that stuff is the provisioner's business | 15:07 |
| fwereade | rogpeppe, if anything should know the env config it's the env ;) | 15:07 |
| rogpeppe | fwereade: hmm, good point, the environ should already *know* the agent version! | 15:07 |
| rogpeppe | fwereade: but currently it can't tell if it's been set explicitly | 15:08 |
| rogpeppe | fwereade: because i think that only if it's not set explicitly should the environ pass HighestVersion in the FindTools flags | 15:09 |
| === teknico_ is now known as teknico | ||
| rogpeppe | fwereade: last thing you saw? | 15:29 |
| fwereade | rogpeppe, I said "although I'm now less inclined to take a hard line on that -- if the provisioner were to be running a locatoralike, that would probably be best" | 15:30 |
| rogpeppe | fwereade: ah, last thing i saw was: | 15:30 |
| rogpeppe | [15:07:15] <fwereade> rogpeppe, if anything should know the env config it's the env ;) | 15:30 |
| rogpeppe | fwereade: last thing you saw from me? | 15:30 |
| fwereade | rogpeppe, in between the two I said " just like the state/pi info we pass in" | 15:30 |
| fwereade | rogpeppe, nothing from you since "(optionally)" | 15:31 |
| rogpeppe | [15:07:50] <rogpeppe> fwereade: hmm, good point, the environ should already *know* the agent version! | 15:31 |
| rogpeppe | [15:08:05] --> teknico_ has joined this channel (~quassel@93-42-34-107.ip84.fastwebnet.it). | 15:31 |
| rogpeppe | [15:08:24] <rogpeppe> fwereade: but currently it can't tell if it's been set explicitly | 15:31 |
| rogpeppe | [15:09:19] <rogpeppe> fwereade: because i think that only if it's not set explicitly should the environ pass HighestVersion in the FindTools flags | 15:31 |
| fwereade | rogpeppe, hmm, yeah, maybe | 15:32 |
| === teknico_ is now known as teknico | ||
| fwereade | rogpeppe, I'm not totally wild about the "highest" behaviour because of the conflict with what the agent upgrader does | 15:32 |
| fwereade | rogpeppe, "highest" STM like a good default for what to set it to when we do upgrde-juju | 15:32 |
| rogpeppe | fwereade: i think highest is fine when deploying without a specified version | 15:32 |
| rogpeppe | fwereade: if the agent version is not set, that is | 15:34 |
| fwereade | rogpeppe, hmm, ok, if there's nothing in agent-version at *all*, that could make sense | 15:34 |
| fwereade | rogpeppe, but is there ever such a situation? | 15:34 |
| rogpeppe | fwereade: that is almost always the case currently | 15:34 |
| fwereade | rogpeppe, I think we set one up at bootstrp time if none is set | 15:34 |
| rogpeppe | fwereade: i think | 15:34 |
| fwereade | rogpeppe, environs/config.go:179 | 15:35 |
| fwereade | rogpeppe, seems strange not to aim to have everything on the same version in general | 15:35 |
| rogpeppe | fwereade: yeah, i'm not sure about that. | 15:35 |
| rogpeppe | fwereade: the main case for not doing so is when bootstrapping | 15:35 |
| fwereade | rogpeppe, a random smorgasbord of versions, until one is set explicitly, seems more likely to confuse and upset than nything else | 15:36 |
| fwereade | rogpeppe, I'd be fine with bootstrapping to the highest available when not otherwise set, I think | 15:36 |
| rogpeppe | fwereade: i think if you bootstrap, you should get the latest version, regardless of the client | 15:36 |
| fwereade | rogpeppe, ok, sgtm | 15:36 |
| rogpeppe | fwereade: so this brings us back to Bootstrap needing to know if the agent version is explicitly set or not | 15:37 |
| rogpeppe | fwereade: i *think* i'd be ok if config.Config never looked at version.Current. then all agent-version setting is done explicitly where necessary. | 15:39 |
| rogpeppe | fwereade: and then perhaps upgrader would just do nothing if agent-version was not set | 15:40 |
| rogpeppe | fwereade: although that should never happen in practice. | 15:41 |
| rogpeppe | fwereade: gone again? | 15:44 |
| fwereade | rogpeppe, sorry, only half looking t my own screen | 15:48 |
| rogpeppe | fwereade: np | 15:49 |
| fwereade | rogpeppe, yeah, I think I'm ok with that | 15:49 |
| fwereade | rogpeppe, I agree that the upgrader should never see a missing agent-version | 15:49 |
| rogpeppe | fwereade: BTW i'd like to know your thoughts on https://codereview.appspot.com/7554046/diff/1/state/apiserver/api_test.go#newcode404 | 15:49 |
| * fwereade looks | 15:50 | |
| fwereade | rogpeppe, heh, tricky | 15:52 |
| fwereade | rogpeppe, no, wait, we have a state.State | 15:52 |
| fwereade | rogpeppe, we can implement the test by adding via state and trying to remove via api | 15:52 |
| fwereade | rogpeppe, yesno? | 15:52 |
| fwereade | rogpeppe, I'm -1 on dropping those tests, I think they will become very meaningful again when we start doing the internal API | 15:53 |
| rogpeppe | fwereade: i don't want to drop all those tests, just the Client ones. | 15:54 |
| rogpeppe | fwereade: and if we ever had any other logic behind them (say different users could do different things), we'd need more again. | 15:54 |
| rogpeppe | fwereade: i'd drop all the Client tests except one | 15:55 |
| fwereade | rogpeppe, I think there's still value in checking that all the client methods work the same | 15:57 |
| rogpeppe | fwereade: ok. but we really are just testing that single "if" statement in every one of them, because they're all gated through that method. | 15:58 |
| fwereade | rogpeppe, even if it's obvious from the implementation that they do, that only applies today and not in the face of unknowable future changes | 15:58 |
| rogpeppe | fwereade: if those changes come, we'll be needing to change the tests anyway. | 15:58 |
| fwereade | rogpeppe, I have a great fondness for the refactor-with-axe, run-tests, see-what-failed approach | 15:58 |
| fwereade | rogpeppe, if we keep those tests then when I do that I can at least see that the bits of implementation that are important enough to care about still act as they did before | 15:59 |
| rogpeppe | fwereade: FWIW dropping all but one of those tests shaves 8s off the api test runtime | 16:02 |
| fwereade | rogpeppe, that is to me an argument for making them faster, not for just dropping them ;p | 16:02 |
| rogpeppe | fwereade: do you see any way we can make them faster? | 16:02 |
| rogpeppe | fwereade: fundamentally we are pretty slow at mutating state | 16:03 |
| rogpeppe | fwereade: perhaps there's a magic switch we haven't given to mongod | 16:04 |
| fwereade | rogpeppe, tbh, no, nothing that isn't deeply hackish | 16:04 |
| fwereade | rogpeppe, (magically poke everything into state at once!) | 16:04 |
| rogpeppe | fwereade: i don't think that'll help here. | 16:04 |
| rogpeppe | fwereade: we're not spending the time in setUpScenario | 16:05 |
| rogpeppe | fwereade: (even if we could do that, of course) | 16:05 |
| fwereade | rogpeppe, ah, ok, I thought that was where you'd had problems in the past | 16:05 |
| fwereade | rogpeppe, I bet we *could*, but I'm petty sure it'd be a bad idea | 16:05 |
| rogpeppe | fwereade: yeah, that's why i took it out of the loop and made functions return an "undo" function | 16:05 |
| rogpeppe | fwereade: if we were doing setUpScenario, the tests would take ages and ages to run | 16:06 |
| fwereade | rogpeppe, if the problem is the operations themselves then I don't see a way out then | 16:06 |
| rogpeppe | fwereade: indeed. that's why i think that removing most of the tests which aren't actually testing any existing logic is a reasonable way forward. | 16:07 |
| fwereade | rogpeppe, I dunno, I've been bitten before by changing stuff and assuming that the lack of test failures indicated smart tests and a good change | 16:08 |
| fwereade | rogpeppe, when in fact it was just that nobody had written them in the first place | 16:08 |
| rogpeppe | fwereade: to change this behaviour, we'd have to change the Client method - if we put a comment there, i think we'd be ok. | 16:09 |
| rogpeppe | fwereade: BTW, if we were running setUpScenario each time, it would add about 1 | 16:13 |
| rogpeppe | 3 minutes to the test time | 16:13 |
| rogpeppe | (that's 3, not 13) | 16:13 |
| fwereade | rogpeppe, ouch :/ | 16:14 |
| fwereade | rogpeppe, still, if we can't test authorization behaviour directly I think we have no choice but to do so indirectly | 16:15 |
| fwereade | rogpeppe, and suck up the costs | 16:16 |
| rogpeppe | fwereade: actually, we *could* theoretically provide an entry point into rpc that allowed testing access to a particular rpc object without actually calling a method on that object. | 16:16 |
| rogpeppe | fwereade: i'd much prefer it if we could make operations on the state faster though. | 16:17 |
| rogpeppe | fwereade: so we wouldn't be worrying about this. | 16:18 |
| fwereade | rogpeppe, +1 in at least theory, but I'm hoping to hear something about things that hurt real-world performance from dave before I fret too much in that direction | 16:19 |
| rogpeppe | fwereade: i guess so. it might takes 15 minutes just to add 10000 units to state, it's probably not that huge a deal | 16:20 |
| rogpeppe | s/takes/take/ | 16:20 |
| rogpeppe | s/it's/but it's/ | 16:20 |
| fwereade | rogpeppe, yeah, we shall see the actual numbers | 16:21 |
| rogpeppe | fwereade: well, you can get a best-case scenario by simply calling AddUnit 10000 times on a local state server. | 16:21 |
| rogpeppe | fwereade: i suspect it'll be about 10-15 minutes to do that | 16:22 |
| fwereade | rogpeppe, I'm about to head off -- if you have a chance to take a look at https://codereview.appspot.com/7715046/ and/or https://codereview.appspot.com/7591044/ I would be most grateful | 16:58 |
| rogpeppe | fwereade: ok, will do | 16:58 |
| fwereade | rogpeppe, cheers | 16:59 |
| fwereade | gn all | 16:59 |
| rogpeppe | right, that's me for the day. good night. | 19:37 |
| === _thumper_ is now known as thumper | ||
| thumper | morning | 21:10 |
| hatch | morning thumper | 21:11 |
| m_3 | thumper: hey... what's the best way to fix `bzr info lp:charms/juju-gui` in-place (without deleting and re-pushing any branches)? | 21:42 |
| thumper | m_3: what do you mean byfix? | 21:43 |
| m_3 | thumper: well the stacking seems screwed up | 21:43 |
| * thumper takes a look | 21:43 | |
| m_3 | thumper: and we discussed how to _prevent_ such a thing in the future | 21:43 |
| m_3 | but I don't think I ever asked how to fix it after the fact | 21:44 |
| thumper | are people creating stacked branches locally ? like on their own machines? | 21:44 |
| thumper | because that way lies insantiy | 21:44 |
| m_3 | thumper: dunno, I think this is done when people shortcut the promulgation process we discussed last week | 21:44 |
| m_3 | i.e., to promulgate, they pull the user branch like lp:~juju-gui/charms/precise/juju-gui/trunk | 21:45 |
| thumper | m_3: you have a copy of it locally? | 21:46 |
| m_3 | and then push that same branch up to lp:~charmers/charms/precise/juju-gui/trunk without first creating this repository as a non-stacked branch | 21:46 |
| m_3 | yes, I have it locally now | 21:46 |
| m_3 | (just merged an MP) | 21:46 |
| m_3 | the cleanest way I know of to fix the stacking is to pull a fresh lp:charms/juju-gui, then delete the lp:~charmers owned branch... then push it back up to lp:charms/juju-gui | 21:47 |
| m_3 | but that feels heavy-handed, and probably just dumb | 21:47 |
| thumper | m_3: I'm looking... | 21:49 |
| thumper | m_3: even if I find a way, it is not likely to be easy... | 21:49 |
| m_3 | thumper: I mainly wanted to check if there was a nice `bzr reconfigure --unstack lp:charms/juju-gui` that should work on the lp repo | 21:49 |
| thumper | well... | 21:49 |
| m_3 | but I've only seen that work locally | 21:49 |
| thumper | kinda, but it breaks if the branch is broken | 21:49 |
| thumper | although I have an idea | 21:49 |
| m_3 | ack | 21:49 |
| m_3 | I think we have like a dozen or so broken ones... I can count | 21:49 |
| thumper | m_3: try the reconfigure --unstacked | 21:51 |
| thumper | m_3: it may be that just the stacked on url is buggered | 21:51 |
| * thumper no longer has write access to every branch on LP | 21:51 | |
| thumper | gave that up ages ago | 21:52 |
| m_3 | nope, same... http://pastebin.ubuntu.com/5614925/ | 21:52 |
| m_3 | most of the others could probably be brute-force fixed... this one'll probably be more difficult b/c of stuff brached _from_ this | 21:54 |
| m_3 | dunno tho | 21:54 |
| m_3 | thumper: well nothing urgent... it's not killing anything afaik... just ugly | 21:54 |
| m_3 | and we'll hopefully stop making them this way soon | 21:55 |
| thumper | davecheney: what is the builddb command? | 22:51 |
| thumper | davecheney: rogpeppe suggested to remove the bootstrap from it, but I have no idea what the command is supposed to be | 22:51 |
| davecheney | builddb was a command that wrapped a charm that built the mongodb that we use in juju | 22:55 |
| davecheney | once we have mongo in a package | 22:55 |
| davecheney | we can remove it | 22:55 |
| davecheney | s/was/is | 22:56 |
| thumper | hmm... ok | 22:56 |
| thumper | davecheney: do you agree that builddb shouldn't do a bootstrap? | 22:57 |
| thumper | I'm removing the env cert gen from the environs.Bootstrap function | 22:57 |
| thumper | the other place this is called is from the builddb command | 22:58 |
| thumper | we have two choices... | 22:58 |
| thumper | put the cert gen in there too | 22:58 |
| thumper | or as rogpeppe suggests, remove the bootstrap from the builddb command | 22:58 |
| davecheney | do the second | 22:58 |
| thumper | ok | 22:58 |
| thumper | simple enough | 22:58 |
| davecheney | builddb bootstraps an environment | 22:59 |
| * thumper will have the branch tweaked and ready for a rereview soon | 22:59 | |
| thumper | davecheney: here is some leankit mojo for you | 22:59 |
| davecheney | we can live without that feature if it spawns a charm in an existing charm | 22:59 |
| * davecheney listenes | 22:59 | |
| thumper | davecheney: put a link to the codereview on the card | 22:59 |
| davecheney | using the advanced tab ? | 22:59 |
| thumper | davecheney: see my card for the cert gen | 22:59 |
| thumper | davecheney: yeah | 22:59 |
| thumper | so I added one for "review" with the url | 23:00 |
| davecheney | ahh nice, i once tried using hte general 'link' field | 23:00 |
| thumper | so anyone can go from the board, seeing that there are reviews needed, and go right to the review using the right click, "link to" entry | 23:00 |
| davecheney | but it was unsatisfying | 23:00 |
| davecheney | oh thank fuck | 23:00 |
| thumper | we should tell everyone | 23:01 |
| thumper | it would make it helpful if you are using the board to drive work (which we should) | 23:01 |
| davecheney | i think some folks use the LP review queue | 23:01 |
| davecheney | but that is just historical | 23:01 |
| davecheney | not an indication of best practice | 23:02 |
| thumper | yeah... but everyone should have the board open | 23:02 |
| thumper | I should talk about this more in oakland | 23:02 |
| thumper | I found it very helpful when working with my LP team | 23:02 |
| thumper | not so useful with PS | 23:02 |
| thumper | as there wasn't buy-in from the devs | 23:02 |
| thumper | which was a shame | 23:02 |
| davecheney | what did PS use for plannign ? | 23:04 |
| davecheney | (or do I not want to know) | 23:04 |
| thumper | planning? | 23:05 |
| sidnei | davecheney: does go juju work with canonistack? | 23:05 |
| thumper | you really don't want to know | 23:05 |
| davecheney | sidnei: in theory yes, in practice, no | 23:06 |
| sidnei | LOL | 23:06 |
| thumper | heh | 23:06 |
| sidnei | what's the blocker? | 23:06 |
| davecheney | due to our requirements for public IPs and the lack of said in canonistack | 23:06 |
| davecheney | IPv4 | 23:06 |
| sidnei | really? it needs public ips? | 23:07 |
| davecheney | yes | 23:07 |
| thumper | TODO: IPv6 everywhere | 23:07 |
| sidnei | yesplease | 23:08 |
| davecheney | thumper: do you remember what we decided in atlanta ? | 23:09 |
| davecheney | was it going to be a small or large piece of work to separate the concept of instance and public ip ? | 23:09 |
| thumper | davecheney: for canonistack? | 23:09 |
| thumper | I think jam had a ssh tunnel magic thing that worked | 23:10 |
| davecheney | that was mgz's 'instance addressing card', from memory | 23:10 |
| davecheney | oh yeah, stunnel | 23:10 |
| sidnei | sshuttle i guess | 23:13 |
| sidnei | (eod) | 23:14 |
| davecheney | that's the one | 23:14 |
| davecheney | thumper: are you going to do another propose on https://codereview.appspot.com/7809043/ ? | 23:19 |
| thumper | davecheney: yes | 23:19 |
| thumper | that is what I'm working on now | 23:19 |
| davecheney | ok, will hold off | 23:20 |
| thumper | davecheney: what is a LoggingSuite, and why would I want it for the tests? | 23:22 |
| * davecheney can't remember | 23:23 | |
| thumper | davecheney: another thing | 23:23 |
| thumper | I have two test types I want | 23:24 |
| thumper | one tests internal package functions | 23:24 |
| thumper | the other tests the exported function | 23:24 |
| thumper | so I want two different _test.go files | 23:24 |
| thumper | so... | 23:24 |
| thumper | do we have a standard | 23:24 |
| * thumper tries to remember the defined standard for _test files that magically don't appear in the built files | 23:27 | |
| davecheney | export_test.go ? | 23:27 |
| davecheney | thumper: the Juju practice of testing outside the package, compared to majority of Go code out there is considered unorthadox | 23:28 |
| thumper | what is export_test? | 23:28 |
| davecheney | just a hack to expose private symbols to external test code | 23:28 |
| davecheney | thumper: i overhead you talking about this to william in Atlanta | 23:29 |
| davecheney | it sounded like he didn't have much support for the idea of external testing | 23:29 |
| thumper | was talking with rogpeppe about this too | 23:29 |
| thumper | I think we should have both types | 23:29 |
| thumper | testing only at the export boundary is not so good | 23:29 |
| === wedgwood is now known as wedgwood_away | ||
| thumper | so... | 23:29 |
| davecheney | one suggestoin, not fully considered, $PKG/*_test.go << internal tests | 23:30 |
| thumper | I have added a file environs/cert.go, and I want environs/cert_test.go to be in the environs package and test the internals, and environs/cert_something_test.go to test the public func | 23:30 |
| davecheney | $PKG/test/*_test.go << extenal tests of pkg $PKG | 23:30 |
| thumper | +1 if $PKG/test is $PKG/tests as wthere should be more than one :) | 23:31 |
| davecheney | sure | 23:32 |
| davecheney | it was just a throwaway idea | 23:32 |
| thumper | actually I like it | 23:32 |
| thumper | but I think we should go for general acceptance | 23:32 |
| thumper | do you have any suggestion for just getting this branch in? | 23:32 |
| thumper | I want the external test func in environs_test package | 23:33 |
| thumper | is it all "*_test.go" files that are ignored? | 23:33 |
| davecheney | yes, _test.go is ignored during go build/install | 23:35 |
| thumper | so... | 23:35 |
| thumper | cert_blackbox_test.go? | 23:35 |
| * davecheney reads your branch, i'm not sure what the problem is you are hitting | 23:35 | |
| * thumper does that | 23:35 | |
| thumper | davecheney: you don't see the new changes | 23:35 |
| thumper | let me do this and it will all be obvious :) | 23:36 |
| davecheney | ok | 23:36 |
| thumper | davecheney: there aren't enums in go are there? | 23:46 |
| davecheney | no | 23:46 |
| davecheney | there are consts, and iota defined consts | 23:46 |
| davecheney | but no enums | 23:46 |
| thumper | I want a two value result, but bool doesn't feel right | 23:47 |
| thumper | davecheney: so I have a method called EnsureCertificates which returns error right now | 23:47 |
| thumper | but I want it to also return whether it created the cert | 23:47 |
| thumper | so like (bool, error) | 23:47 |
| thumper | but bool doesn't really have a meaning with Ensure... | 23:47 |
| thumper | what does true mean? | 23:47 |
| thumper | perhaps we just need to put meaning on it | 23:48 |
| thumper | true means ensure was all good | 23:48 |
| thumper | false means we had to generate the certs | 23:48 |
| * thumper does that | 23:48 | |
| davecheney | thumper: closes we have http://play.golang.org/p/UJV5ff9fCg | 23:48 |
| davecheney | thumper: sounds good enough for the moment | 23:49 |
| thumper | davecheney: http://play.golang.org/p/zgPWMG_qyj | 23:53 |
| thumper | davecheney: I prefer to be explicit | 23:53 |
| thumper | davecheney: how does that look for a public interface? | 23:53 |
| davecheney | thumper: sgtm | 23:57 |
| davecheney | bools are always hard | 23:57 |
| thumper | cool | 23:57 |
| davecheney | i dislike function calls with f(bool, bool, int, bool, bool) | 23:57 |
| davecheney | they encourge people to demand named args | 23:57 |
| davecheney | thumper: anyway, this is low level nitty gritty shit | 23:57 |
| davecheney | so i think it is fine to be explicit | 23:58 |
| thumper | davecheney: well, we do work for pedantical :) | 23:58 |
| * davecheney rimshot | 23:59 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!