[12:14] <teknico> is anyone around today? :-) I need a second review of https://codereview.appspot.com/8094045/
[12:52] <dimitern> teknico: will take a look
[12:52] <fwereade> tkeLGTM
[12:53] <fwereade> teknico, LGTM
[12:53] <dimitern> :)
[12:53] <dimitern> sorry to be so late
[12:53] <fwereade> teknico, just one thing to think about and one test to add
[12:54] <teknico> 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] <fwereade> teknico, I thought he suggested dropping it at the api layer
[12:54] <fwereade> teknico, I definitely agree with that
[12:55] <teknico> fwereade, oh, so I probably misunderstood his suggestion, I'll check
[12:55] <fwereade> teknico, it's just that the CLI can take a moment to connect, and failing fast there is nice for the user
[12:55] <teknico> fwereade, right
[12:55] <teknico> fwereade, and I'll add the test you mentioned, thanks for the review
[12:55] <fwereade> teknico, thanks for the code :)
[13:17] <fwereade> dimitern, a couple of things btw
[13:17] <dimitern> fwereade: yeah?
[13:17] <fwereade> dimitern, I have reviews up: https://codereview.appspot.com/8140044/ https://codereview.appspot.com/8154043/
[13:17] <dimitern> fwereade: I'll take a look a bit later - i'm deeply into status tests refactoring right now
[13:18] <fwereade> 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] <dimitern> fwereade: which one?
[13:18] <fwereade> 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] <dimitern> fwereade: but i'm not yet changing what status outputs
[13:19] <fwereade> dimitern, it really is only a suggestion
[13:20] <dimitern> fwereade: it could be a follow-up though, after the status refactoring
[13:20] <dimitern> fwereade: thanks
[13:20] <fwereade> 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] <dimitern> fwereade: sure :) sgtm
[13:20] <fwereade> 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] <dimitern> fwereade: yep
[13:24] <dimitern> fwereade: can I use export_test to provide access to the internals to some other module?
[13:25] <fwereade> dimitern, only to the associated foo_test package
[13:25] <dimitern> fwereade: I'm writing the cleanup stuff for the status tests, and one step is destroying all machines created
[13:25] <fwereade> dimitern, not to everything
[13:25] <dimitern> fwereade: but I cannot destroy (normally) machines with JobManageEnviron
[13:25] <dimitern> fwereade: yeah, that sucks.. any thoughts how to work around it?
[13:25] <fwereade> dimitern, maybe just Reset the test suite?
[13:26] <fwereade> dimitern, UniterSuite has the same sort of problem
[13:26] <dimitern> fwereade: ah, let me try
[13:26] <fwereade> dimitern, check what we do there
[13:43] <dimitern> fwereade: cool, it worked like charm, 10x :)
[13:43] <fwereade> dimitern, sweet
[15:16] <dimitern> fwereade: nice CLs - LGTM on both
[15:16] <fwereade> dimitern, tyvm
[15:16] <dimitern> fwereade: and I'm ready with the status stuff - would you take a look? https://codereview.appspot.com/8133045
[15:16] <fwereade> dimitern, soon I hope; thinking hard about something
[15:17] <dimitern> fwereade: sure, np. I'll go for a run and a short lunch afterwards
[17:02] <fwereade> dimitern, reviewed
[17:02] <fwereade> dimitern, not quite an LGTM but just because I want to know what you think about the change I suggest
[17:02] <dimitern> fwereade: cheers! I just made a small change to optimize test speed
[17:02] <fwereade> dimitern, if you're fine with it and clear about it then I'll LGTM it; let me know what you think
[17:02] <dimitern> fwereade: I'm running status --format {yaml,json} concurrently
[17:03] <dimitern> fwereade: i'll take a look
[17:03] <dimitern> 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] <fwereade> dimitern, excellent
[17:04] <fwereade> dimitern, very good idea
[17:06] <dimitern> fwereade: sweet! I'll implement your changes and repropose then
[17:50] <dimitern> fwereade: thinking about it, if I drop the output from the test case, the concurrency won't be even needed
[17:51] <fwereade> dimitern, ah, better still
[17:51] <dimitern> fwereade: and I can speed-up and simplify the tests even further by putting basically everything into one case with multiple expects
[17:51] <dimitern> fwereade: because the nature of the tests are incremental
[17:51] <fwereade> dimitern, I am a little bit suspicious here
[17:52] <dimitern> fwereade: but with the added benefit of independence between tests and multiple cases still possible
[17:52] <fwereade> dimitern, this sort of thing becomes a real hassle as the incremental chains grow
[17:52] <fwereade> dimitern, what's the speed/nastiness tradeoff?
[17:52] <dimitern> fwereade: yes, but my point is - you can still have small separate cases, well isolated
[17:53] <dimitern> fwereade: and have bigger aggregated cases which built incrementally and expect as they go
[17:53] <fwereade> dimitern, I agree that several of those test cases can be profitably combined into incremental ones
[17:53] <dimitern> fwereade: well, if most of the cases are incremental the speed will increase, because the state will be setup once
[17:54] <fwereade> dimitern, understood
[17:54] <dimitern> fwereade: but unlike before, you can easily add independent other cases with the same framework
[17:55] <fwereade> dimitern, but faster doesn't beat easier-to-modify, so ideally none of the cases should have more than a few expects
[17:55] <dimitern> fwereade: actually concurrency can still be there - but in expect{} only
[17:55] <fwereade> dimitern, yeah, that's probably worth it
[17:55] <dimitern> fwereade: ok, i'll try both and see how it looks
[18:05] <dimitern> 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] <fwereade> dimitern, comment field for the expects? :)
[18:06] <dimitern> fwereade: hmm.. why not
[18:08] <dimitern> fwereade: but then the summary is redundant - it'll be in expect only
[18:08] <fwereade> dimitern, that's probably fine, isn't it?
[18:08] <dimitern> fwereade: yeah, doesn't look so bad actually
[18:08] <fwereade> dimitern, but, well
[18:08] <dimitern> fwereade: will propose shortly
[18:08] <fwereade> dimitern, if you're gruping them yu probably ought to be able to explain why those tests need to go together
[18:09] <fwereade> dimitern, so maybe it's not so redundant :)
[18:09] <dimitern> fwereade: ah, good point
[18:33] <dimitern> 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] <dimitern> fwereade: ah, actually no - they're about as fast as before, just checked - but the added functionality/flexibility is much greater
[18:40] <fwereade> dimitern, sounds like a win regardless
[18:40] <fwereade> dimitern, they were way nicer even at the first propose
[18:40] <dimitern> fwereade: reproposed
[18:41] <dimitern> fwereade: oh yeah - a joy to look at, rather than a disgust :)
[18:44] <hazmat> 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] <hazmat> pack: cannot open $WORK/launchpad.net/juju-core/cmd/jujud/_obj/_go_.8
[18:45] <hazmat> i'm using go tip.. and just did a recursive update via go get
[18:47] <dimitern> hazmat: can you try going inside cmd/jujud/ and running go install there ?
[18:48] <hazmat> dimitern, returns immediately.. juju version returns -> 1.9.13-quantal-i386
[18:48] <hazmat> ditto for jujud
[18:48] <dimitern> 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] <dimitern> hazmat: ah, you're on i386?
[18:48] <hazmat> dimitern, that's probably it, thanks.. indeed re 32b
[18:49] <fwereade> dimitern, I think I see another avenue for niceness, potentially
[18:49] <fwereade> dimitern, there are quite a lot of duplicated chunks of data in the tests still
[18:49] <dimitern> hazmat: not sure if it'll ever work on i386 for now
[18:49] <hazmat> i should probably do separate gopaths for go tip vs release
[18:49] <hazmat> dimitern, it used to
[18:49] <dimitern> hazmat: yeah, that's best
[18:50] <dimitern> fwereade: yeah?
[18:50] <dimitern> fwereade: in the expected output?
[18:50] <fwereade> dimitern, I'm thinking things like var exposedService = M{"exposed":true, "charm": "cs:foo/bar-1"}
[18:51] <dimitern> 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] <fwereade> dimitern, and var machine0 = M{"dns-name": "dummyenv-0.dns", "instance-id": "dummyenv-0"}
[18:52] <dimitern> fwereade: it has to be explicit somewhere
[18:52] <dimitern> at least
[18:52] <fwereade> dimitern, I *think* there's enough duplication that a list of <10 building blocks will collapse the rest a lot
[18:52] <dimitern> fwereade: even if in subsequent cases it's short-circuited
[18:53] <fwereade> dimitern, but I'll leave it to your judgment
[18:53] <dimitern> fwereade: ok, I'll think about it
[18:53] <dimitern> fwereade: otherwise?
[18:54] <fwereade> dimitern, otherwise LGTM, I think, just skimming the rest again
[18:56] <fwereade> dimitern, sent
[18:56] <dimitern> fwereade: cool! tyvm
[18:57] <dimitern> fwereade: I think I prefer having the output unchanged first time we're testing a scenario, and use shortcuts afterwards, sane?
[18:57] <fwereade> dimitern, I'm fine with that
[18:57] <fwereade> dimitern, we can decide next pass
[18:58] <dimitern> fwereade: great, I'll do it like that then
[19:05] <dimitern> fwereade: looks nicer now I think
[19:05] <dimitern> fwereade: which status-related card were you referring to earlier? I can't seem to see it.
[19:24] <fwereade> dimitern, the big red one in core's TODO
[19:24] <dimitern> fwereade: right! thanks
[19:25] <fwereade> dimitern, np
[19:25] <dimitern> fwereade: so what's the idea in bug 1121916
[19:25] <_mup_> Bug #1121916: status compatibility <cmdline> <juju-core:New> < https://launchpad.net/bugs/1121916 >
[19:26] <dimitern> fwereade: changing the machine id to int?
[19:45] <hazmat> latest failure trying to bootstrap with constraints, no image found.. http://pastebin.ubuntu.com/5659178/
[19:46] <hazmat> if i already uploaded tools to the bucket, shouldn't it defer to those tools on subsequent bootstraps?
[19:47] <hazmat> dimitern, yeah.. that's the core
[19:48] <hazmat> perhaps constraints are working yet.. ?
[19:48] <hazmat> i though they worked for the bootstrap node
[20:00] <hazmat> 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] <hazmat> 32bit client dictation
[20:01] <dimitern> hazmat: exactly, that was my point - there is no mongo 2.2 for i386
[20:01] <hazmat> oh.. thats why it can't find the image either.. its client dictating 32bit for tools and cascading to images.. mongo. etc
[20:02] <dimitern> hazmat: yeah, it's viral
[20:02] <dimitern> hazmat: you could try without --upload-tools
[20:03] <dimitern> hazmat: but i'm not sure there are i386 tools in the dist bucket at all
[20:06] <hazmat> dimitern, no matching tools version without the --upload
[20:07] <dimitern> hazmat: you can probably build mongo from source + ssl and put at the same path in your private bucket from environ
[20:07] <hazmat>    dimitern easier to fire up a 64bit vm
[20:07] <hazmat> but thanks for helping debug
[20:07] <dimitern> :) sure
[21:08] <fwereade> hazmat, sorry about that -- fixing up the tool-finding is in the backlog and I hope we will get there soon