[00:27] <thumper> wallyworld__: taking the dog for a walk in the hope she'll go outside...
[00:27] <wallyworld__> ok
[01:20] <thumper> geez luise
[01:21] <thumper> she has great bladder control holding it for inside
[01:21] <thumper> although this time, she did go outside
[01:21] <thumper> thankfully
[01:21] <thumper> only after we had got back, and I refused to take her inside
[01:25] <thumper> wallyworld__: wow some of our code is wonderfully inefficient
[01:42] <wallyworld__> thumper: which bit?
[01:42] <thumper> wallyworld__: the provisioner, that I'm currently refactoring
[01:42] <wallyworld__> ah. good thing you're onto it then
[01:43] <thumper> I'm busy teasing apart dependencies
[01:48] <wallyworld__> i find not having generics leads to a *lot* of code duplication
[01:50] <thumper> I really wish our state methods would return interfaces rather than real structs
[01:50] <thumper> makes mocking and testing a PITA
[01:52] <wallyworld__> oh yes +100 to that one
[01:53] <thumper> wallyworld__: I've made interfaces for the provisioner on all things that I can
[01:53] <thumper> so the new bits take interfaces not structs
[01:54] <wallyworld__> \o/
[03:21] <thumper> wallyworld__: OMFG
[03:29] <thumper> bigjools: ok, can have that talk now
[03:41] <wallyworld__> thumper: what?
[03:41] <thumper> wallyworld__: my day of refactoring passed the tests first time (once I fixed all the compile errors)  https://codereview.appspot.com/9937046
[03:42] <wallyworld__> cool, i'll look
[04:29] <bigjools> wallyworld__: need to talk to you, and it's about work.... <shock>
[04:29] <wallyworld__> shit eh
[04:29] <bigjools> it could be
[04:30] <bigjools> I could just call you but reception is shit so get on a hangout will ya
[04:30] <wallyworld__> i thought since you asked you'd create one, but i will
[04:31] <wallyworld__> https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a6
[06:54] <rogpeppe1> mornin' all
[07:02] <TheMue> rogpeppe1: heya
[07:02] <rogpeppe1> TheMue: hiya
[08:48] <fwereade__> TheMue, ping
[08:55] <TheMue> fwereade__: pong
[08:56] <TheMue> fwereade__: just wanted to ping roger ;) but will do it afterwards
[09:01] <TheMue> fwereade__: ah, it seems i found what i wanted to ask
[09:02] <fwereade__> TheMue, would you please send thumper a link to your lxc work so he can maybe look through it tonight?
[09:03] <TheMue> fwereade__: sure, it's a very interesting topic
[09:03] <TheMue> fwereade__: will do immediately
[09:04] <TheMue> fwereade__: btw, watcher with test is working
[09:23] <dimitern> mgz: hey
[09:29] <mgz> dimitern: hey
[09:29] <dimitern> mgz: how can we go about pairing on implementing the deployer stuff?
[09:30] <mgz> I'm open to suggestions
[09:31] <mgz> one option is using my vm with go stuff on and screen or something, plus voice
[09:37] <dimitern> mgz: not sure i see the process will go - can you expand a bit?
[09:40] <dimitern> rogpeppe1: responed to https://codereview.appspot.com/10044043/
[09:44] <mgz> trying to remember what jam and I did exactly, basically you just ssh into my canonistack box then run screen with a couple of flags, then we try to not type at the same time :)
[09:49] <dimitern> mgz: I see, well we can try it out
[09:49] <dimitern> just as soon as I finish responding to my reviews
[09:52] <mgz> sure
[09:59] <dimitern> rogpeppe1: responded to this as well https://codereview.appspot.com/10026044/
[10:01] <dimitern> looking for reviews on this: https://codereview.appspot.com/9937045/
[10:02] <dimitern> fwereade__: ^^
[10:06] <dimitern> mgz: i'm afraid we have to base the deployer stuff on one of my CLs, so until it lands maybe we can discuss it first
[10:07] <mgz> that sounds good
[10:07] <dimitern> but hopefully it won't change much, we can just use it as a prereq before it lands actually
[10:08] <mgz> what's the branch?
[10:08] <dimitern> lp:~dimitern/juju-core/055-api-machiner-subpackages
[10:09] <dimitern> assuming we take the same path and develop it first server-side as a subpackage
[10:09] <dimitern> that branch introduces some interfaces and decouples the root api object from the machiner facade
[10:10] <dimitern> fwereade__: ping
[10:10] <dimitern> mgz: g+?
[10:10] <fwereade__> dimitern, pong
[10:10] <dimitern> fwereade__: do you have some time to g+ about the general direction of the deployer facade?
[10:11] <fwereade__> dimitern, give me 5 mins
[10:11] <dimitern> fwereade__: sure
[10:12] <mgz> dimitern: can do, but mumble would work as well for this
[10:13] <dimitern> mgz: sgtm, if fwereade__ won't mind
[10:13] <mgz> ah, g+ then if we want him too
[10:13] <dimitern> mgz: in the mean time have a look at worker/deployer/deployer.go
[10:14] <dimitern> mgz: i have extracted a list of state calls used by it: http://paste.ubuntu.com/5738348/
[10:14] <mgz> ah, crap, but I need to reboot into chromeos for that...
[10:14] <dimitern> mgz: oh, sorry about that
[10:15] <mgz> fwereade__: can you install mumble quickly?
[10:19] <fwereade__> mgz, dimitern, ok, I am here, I need the bathroom, I will try to get mumble up in a sec
[10:20] <dimitern> mgz, fwereade__: we can meet in the Blue room on mumble (Root -> Cloud Engineering -> Blue)
[10:25] <fwereade__> cheers
[10:26] <dimitern> fwereade__: if you go to settings in mumble and enable push to talk + set the shortcut to something (i use numlock) and hold it while speaking I found it works best
[10:26] <dimitern> fwereade__: because the noise/silence detection sucks in mumble
[10:32] <jamespage> mgz, bug 1188126
[10:32] <jamespage> https://bugs.launchpad.net/juju-core/+bug/1188126
[10:37] <TheMue> *: any known problems with the .../worker tests? they timed out here when doing test ./...
[10:40] <mgz> jamespage: thanks!
[10:51] <TheMue> rogpeppe1: ping
[10:51] <rogpeppe1> TheMue: pong
[10:52] <TheMue> rogpeppe1: just performing tests including the worker package. had a timeout, tests ran too long.
[10:52] <rogpeppe1> TheMue: can you reproduce the problem?
[10:52] <TheMue> rogpeppe1: now I took TestOneWorkerStart as a first one to see where it happens
[10:52] <TheMue> rogpeppe1: and already this hangs
[10:53] <TheMue> rogpeppe1: yes, reproducable
[10:54] <rogpeppe1> TheMue: this is with trunk?
[10:55] <TheMue> rogpeppe1: had done a pull before and run 1253 (plus my change which is something added, but not used so far)
[10:55] <TheMue> rogpeppe1: will go into trunk and try there too, to get sure
[10:55] <rogpeppe1> TheMue: if you get a stack trace from the hanging test (with ctrl-\), what does it print?
[10:57] <TheMue> rogpeppe1: one moment
[10:58] <TheMue> rogpeppe1: strange
[10:59] <TheMue> rogpeppe1: the .\... timed out on the shell and from inside the editor
[10:59] <TheMue> rogpeppe1: now i'm only in that package and is passes
[10:59]  * TheMue shakes his head
[11:00] <rogpeppe1> TheMue: how do you know that the worker package is the one that's failing?
[11:00] <TheMue> rogpeppe1: *** Test killed: ran too long.
[11:00] <TheMue> FAIL	launchpad.net/juju-core/worker	600.005s
[11:01] <TheMue> rogpeppe1: and then i started the worker tests from inside the editor with the same result (the output above has been on the shell)
[11:02] <rogpeppe1> TheMue: can you kill -QUIT the worker tests that are hanging within the editor, please.
[11:02] <rogpeppe1> TheMue: so we can find out what they're hanging
[11:02] <rogpeppe1> s/what/where/
[11:03] <TheMue> rogpeppe1: btw, just started it in the shell again and it hangs
[11:03] <TheMue> rogpeppe1: will paste you the output
[11:03] <rogpeppe1> TheMue: thanks
[11:04] <TheMue> rogpeppe1: http://paste.ubuntu.com/5738426/
[11:04] <TheMue> rogpeppe1: carmen just called me for lunch, biab
[11:05] <rogpeppe1> TheMue: oh dammit
[11:05] <dimitern> rogpeppe1: https://codereview.appspot.com/10044043/ - need some help on this please
[11:06] <rogpeppe1> TheMue: when you get the chance, please run go test -c; then run the resulting binary and kill that
[11:06] <rogpeppe1> TheMue: then we won't be seeing two stack traces interleaved
[11:06] <rogpeppe1> dimitern: looking
[11:07] <rogpeppe1> dimitern: sorry for not responding earlier - i was in a call
[11:07] <dimitern> rogpeppe1: np
[11:08] <dimitern> rogpeppe1: i'm really bad at phrasing doc comments and will appreciate suggestions like for the Tagger
[11:31] <danilos_> dimitern, wallyworld: hi, hangout?
[11:32] <wallyworld> ok
[11:32] <rogpeppe1> dimitern: responded
[11:32] <dimitern> danilos_: uh, yes, forgot we're doing g+ now
[11:32] <dimitern> rogpeppe1: cheers
[11:34] <danilos> dimitern, https://codereview.appspot.com/9876043/
[11:40] <dimitern> rogpeppe1: can we agree to disagree on the apiserver machiner_test? :)
[11:41] <dimitern> rogpeppe1: and it's value
[11:41] <dimitern> (that it's valueable)
[11:42] <rogpeppe1> dimitern: is there anything there that can't be tested just as easily through the client API interface?
[11:42] <rogpeppe1> dimitern: i'm not saying the tests themselves aren't valuable
[11:42] <dimitern> rogpeppe1: the client-side test is a different one
[11:42] <dimitern> rogpeppe1: this is a unit test
[11:42] <dimitern> rogpeppe1: the client one is an integration test
[11:43] <dimitern> rogpeppe1: and they tests different things
[11:43] <rogpeppe1> dimitern: i see the tests in apiserver/machiner and i think that i would like to verify that all of that stuff works from the client all the way to the server
[11:43] <dimitern> rogpeppe1: not quite
[11:44] <rogpeppe1> dimitern: i think by testing everything through the client, we get the best of both worlds
[11:44] <dimitern> rogpeppe1: the client-side doesn't need to care about the correctness of bulk operations: handling parameters and results, as well as permission checks
[11:44] <rogpeppe1> dimitern: surely it does
[11:45] <dimitern> rogpeppe1: no it doesn't, because the interface as exposed is not about that
[11:45] <dimitern> rogpeppe1: permissions yes, but not bulk ops
[11:45] <rogpeppe1> dimitern: you mean because we don't expose the bulk operations currently as part of the machiner client API?
[11:45] <dimitern> rogpeppe1: and the permissions are already tested at server side
[11:45] <dimitern> rogpeppe1: yeah
[11:46] <rogpeppe1> dimitern: this feels wrong to me
[11:46] <rogpeppe1> dimitern: all the way along we have implemented client functionality that gives us access to the operations supported by the server
[11:46] <dimitern> rogpeppe1: client-side tests are about how you use the interface as a client, not so much how the transport layer works
[11:46] <rogpeppe1> dimitern: and this is breaking that correspondence
[11:47] <dimitern> rogpeppe1: don't you agree unit tests are a good thing?
[11:47] <rogpeppe1> dimitern: i think more test coverage is a good thing
[11:48] <rogpeppe1> dimitern: and AFAICS by putting the tests on the server side only, we get less test coverage
[11:48] <rogpeppe1> dimitern: whereas if we do it at the client side, we get to test the full path almost for free
[11:48] <dimitern> rogpeppe1: exactly
[11:48] <dimitern> rogpeppe1: and also testing at the right place
[11:48] <dimitern> rogpeppe1: no, they are client-side tests in the follow-up
[11:49] <dimitern> rogpeppe1: testing end-to-end only is not sufficient imho
[11:49] <rogpeppe1> dimitern: really? why not?
[11:49] <rogpeppe1> dimitern: what can you test on the server side that you cannot test end to end?
[11:49] <dimitern> rogpeppe1: and incurrs the overhead of starting a server when you need only to test the server-part in isolation
[11:50] <rogpeppe1> dimitern: starting a server takes microseconds
[11:50] <dimitern> rogpeppe1: as i already mentioned, bulk operations, parameters handling, permissions, results
[11:50] <rogpeppe1> dimitern: we should have client entry points for the bulk operations. i'm not sure how the rest aren't (or can't) be tested with end-to-end tests
[11:51] <rogpeppe1> dimitern: that's how it's all going to be used, after all
[11:51] <dimitern> rogpeppe1: we don't need client-side bulk entry points for the machiner yet
[11:51] <dimitern> rogpeppe1: but we need to support them at transport level, as agreed
[11:51] <rogpeppe1> dimitern: i think we should have them anyway, so we can test them end to end
[11:52] <rogpeppe1> dimitern: it's not much code, as fwereade__ keeps on reassuring me
[11:52] <dimitern> rogpeppe1: ok, i think we're in a vicious circle here
[11:52] <dimitern> rogpeppe1: we need to test both sides
[11:52] <fwereade__> rogpeppe1, dimitern, damn, I meant to address this before
[11:52] <fwereade__> rogpeppe1, dimitern, shall we have a quick g+?
[11:52] <rogpeppe1> fwereade__: sure
[11:52] <dimitern> fwereade__: what?
[11:53] <dimitern> fwereade__: ok
[11:53] <TheMue> rogpeppe1: http://paste.ubuntu.com/5738483/ <= this one is by calling the test binary
[11:53] <dimitern> fwereade__: shall I start one or you will?
[11:54] <rogpeppe1> TheMue: thanks. that's more useful.
[11:54] <fwereade__> dimitern, would you, just finishing an email
[11:54] <dimitern> fwereade__: sure
[11:54] <TheMue> rogpeppe1: great
[11:55] <dimitern> rogpeppe1, fwereade__: https://plus.google.com/hangouts/_/1842432ac5d47d4b006b6679b49cb3d836c8163f?authuser=0&hl=en
[12:28] <AeroNotix> When using the goose Nova binding, is the client expected to provide an imageRef themselves?
[12:29] <AeroNotix> I have a similar set of bindings and I'm at a loss at how to combat the ever-changing list of images.
[12:30] <TheMue> *: one CL for review: https://codereview.appspot.com/10078043
[13:50] <dimitern> rogpeppe3: can you sketch out an idea of how requireMachiner should look like?
[13:51] <dimitern> rogpeppe3: not sure about the check "is machine"
[13:51] <rogpeppe3> dimitern: http://paste.ubuntu.com/5738805/
[13:52] <rogpeppe3> oops
[13:52] <rogpeppe3> dimitern: s/requireAgent/requireMachine/
[13:52] <dimitern> rogpeppe3: ah, ok, so it's simpler than i thought
[13:52] <rogpeppe3> dimitern: not too bad, eh?
[13:52] <dimitern> rogpeppe3: will add it in machiner.new
[13:53] <rogpeppe3> dimitern: cool
[13:54] <dimitern> rogpeppe3: indeed
[13:55] <dimitern> rogpeppe3: and then there's this: https://codereview.appspot.com/10026044/
[13:56] <rogpeppe3> dimitern: FWIW i think we could just have api.Machine
[13:56] <rogpeppe3> dimitern: and have it talk to whatever facade is appropriate
[13:57] <dimitern> rogpeppe3: not really, because machiner.Machine and provisioner.Machine, etc. are different
[13:57] <rogpeppe3> dimitern: we could quite easily keep them compatible
[13:57] <dimitern> rogpeppe3: i think the point of the separation is not making them compatible
[13:58] <rogpeppe3> dimitern: ah. i thought the reason was security.
[13:58] <dimitern> rogpeppe3: that's the main part of it
[13:59] <dimitern> rogpeppe3: not having a method you can call is the best security, isn't it?
[13:59] <rogpeppe3> dimitern: on the server side, yes.
[13:59] <dimitern> rogpeppe3: and will catch issues like this at compile time
[13:59] <dimitern> oh well.. kanban time
[14:00] <rogpeppe> dimitern: agreed. it depends how much code we're willing to write for that.
[14:44] <rogpeppe> time to reboot. this machine is becoming unusable.
[14:58] <dimitern> rogpeppe: so wrt unifying ServerError and ServerErrorToParams
[14:59] <dimitern> rogpeppe: the only thing that needs wrapping is the param to conn.Serve in apiserver.go
[15:00] <rogpeppe> dimitern: i'm not sure i understand
[15:00] <dimitern> rogpeppe: i'd still rather have separate helpers, but if you insist, I'll create a serverError there locally which will call common.ServerError and converting the result to error
[15:00] <rogpeppe> dimitern: why can't serverError serve both purposes?
[15:01] <rogpeppe> dimitern: you'll need to type-convert the result, but i don't think that's a big issue
[15:01] <dimitern> rogpeppe: because conn.Server needs func (error) error
[15:01] <dimitern> rogpeppe: and in all other places I need *params.Error, not error
[15:02] <rogpeppe> dimitern: serverError(err).(*params.Error) ?
[15:02] <dimitern> rogpeppe: it's not a method call, it's a callback
[15:02] <dimitern> rogpeppe: so I need to pass something that takes error, calls common.ServerError internally and returns error
[15:03] <rogpeppe> dimitern: i mean: keep serverError as it is (but make it always return *params.Error for non-nil errors). then do the above if you want a params.Error
[15:04] <rogpeppe> dimitern: then there's no need for two separate error types (api.Error and params.Error)
[15:04] <rogpeppe> dimitern: or for the somewhat awkwardly named ServerErrorToParams
[15:04] <dimitern> rogpeppe: it has to be something like: err1 := common.ServerError(err); if err1 != nil { return err1.(error) }
[15:05] <rogpeppe> dimitern: in all the cases you're currently using ServerErrorToParams, you've got that err != nil check anyway
[15:05] <dimitern> rogpeppe: just a sec, let me paste it
[15:06] <jtv> Hi folks — quick question if you don't mind.  Is this test failure normal?  http://paste.ubuntu.com/5738993/
[15:06] <jtv> It looks a bit like the machine's speed just happens to be barely out of the test's tolerance range or something.
[15:06] <rogpeppe> dimitern: alternatively do as you suggested above: make a local closure in side serveConn
[15:07] <rogpeppe> jtv: i haven't seen that failure before, but that test is quite new
[15:08]  * rogpeppe hates timing-sensitive tests
[15:08] <jtv> rogpeppe hates with reason.
[15:09] <rogpeppe> jtv: i think the test just needs more wiggle room
[15:09] <jtv> Heh.
[15:09] <rogpeppe> jtv: feel free to propose a change that ups it to 200ms or so
[15:09] <dimitern> rogpeppe: http://paste.ubuntu.com/5739005/ something like this
[15:09] <jtv> rogpeppe: I'll try to get other tests running first though... not feeling very lucky today!
[15:10] <rogpeppe> dimitern: that looks ok. i'm not sure i'd bother with the separate func for serverError though - just inline it in the call to conn.Serve
[15:11] <dimitern> rogpeppe: that's nasty :) but might do
[15:11] <rogpeppe> dimitern: oh, one other thing:
[15:11] <dimitern> rogpeppe: or just make it serverError := func(error) error { ... }
[15:11] <rogpeppe> dimitern: that's fine too
[15:12] <rogpeppe> dimitern: you'll need to check if the error returns an error code
[15:13] <dimitern> rogpeppe: why?
[15:13] <rogpeppe> dimitern: and use that error code if o
[15:13] <rogpeppe> so
[15:13] <dimitern> rogpeppe: not sure i get you
[15:13] <dimitern> rogpeppe: you mean in common.ServerError at the end?
[15:13] <rogpeppe> dimitern: yeah
[15:14] <rogpeppe> dimitern: it means that API server code can return coded errors if they want
[15:14] <dimitern> rogpeppe: I'll look into it (hopefully some tests will fail if I mess it up :)
[15:15] <rogpeppe> dimitern: good point. i'm not sure that functionality *is* specifically tested. you'll probably want to add a test.
[15:16] <rogpeppe> dimitern: ah, actually that functionality is tested
[15:16] <dimitern> rogpeppe: so then it becomes: http://paste.ubuntu.com/5739032/
[15:16] <rogpeppe> dimitern: and the test will indeed fail
[15:16] <rogpeppe> dimitern: not quite
[15:17] <dimitern> rogpeppe: that's a direct translation of what happens when servererrortoparams is removed
[15:17] <dimitern> rogpeppe: and integrated into servererror
[15:18] <rogpeppe> dimitern: code will never be non-empty on line 17
[15:18] <dimitern> rogpeppe: how did it work before?
[15:19] <rogpeppe> dimitern: it returned the error unwrapped
[15:20] <rogpeppe> dimitern: i think you want something more like this: http://paste.ubuntu.com/5739044/
[15:21] <rogpeppe> dimitern: oops
[15:21] <rogpeppe> dimitern: this: http://paste.ubuntu.com/5739046/
[15:21] <rogpeppe> dimitern: oops, wrong still
[15:21] <rogpeppe> dimitern: better i think: http://paste.ubuntu.com/5739049/
[15:24] <dimitern> rogpeppe: ok, sgtm
[15:42] <jtv> Build failure in trunk as well?  ../../../go/src/launchpad.net/juju-core/environs/openstack/provider.go:592: undefined: identity.AuthKeyPair
[15:42] <jtv> I wonder why that didn't affect my testing.
[15:45] <mgz> jtv: just the normal "you need to pull lp:goose" thing? trunk builds for me
[15:46] <jtv> mgz: I haven't been on juju for a while... what is the "normal you-need-to-pull-lp:goose" thing?  Mind you this is a completely fresh "go get launchpad.net/juju-core/..." from scratch.
[15:48] <mgz> completely fresh? I'd double check your latest revs in launchpad.net/goose
[15:48] <mgz> because your error strongly suggests you have an older goose
[15:48] <mgz> which won't cook as well
[15:49] <jtv> mgz: yes, completely fresh — cloud instance
[15:49] <mgz> I'm on r95 "Add juju-tools keystone endpoint and fix tests"
[15:50] <jtv> Oh, of course I just discarded my instance.  The compile error is on a different system.  Sigh.
[15:51] <jtv> mgz: ah... I'm on a much older revision — "go get launchpad.net/goose" didn't update it apparently.
[15:52] <mgz> indeed not
[15:53] <jtv> mgz: apparently the branch URL has changed... how do I fix that?
[15:54] <rogpeppe> hmm, my 2-factor auth is no longer working. any suggestions anyone?
[15:54] <mgz> `bzr pull --remember lp:goose`
[15:54] <mgz> rogpeppe: use your carefully prepared backup
[15:54] <rogpeppe> mgz: the keyfob itself is working fine
[15:55] <rogpeppe> mgz: but i see "The password is invalid" when I try to log in with it
[15:55] <rogpeppe> mgz: hmm, maybe i messed up "Do not activate your authentication device when not needed. Extra activations will get your device out of sync with the server and lock you out of your account."
[15:55] <jtv> mgz: that updated it, thanks!  I thought I'd run a "go get -u launchpad.net/juju-core/..." which I assumed would update my dependencies.
[15:55] <mgz> if you only have one device regiestered, and it's desynced, you need to ping webops in #webops
[15:55] <mgz> rogpeppe: ^
[15:56] <mgz> jtv: go get is not very smart, and bzr recording resolved locations messes it up
[15:56] <rogpeppe> mgz: thanks, i'll try that. hopefully that's the problem.
[15:56] <rogpeppe> mgz: i didn't know the devices were synced. that seems highly fragile to me.
[15:57] <mgz> timebased auth is much nicer
[15:58] <mgz> but this way is simple, and easily fixable if you have a backup (generally just remove and readd the device)
[15:58] <rogpeppe> mgz: yeah. or challenge response can work ok too.
[15:58] <rogpeppe> mgz: readd ?
[15:58] <mgz> that's not really yubikey accessible though
[15:59] <mgz> ^re-add
[15:59] <mgz> generate a new seed, register device with it, start on fresh sequence
[16:00] <rogpeppe> mgz: ah i see
[16:00] <rogpeppe> mgz: if i'd had any idea that pressing the button one two many times would be a problem, i would have been a bit more careful
[16:00] <rogpeppe> s/two/too/
[16:02] <rogpeppe> mgz: are you sure #webops is the place? perhaps #admin or something might be better?
[16:03] <gnuoy> rogpeppe, web0ps do 2fa resets, and they hangout in #web0ps
[16:04] <dimitern> gnuoy: tis with "0" (zero) or upper-case "O"?
[16:05] <gnuoy> dimitern, sorry I am in the habit of not typing the word as it causes them (and I am one) to be pinged
[16:05] <dimitern> gnuoy: so web0ps is the magic word, good to know :)
[16:06] <gnuoy> dimitern, sorry, no. It is webops and #webops
[16:06] <dimitern> gnuoy: ah, ok
[16:06] <mgz> (on the internal server, not freenode, too)
[16:06] <dimitern> gnuoy: now i got you :) sorry
[16:07] <gnuoy> dimitern, np, sorry for causing the confusion :)
[16:09] <dimitern> jtv: that's a very good skeleton you have there
[16:09] <fwereade__> a second review of https://codereview.appspot.com/9698047/ would be good, it's almost trivial
[16:09] <dimitern> jtv: will you develop most of it like the maas provider?
[16:09] <dimitern> fwereade__: i'm on it
[16:10] <jtv> dimitern: so far it's been _exactly_ like the maas provider.  :)
[16:10] <fwereade__> TheMue, ping
[16:14] <dimitern> jtv: :) i meant in a separate branch
[16:15] <jtv> dimitern: ah, no, we won't be doing that this time.  Too much integration pain.
[16:15] <jtv> That's why I put this up for review now.
[16:15]  * jtv → eod
[16:16] <dimitern> jtv: ah ok
[16:18] <dimitern> rogpeppe: https://codereview.appspot.com/10044043/ all done I think
[16:20] <dimitern> rogpeppe: if you think it's worth an lgtm now i'd like to land it
[16:20] <rogpeppe> dimitern: i'm looking now
[16:33] <rogpeppe> dimitern: reviewed
[16:33] <fwereade__> danilos, ping
[16:34] <dimitern> rogpeppe: cheers
[16:37] <dimitern> rogpeppe: are you sure you want to leak info by reporting CodeNotFound when id != "" and we haven't checked the user is authorized?
[16:38] <rogpeppe> dimitern: i don't think i'm suggesting that. i'm suggesting returning errBadId in that case
[16:38] <dimitern> rogpeppe: istm it good to always first check for authorization
[16:38] <rogpeppe> dimitern: which doesn't leak any info at all
[16:38] <dimitern> rogpeppe: hmm..
[16:38] <dimitern> rogpeppe: ok, i'll go with this, but seems fishy
[16:39] <rogpeppe> dimitern: when/if we start using the ids in this case, we will check first
[16:39] <rogpeppe> dimitern: in that case, we'll probably pass the id into machiner.New
[16:39] <dimitern> rogpeppe: yeah, fair enough
[16:47] <dimitern> rogpeppe: if we lose the ErrNotLoggedIn returned from RequireMachiner, we'll need to have IsLoggedIn() bool in the Authorizer and check that separately
[16:48] <rogpeppe> dimitern: hmm, i wonder if a simple ErrPerm would be good enough in that case
[16:48] <dimitern> rogpeppe: that'll change the login_test
[16:48] <dimitern> rogpeppe: and complicate things
[16:49] <dimitern> rogpeppe: so I prefer to have a separate check in root.Machiner() to return ErrNotLoggedIn as well as now
[16:49] <rogpeppe> dimitern: that seems reasonable actually
[16:49] <dimitern> rogpeppe: ok
[16:50] <rogpeppe> dimitern: then it's easy to check that all the root methods check for logged-in-edness
[16:50] <rogpeppe> dimitern: and it's up to the individual facades to decide on further checks
[16:50] <dimitern> rogpeppe: exactly
[17:21] <dimitern> rogpeppe: last time for today, i promise :) https://codereview.appspot.com/10026044/ - you already reviewed it once, I fixed it as suggested, and the version arg is gone now
[17:22] <rogpeppe> dimitern: looking
[17:22] <rogpeppe> dimitern: oh darn, you've merged trunk
[17:22] <dimitern> rogpeppe: what?
[17:22] <rogpeppe> dimitern: i'm seeing lots more diffs than i should
[17:23] <rogpeppe> dimitern: between patchset 4 and 5
[17:23] <dimitern> rogpeppe: when i proposed it the diff was screwed, so i repropsed
[17:24] <dimitern> rogpeppe: i haven't merged trunk
[17:24] <rogpeppe> dimitern: ah, sorry, i thought it was the other CL again!
[17:24] <dimitern> rogpeppe: np
[17:28] <rogpeppe> dimitern: can't we just delete the id argument to api.State.Machiner, as we discussed?
[17:29] <rogpeppe> dimitern: reviewed
[17:32] <dimitern> rogpeppe: not unless we need to test it
[17:33] <dimitern> rogpeppe: and i'd like to have a test for id != ""
[17:33] <rogpeppe> dimitern: you could use Call directly for that
[17:33] <rogpeppe> dimitern: that's the kind of thing it's there for
[17:33] <dimitern> rogpeppe: that's seems wrong
[17:34] <rogpeppe> dimitern: it seems wrong to me to clutter the user-facing client API with meaningless parameters
[17:34] <rogpeppe> dimitern: you could put another method in export_test.go if you don't want to use Call
[17:34] <dimitern> rogpeppe: well, ok, it's a compromise I can live with for now :)
[17:34] <rogpeppe> dimitern: in fact there are quite a few methods we should probably test with non-nil ids (the Client entry points, for example)
[17:35] <dimitern> rogpeppe: agreed
[17:35] <rogpeppe> dimitern: so a table-driven test using Call could work ok there
[17:35] <dimitern> rogpeppe: in the apiserver you mean?
[17:36] <rogpeppe> dimitern: i was thinking in the api client, but you could do server tests instead, yes
[17:37] <dimitern> rogpeppe: that'll be better I think
[17:37] <dimitern> rogpeppe: so that we don't need to expose them all the way to the client
[17:38] <rogpeppe> dimitern: if you use Call, you wouldn't have to
[17:38] <rogpeppe> dimitern: but i'm ok either way. this is trivial functionality and not actually important in any way that i can really think of.
[17:39] <dimitern> rogpeppe: I can add a state_test.go in the api client and test with call
[17:39] <dimitern> rogpeppe: then just drop it and add a todo instead?
[17:39] <dimitern> rogpeppe: to do it in a follow-up
[17:39] <rogpeppe> dimitern: seems ok to me
[17:41] <dimitern> rogpeppe: ok then
[17:54] <dimitern> I'm off to pack
[17:54] <dimitern> good night all!
[17:59] <andreas__> why do I need to add the "local:" prefix when deploying a charm from my disk? Isn't --repository /foo/bar/ specific enough?
[18:49] <fwereade__> andreas__, --repository just overrides the value of $JUJU_REPOSITORY, which might always be set, but doesn't always imply "look in a local repo" unless specified via the charm schema
[18:51] <fwereade__> andreas__, I see your point, but I think it's a bit cleaner to have --repository and JUJU_REPOSITORY match in usage as well as meaning, rather than layer on extra cleverness in --repository that doesn't really fir with the env var
[19:10] <ahasenack> fwereade__: --repository /foo/bar charm-name has surprising results
[19:18] <rogpeppe> g'night all
[21:10] <thumper> morning
[21:12] <hatch> morning
[21:12] <hatch>  /afternoon
[21:12] <hatch> :)
[21:35]  * thumper is now heads up after email reading
[21:35] <thumper> hmm... coffee
[22:35] <fwereade__> thumper, want to chat about the provisioner briefly?
[22:46] <fwereade__> thumper, short version: yes, the machines watcher can be expected to deliver an initial event that represents the complete set of known machines
[22:48] <fwereade__> thumper, subsequent events delivered are relative to that base "change"
[22:48] <fwereade__> thumper, if any machine enters the set, you'll be informed
[22:48] <fwereade__> thumper, if any machine in the set becomes dying, you'll be informed
[22:49] <fwereade__> thumper, if any machine in the set is removed *or becomes Dead*, you'll be informed, and will receive no further notifications for that id
[22:50] <fwereade__> thumper, calling AllMachines just raises questions of raciness that are *probably* ok but not trivial to analyse
[22:53] <fwereade__> thumper, and it seems unnecessary since we have this type sitting there that's designed to send precisely the information necessary to maintain a consistent view of the set of machines in play over the lifetime of the watcher
[23:07] <thumper> fwereade__: oh hai
[23:07] <thumper> fwereade__: was taking the dog for a walk
[23:09] <thumper> fwereade__: given this knowledge then, I'll refactor a little...
[23:09] <fwereade__> thumper, nice weather for it?
[23:14] <thumper> fwereade__: is actually, sunny and cool (well cold)
[23:14] <thumper> but nice
[23:15] <thumper> given what you have said about the watcher
[23:15] <thumper> I think we can get rid of the "all machines" bit
[23:15] <fwereade__> thumper, I'm starting to think wistfully of that sort of wather
[23:15] <fwereade__> thumper, cool, I'm glad that makes sense
[23:15] <fwereade__> thumper, I think it's a strict simplification
[23:15] <thumper> yeah
[23:15] <thumper> shouldn't be too hard
[23:15] <thumper> rogpeppe had a lot of good changes for loggo too
[23:16] <thumper> but includes breaking API
[23:16] <fwereade__> thumper, awesome
[23:16] <fwereade__> thumper, bah
[23:16] <thumper> so would need to land branches in parallel
[23:16] <thumper> because we don't have library dependencies :)
[23:16]  * fwereade__ sighs wistfully
[23:17] <thumper> perhaps soon
[23:17] <thumper> maybe
[23:23] <wallyworld> fwereade__: i was going top ask you about the machineContainer stuff but it's very late for you
[23:23] <fwereade__> wallyworld, I responded to your CL I think, saying essentially machineContainer is good but quibble quibble quibble bikeshed bikeshed
[23:24] <thumper> :)
[23:24] <wallyworld> fwereade__: yeah, saw that thanks. question though - if we have fast regex queries, then there's really no need for the children slice
[23:24] <fwereade__> wallyworld, I've realised there is, I think
[23:25] <wallyworld> ok. i notice you also nixed by surrogate key :-)
[23:25] <fwereade__> wallyworld, when we're able to watch an index of documents we care about, in a single document, we get to avoid watching a whole collection
[23:25] <fwereade__> wallyworld, grep for globalKey in state
[23:26] <wallyworld> ok
[23:26] <fwereade__> wallyworld, no objection to the concept, but would prefer to maintain convention when context does not specifically indicate otherwise
[23:26] <arosales> davechaney around?
[23:26] <wallyworld> thanks for the input. will push up something today
[23:27] <wallyworld> fwereade__: also, i'll think about the txrevno - i do think it is required since the documwent has to be loaded, modified, and saved again
[23:27] <wallyworld> and that is not atomic
[23:28] <wallyworld> i think settings uses it for the same reason
[23:29] <wallyworld> thumper: you ok to +1 that clean flag branch?
[23:29] <fwereade__> wallyworld, the model is more for a client to make exactly the document change it needs, asserting only the minimal conditions that must pertain to maintain consistency in state
[23:29] <fwereade__> wallyworld, settings is crazy
[23:30] <thumper> wallyworld: I can take a look
[23:30] <wallyworld> fwereade__:  ok. i'll have to think about it some more. i can't tight now see how we can do it without the possibility of two threads adding a container and stomping on each other
[23:31] <wallyworld> thumper: i didn't change anything, just answered your question
[23:31] <thumper> wallyworld: I'd have to page in the information, so can't +1 just yet
[23:31] <wallyworld> sure
[23:31] <thumper> hence needing to look at it again :)
[23:31] <thumper> my brain holds only so much in active state :)
[23:34] <fwereade__> wallyworld, so, briefly, there are mongo ops like $addToSet that we can use in transactions
[23:35] <wallyworld> ok, will look into that
[23:35] <wallyworld> thanks
[23:47] <fwereade__> wallyworld, yeah, $addToSet and $pull are used on the Principals and Subordinates fields that this is exactly analogous to
[23:48] <wallyworld> fwereade__: i also have the issue that the machineContainers doc might not exist yet
[23:48] <wallyworld> so i need to create sometimes and update via $addToSet other times
[23:49] <wallyworld> so there's still a potential race condition
[23:50] <wallyworld> with Principals, the doc always exists
[23:50] <thumper> wallyworld: I'm out for a bit, back hopefully in 30-40 minutes
[23:50] <fwereade__> wallyworld, for simplicity you could just create it alongside the machine doc, earlier in the ops list, and remove it later in its removal ops, such that the existence of a machine document always implies the existence of the containers doc?
[23:51] <wallyworld> fwereade__: thought about that but seems very wasteful since the machine may not even have a container added and we may hav lots of machines?
[23:51] <fwereade__> wallyworld, if you ever get a mongo NotFound from such an ancillary document, you can infallibly infer that the parent document was removed
[23:52] <fwereade__> wallyworld, they're tiny documents
[23:52] <wallyworld> ok. i'll create one regardless when a machine is created. it feels very dirty to me to have to do that though
[23:55] <fwereade__> wallyworld, I infer that given http://docs.mongodb.org/manual/faq/developers/#how-do-i-optimize-storage-use-for-small-documents -- in which it recommends the possibility of noticeably reducing overhead by using shorter field names -- that we're not going to be paying a significantly greater storage cost by moving som of those field names to a distinct document with a clear purpose
[23:56] <wallyworld> ok. thanks. i'll read that faq also
[23:56] <fwereade__> wallyworld, I would very much prefer not to go down the route of mangling our field names for the db's convenience until I get evidence that it's a source of trouble for us
[23:57] <fwereade__> ;p
[23:57] <wallyworld> i never suggested we mangle field names i don't think
[23:57] <fwereade__> wallyworld, no, I wasn't trying to imply you were
[23:58] <wallyworld> ah ok :-)
[23:59] <fwereade__> wallyworld, I'm saying that if the few bytes per document to store the string keys, which will indeed be somewhat repetitive, are a significant enough factor to recommend tweaking, the additional db-imposed overhead is going to be of a similar order of magnitude for that sort of change to be effective