[12:14] is anyone around today? :-) I need a second review of https://codereview.appspot.com/8094045/ [12:52] teknico: will take a look [12:52] tkeLGTM [12:53] teknico, LGTM [12:53] :) [12:53] sorry to be so late [12:53] teknico, just one thing to think about and one test to add [12:54] fwereade, re: the args num removal, I removed it because of a request by rogpeppe, as you can see above in the code review [12:54] teknico, I thought he suggested dropping it at the api layer [12:54] teknico, I definitely agree with that [12:55] fwereade, oh, so I probably misunderstood his suggestion, I'll check [12:55] teknico, it's just that the CLI can take a moment to connect, and failing fast there is nice for the user [12:55] fwereade, right [12:55] fwereade, and I'll add the test you mentioned, thanks for the review [12:55] teknico, thanks for the code :) [13:17] dimitern, a couple of things btw [13:17] fwereade: yeah? [13:17] dimitern, I have reviews up: https://codereview.appspot.com/8140044/ https://codereview.appspot.com/8154043/ [13:17] fwereade: I'll take a look a bit later - i'm deeply into status tests refactoring right now [13:18] dimitern, and, in kanban, there is a defect card that you *might* want topick up after what you're doing now because it leads into it nicely [13:18] fwereade: which one? [13:18] dimitern, it's the machine-id-in-status one: it's in core's TODO lane but feel free to grab it if you want it :) [13:19] fwereade: but i'm not yet changing what status outputs [13:19] dimitern, it really is only a suggestion [13:20] fwereade: it could be a follow-up though, after the status refactoring [13:20] fwereade: thanks [13:20] dimitern, if say at nearly-eod you think to yourself "what tiny follow-up branch do I feel like doing before I go" [13:20] fwereade: sure :) sgtm [13:20] dimitern, I agree it's not the same as what you're doing now and should certainly not be rolled into the same CL :) [13:23] fwereade: yep [13:24] fwereade: can I use export_test to provide access to the internals to some other module? [13:25] dimitern, only to the associated foo_test package [13:25] fwereade: I'm writing the cleanup stuff for the status tests, and one step is destroying all machines created [13:25] dimitern, not to everything [13:25] fwereade: but I cannot destroy (normally) machines with JobManageEnviron [13:25] fwereade: yeah, that sucks.. any thoughts how to work around it? [13:25] dimitern, maybe just Reset the test suite? [13:26] dimitern, UniterSuite has the same sort of problem [13:26] fwereade: ah, let me try [13:26] dimitern, check what we do there [13:43] fwereade: cool, it worked like charm, 10x :) [13:43] dimitern, sweet === wedgwood_away is now known as wedgwood [15:16] fwereade: nice CLs - LGTM on both [15:16] dimitern, tyvm [15:16] fwereade: and I'm ready with the status stuff - would you take a look? https://codereview.appspot.com/8133045 [15:16] dimitern, soon I hope; thinking hard about something [15:17] fwereade: sure, np. I'll go for a run and a short lunch afterwards [17:02] dimitern, reviewed [17:02] dimitern, not quite an LGTM but just because I want to know what you think about the change I suggest [17:02] fwereade: cheers! I just made a small change to optimize test speed [17:02] dimitern, if you're fine with it and clear about it then I'll LGTM it; let me know what you think [17:02] fwereade: I'm running status --format {yaml,json} concurrently [17:03] fwereade: i'll take a look [17:03] fwereade: this change speeds up the tests about 50% (which is a bit better than they used to be, since changing them slowed them down - state is recreated each time) [17:03] dimitern, excellent [17:04] dimitern, very good idea [17:06] fwereade: sweet! I'll implement your changes and repropose then [17:50] fwereade: thinking about it, if I drop the output from the test case, the concurrency won't be even needed [17:51] dimitern, ah, better still [17:51] fwereade: and I can speed-up and simplify the tests even further by putting basically everything into one case with multiple expects [17:51] fwereade: because the nature of the tests are incremental [17:51] dimitern, I am a little bit suspicious here [17:52] fwereade: but with the added benefit of independence between tests and multiple cases still possible [17:52] dimitern, this sort of thing becomes a real hassle as the incremental chains grow [17:52] dimitern, what's the speed/nastiness tradeoff? [17:52] fwereade: yes, but my point is - you can still have small separate cases, well isolated [17:53] fwereade: and have bigger aggregated cases which built incrementally and expect as they go [17:53] dimitern, I agree that several of those test cases can be profitably combined into incremental ones [17:53] fwereade: well, if most of the cases are incremental the speed will increase, because the state will be setup once [17:54] dimitern, understood [17:54] fwereade: but unlike before, you can easily add independent other cases with the same framework [17:55] dimitern, but faster doesn't beat easier-to-modify, so ideally none of the cases should have more than a few expects [17:55] fwereade: actually concurrency can still be there - but in expect{} only [17:55] dimitern, yeah, that's probably worth it [17:55] fwereade: ok, i'll try both and see how it looks [18:05] fwereade: scratch that, having multiple expect is possible, but you still have a single summary, so it's not good to mix cases like that [18:05] dimitern, comment field for the expects? :) [18:06] fwereade: hmm.. why not [18:08] fwereade: but then the summary is redundant - it'll be in expect only === wedgwood is now known as wedgwood_away [18:08] dimitern, that's probably fine, isn't it? [18:08] fwereade: yeah, doesn't look so bad actually [18:08] dimitern, but, well [18:08] fwereade: will propose shortly [18:08] dimitern, if you're gruping them yu probably ought to be able to explain why those tests need to go together [18:09] dimitern, so maybe it's not so redundant :) [18:09] fwereade: ah, good point [18:33] fwereade: sweet! it improved the speed even further: now it's more than 100% better than the old tests (before refactoring) [18:34] * fwereade cheers [18:40] fwereade: ah, actually no - they're about as fast as before, just checked - but the added functionality/flexibility is much greater [18:40] dimitern, sounds like a win regardless [18:40] dimitern, they were way nicer even at the first propose [18:40] fwereade: reproposed [18:41] fwereade: oh yeah - a joy to look at, rather than a disgust :) [18:44] anybody can offer insight into this error.. on bootstrap --upload-tools ... error: cannot upload tools: build command "go" failed: exit status 2; # launchpad.net/juju-core/cmd/jujud [18:44] pack: cannot open $WORK/launchpad.net/juju-core/cmd/jujud/_obj/_go_.8 [18:45] i'm using go tip.. and just did a recursive update via go get [18:47] hazmat: can you try going inside cmd/jujud/ and running go install there ? [18:48] dimitern, returns immediately.. juju version returns -> 1.9.13-quantal-i386 [18:48] ditto for jujud [18:48] hazmat: also, if you switched to go-tip and using the same $GOPATH as with go-1.x it might be the issue - rm -fr $GOPATH/pkg/ and try again [18:48] hazmat: ah, you're on i386? [18:48] dimitern, that's probably it, thanks.. indeed re 32b [18:49] dimitern, I think I see another avenue for niceness, potentially [18:49] dimitern, there are quite a lot of duplicated chunks of data in the tests still [18:49] hazmat: not sure if it'll ever work on i386 for now [18:49] i should probably do separate gopaths for go tip vs release [18:49] dimitern, it used to [18:49] hazmat: yeah, that's best [18:50] fwereade: yeah? [18:50] fwereade: in the expected output? [18:50] dimitern, I'm thinking things like var exposedService = M{"exposed":true, "charm": "cs:foo/bar-1"} [18:51] fwereade: the output was the only thing I didn't want to touch, even with duplicated parts, because it helps to see what's there [18:51] dimitern, and var machine0 = M{"dns-name": "dummyenv-0.dns", "instance-id": "dummyenv-0"} [18:52] fwereade: it has to be explicit somewhere [18:52] at least [18:52] dimitern, I *think* there's enough duplication that a list of <10 building blocks will collapse the rest a lot [18:52] fwereade: even if in subsequent cases it's short-circuited [18:53] dimitern, but I'll leave it to your judgment [18:53] fwereade: ok, I'll think about it [18:53] fwereade: otherwise? [18:54] dimitern, otherwise LGTM, I think, just skimming the rest again [18:56] dimitern, sent [18:56] fwereade: cool! tyvm [18:57] fwereade: I think I prefer having the output unchanged first time we're testing a scenario, and use shortcuts afterwards, sane? [18:57] dimitern, I'm fine with that [18:57] dimitern, we can decide next pass [18:58] fwereade: great, I'll do it like that then [19:05] fwereade: looks nicer now I think [19:05] fwereade: which status-related card were you referring to earlier? I can't seem to see it. === wedgwood_away is now known as wedgwood [19:24] dimitern, the big red one in core's TODO [19:24] fwereade: right! thanks [19:25] dimitern, np [19:25] fwereade: so what's the idea in bug 1121916 [19:25] <_mup_> Bug #1121916: status compatibility

< https://launchpad.net/bugs/1121916 > [19:26] fwereade: changing the machine id to int? [19:45] latest failure trying to bootstrap with constraints, no image found.. http://pastebin.ubuntu.com/5659178/ [19:46] if i already uploaded tools to the bucket, shouldn't it defer to those tools on subsequent bootstraps? [19:47] dimitern, yeah.. that's the core [19:48] perhaps constraints are working yet.. ? [19:48] i though they worked for the bootstrap node [20:00] hmm.. sans constraints, failed bootstrap it looks because it can't find/setup mongo http://juju-dist.s3.amazonaws.com/tools/mongo-2.2.0-quantal-i386.tgz [20:00] 32bit client dictation [20:01] hazmat: exactly, that was my point - there is no mongo 2.2 for i386 [20:01] oh.. thats why it can't find the image either.. its client dictating 32bit for tools and cascading to images.. mongo. etc [20:02] hazmat: yeah, it's viral [20:02] hazmat: you could try without --upload-tools [20:03] hazmat: but i'm not sure there are i386 tools in the dist bucket at all [20:06] dimitern, no matching tools version without the --upload [20:07] hazmat: you can probably build mongo from source + ssl and put at the same path in your private bucket from environ [20:07] dimitern easier to fire up a 64bit vm [20:07] but thanks for helping debug [20:07] :) sure [21:08] hazmat, sorry about that -- fixing up the tool-finding is in the backlog and I hope we will get there soon === wedgwood is now known as wedgwood_away