| thumper | wallyworld__: taking the dog for a walk in the hope she'll go outside... | 00:27 |
|---|---|---|
| wallyworld__ | ok | 00:27 |
| thumper | geez luise | 01:20 |
| 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:21 |
| thumper | wallyworld__: wow some of our code is wonderfully inefficient | 01:25 |
| 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:42 |
| thumper | I'm busy teasing apart dependencies | 01:43 |
| wallyworld__ | i find not having generics leads to a *lot* of code duplication | 01:48 |
| thumper | I really wish our state methods would return interfaces rather than real structs | 01:50 |
| thumper | makes mocking and testing a PITA | 01:50 |
| wallyworld__ | oh yes +100 to that one | 01:52 |
| 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:53 |
| wallyworld__ | \o/ | 01:54 |
| thumper | wallyworld__: OMFG | 03:21 |
| thumper | bigjools: ok, can have that talk now | 03:29 |
| 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:41 |
| wallyworld__ | cool, i'll look | 03:42 |
| 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:29 |
| 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:30 |
| wallyworld__ | https://plus.google.com/hangouts/_/d3f48db1cccf0d24b0573a02f3a46f709af109a6 | 04:31 |
| === thumper is now known as thumper-cooking | ||
| === tasdomas_afk is now known as tasdomas | ||
| rogpeppe1 | mornin' all | 06:54 |
| TheMue | rogpeppe1: heya | 07:02 |
| rogpeppe1 | TheMue: hiya | 07:02 |
| === thumper-cooking is now known as thumper | ||
| === TheRealMue is now known as TheMue | ||
| fwereade__ | TheMue, ping | 08:48 |
| TheMue | fwereade__: pong | 08:55 |
| TheMue | fwereade__: just wanted to ping roger ;) but will do it afterwards | 08:56 |
| TheMue | fwereade__: ah, it seems i found what i wanted to ask | 09:01 |
| fwereade__ | TheMue, would you please send thumper a link to your lxc work so he can maybe look through it tonight? | 09:02 |
| TheMue | fwereade__: sure, it's a very interesting topic | 09:03 |
| TheMue | fwereade__: will do immediately | 09:03 |
| TheMue | fwereade__: btw, watcher with test is working | 09:04 |
| dimitern | mgz: hey | 09:23 |
| mgz | dimitern: hey | 09:29 |
| dimitern | mgz: how can we go about pairing on implementing the deployer stuff? | 09:29 |
| mgz | I'm open to suggestions | 09:30 |
| mgz | one option is using my vm with go stuff on and screen or something, plus voice | 09:31 |
| dimitern | mgz: not sure i see the process will go - can you expand a bit? | 09:37 |
| dimitern | rogpeppe1: responed to https://codereview.appspot.com/10044043/ | 09:40 |
| 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:44 |
| dimitern | mgz: I see, well we can try it out | 09:49 |
| dimitern | just as soon as I finish responding to my reviews | 09:49 |
| mgz | sure | 09:52 |
| dimitern | rogpeppe1: responded to this as well https://codereview.appspot.com/10026044/ | 09:59 |
| dimitern | looking for reviews on this: https://codereview.appspot.com/9937045/ | 10:01 |
| dimitern | fwereade__: ^^ | 10:02 |
| 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:06 |
| 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:07 |
| mgz | what's the branch? | 10:08 |
| dimitern | lp:~dimitern/juju-core/055-api-machiner-subpackages | 10:08 |
| 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:09 |
| 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:10 |
| fwereade__ | dimitern, give me 5 mins | 10:11 |
| dimitern | fwereade__: sure | 10:11 |
| mgz | dimitern: can do, but mumble would work as well for this | 10:12 |
| 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:13 |
| 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:14 |
| mgz | fwereade__: can you install mumble quickly? | 10:15 |
| fwereade__ | mgz, dimitern, ok, I am here, I need the bathroom, I will try to get mumble up in a sec | 10:19 |
| dimitern | mgz, fwereade__: we can meet in the Blue room on mumble (Root -> Cloud Engineering -> Blue) | 10:20 |
| fwereade__ | cheers | 10:25 |
| 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:26 |
| jamespage | mgz, bug 1188126 | 10:32 |
| jamespage | https://bugs.launchpad.net/juju-core/+bug/1188126 | 10:32 |
| TheMue | *: any known problems with the .../worker tests? they timed out here when doing test ./... | 10:37 |
| mgz | jamespage: thanks! | 10:40 |
| TheMue | rogpeppe1: ping | 10:51 |
| rogpeppe1 | TheMue: pong | 10:51 |
| 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:52 |
| TheMue | rogpeppe1: yes, reproducable | 10:53 |
| rogpeppe1 | TheMue: this is with trunk? | 10:54 |
| 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:55 |
| TheMue | rogpeppe1: one moment | 10:57 |
| TheMue | rogpeppe1: strange | 10:58 |
| 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 | 10:59 | |
| 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 | FAILlaunchpad.net/juju-core/worker600.005s | 11:00 |
| 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:01 |
| 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:02 |
| 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:03 |
| TheMue | rogpeppe1: http://paste.ubuntu.com/5738426/ | 11:04 |
| TheMue | rogpeppe1: carmen just called me for lunch, biab | 11:04 |
| rogpeppe1 | TheMue: oh dammit | 11:05 |
| dimitern | rogpeppe1: https://codereview.appspot.com/10044043/ - need some help on this please | 11:05 |
| 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:06 |
| rogpeppe1 | dimitern: sorry for not responding earlier - i was in a call | 11:07 |
| dimitern | rogpeppe1: np | 11:07 |
| dimitern | rogpeppe1: i'm really bad at phrasing doc comments and will appreciate suggestions like for the Tagger | 11:08 |
| danilos_ | dimitern, wallyworld: hi, hangout? | 11:31 |
| wallyworld | ok | 11:32 |
| rogpeppe1 | dimitern: responded | 11:32 |
| === danilos_ is now known as danilos | ||
| dimitern | danilos_: uh, yes, forgot we're doing g+ now | 11:32 |
| dimitern | rogpeppe1: cheers | 11:32 |
| danilos | dimitern, https://codereview.appspot.com/9876043/ | 11:34 |
| dimitern | rogpeppe1: can we agree to disagree on the apiserver machiner_test? :) | 11:40 |
| dimitern | rogpeppe1: and it's value | 11:41 |
| dimitern | (that it's valueable) | 11:41 |
| 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:42 |
| 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:43 |
| 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:44 |
| 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:45 |
| 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:46 |
| 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:47 |
| 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:48 |
| 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:49 |
| 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:50 |
| 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:51 |
| 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:52 |
| 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:53 |
| 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:54 |
| dimitern | rogpeppe1, fwereade__: https://plus.google.com/hangouts/_/1842432ac5d47d4b006b6679b49cb3d836c8163f?authuser=0&hl=en | 11:55 |
| AeroNotix | When using the goose Nova binding, is the client expected to provide an imageRef themselves? | 12:28 |
| 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:29 |
| TheMue | *: one CL for review: https://codereview.appspot.com/10078043 | 12:30 |
| === BradCrittenden is now known as bac | ||
| === gary_poster|away is now known as gary_poster | ||
| dimitern | rogpeppe3: can you sketch out an idea of how requireMachiner should look like? | 13:50 |
| dimitern | rogpeppe3: not sure about the check "is machine" | 13:51 |
| rogpeppe3 | dimitern: http://paste.ubuntu.com/5738805/ | 13:51 |
| 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:52 |
| rogpeppe3 | dimitern: cool | 13:53 |
| dimitern | rogpeppe3: indeed | 13:54 |
| dimitern | rogpeppe3: and then there's this: https://codereview.appspot.com/10026044/ | 13:55 |
| 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:56 |
| 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:57 |
| rogpeppe3 | dimitern: ah. i thought the reason was security. | 13:58 |
| dimitern | rogpeppe3: that's the main part of it | 13:58 |
| 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 |
| === rogpeppe3 is now known as rogpeppe | ||
| dimitern | oh well.. kanban time | 13:59 |
| rogpeppe | dimitern: agreed. it depends how much code we're willing to write for that. | 14:00 |
| === wedgwood_away is now known as wedgwood | ||
| rogpeppe | time to reboot. this machine is becoming unusable. | 14:44 |
| === BradCrittenden is now known as bac_ | ||
| dimitern | rogpeppe: so wrt unifying ServerError and ServerErrorToParams | 14:58 |
| dimitern | rogpeppe: the only thing that needs wrapping is the param to conn.Serve in apiserver.go | 14:59 |
| 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:00 |
| 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:01 |
| 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:02 |
| 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:03 |
| 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:04 |
| 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:05 |
| 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:06 |
| rogpeppe | jtv: i haven't seen that failure before, but that test is quite new | 15:07 |
| * rogpeppe hates timing-sensitive tests | 15:08 | |
| jtv | rogpeppe hates with reason. | 15:08 |
| 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:09 |
| === racedo` is now known as racedo | ||
| 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:10 |
| 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:11 |
| rogpeppe | dimitern: you'll need to check if the error returns an error code | 15:12 |
| 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:13 |
| === bac_ is now known as bac | ||
| 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:14 |
| rogpeppe | dimitern: good point. i'm not sure that functionality *is* specifically tested. you'll probably want to add a test. | 15:15 |
| 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:16 |
| dimitern | rogpeppe: that's a direct translation of what happens when servererrortoparams is removed | 15:17 |
| dimitern | rogpeppe: and integrated into servererror | 15:17 |
| rogpeppe | dimitern: code will never be non-empty on line 17 | 15:18 |
| dimitern | rogpeppe: how did it work before? | 15:18 |
| rogpeppe | dimitern: it returned the error unwrapped | 15:19 |
| rogpeppe | dimitern: i think you want something more like this: http://paste.ubuntu.com/5739044/ | 15:20 |
| 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:21 |
| dimitern | rogpeppe: ok, sgtm | 15:24 |
| 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:42 |
| mgz | jtv: just the normal "you need to pull lp:goose" thing? trunk builds for me | 15:45 |
| 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:46 |
| 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:48 |
| jtv | mgz: yes, completely fresh — cloud instance | 15:49 |
| mgz | I'm on r95 "Add juju-tools keystone endpoint and fix tests" | 15:49 |
| jtv | Oh, of course I just discarded my instance. The compile error is on a different system. Sigh. | 15:50 |
| jtv | mgz: ah... I'm on a much older revision — "go get launchpad.net/goose" didn't update it apparently. | 15:51 |
| mgz | indeed not | 15:52 |
| jtv | mgz: apparently the branch URL has changed... how do I fix that? | 15:53 |
| 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:54 |
| 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:55 |
| 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:56 |
| mgz | timebased auth is much nicer | 15:57 |
| 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:58 |
| mgz | ^re-add | 15:59 |
| mgz | generate a new seed, register device with it, start on fresh sequence | 15:59 |
| 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:00 |
| rogpeppe | mgz: are you sure #webops is the place? perhaps #admin or something might be better? | 16:02 |
| gnuoy | rogpeppe, web0ps do 2fa resets, and they hangout in #web0ps | 16:03 |
| dimitern | gnuoy: tis with "0" (zero) or upper-case "O"? | 16:04 |
| 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:05 |
| 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:06 |
| gnuoy | dimitern, np, sorry for causing the confusion :) | 16:07 |
| 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:09 |
| jtv | dimitern: so far it's been _exactly_ like the maas provider. :) | 16:10 |
| fwereade__ | TheMue, ping | 16:10 |
| === BradCrittenden is now known as bac | ||
| dimitern | jtv: :) i meant in a separate branch | 16:14 |
| 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:15 | |
| dimitern | jtv: ah ok | 16:16 |
| dimitern | rogpeppe: https://codereview.appspot.com/10044043/ all done I think | 16:18 |
| 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:20 |
| rogpeppe | dimitern: reviewed | 16:33 |
| fwereade__ | danilos, ping | 16:33 |
| dimitern | rogpeppe: cheers | 16:34 |
| 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:37 |
| 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:38 |
| 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:39 |
| 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:47 |
| 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:48 |
| 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:49 |
| 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 | 16:50 |
| === tasdomas is now known as tasdomas_afk | ||
| 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:21 |
| 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:22 |
| rogpeppe | dimitern: between patchset 4 and 5 | 17:23 |
| dimitern | rogpeppe: when i proposed it the diff was screwed, so i repropsed | 17:23 |
| 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:24 |
| rogpeppe | dimitern: can't we just delete the id argument to api.State.Machiner, as we discussed? | 17:28 |
| rogpeppe | dimitern: reviewed | 17:29 |
| dimitern | rogpeppe: not unless we need to test it | 17:32 |
| 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:33 |
| 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:34 |
| 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:35 |
| rogpeppe | dimitern: i was thinking in the api client, but you could do server tests instead, yes | 17:36 |
| 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:37 |
| 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:38 |
| 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:39 |
| dimitern | rogpeppe: ok then | 17:41 |
| dimitern | I'm off to pack | 17:54 |
| dimitern | good night all! | 17:54 |
| andreas__ | why do I need to add the "local:" prefix when deploying a charm from my disk? Isn't --repository /foo/bar/ specific enough? | 17:59 |
| 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:49 |
| 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 | 18:51 |
| ahasenack | fwereade__: --repository /foo/bar charm-name has surprising results | 19:10 |
| rogpeppe | g'night all | 19:18 |
| === BradCrittenden is now known as bac | ||
| thumper | morning | 21:10 |
| hatch | morning | 21:12 |
| hatch | /afternoon | 21:12 |
| hatch | :) | 21:12 |
| * thumper is now heads up after email reading | 21:35 | |
| thumper | hmm... coffee | 21:35 |
| fwereade__ | thumper, want to chat about the provisioner briefly? | 22:35 |
| 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:46 |
| 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:48 |
| 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:49 |
| fwereade__ | thumper, calling AllMachines just raises questions of raciness that are *probably* ok but not trivial to analyse | 22:50 |
| 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 | 22:53 |
| === wedgwood is now known as wedgwood_away | ||
| thumper | fwereade__: oh hai | 23:07 |
| thumper | fwereade__: was taking the dog for a walk | 23:07 |
| thumper | fwereade__: given this knowledge then, I'll refactor a little... | 23:09 |
| fwereade__ | thumper, nice weather for it? | 23:09 |
| === wedgwood_away is now known as wedgwood | ||
| thumper | fwereade__: is actually, sunny and cool (well cold) | 23:14 |
| thumper | but nice | 23:14 |
| 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:15 |
| 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:16 | |
| thumper | perhaps soon | 23:17 |
| thumper | maybe | 23:17 |
| === wedgwood is now known as wedgwood_away | ||
| 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:23 |
| 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:24 |
| 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:25 |
| 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:26 |
| 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:27 |
| wallyworld | i think settings uses it for the same reason | 23:28 |
| 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:29 |
| 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:30 |
| 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:31 |
| fwereade__ | wallyworld, so, briefly, there are mongo ops like $addToSet that we can use in transactions | 23:34 |
| wallyworld | ok, will look into that | 23:35 |
| wallyworld | thanks | 23:35 |
| fwereade__ | wallyworld, yeah, $addToSet and $pull are used on the Principals and Subordinates fields that this is exactly analogous to | 23:47 |
| 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:48 |
| wallyworld | so there's still a potential race condition | 23:49 |
| 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:50 |
| === thumper is now known as thumper-afk | ||
| 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:51 |
| 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:52 |
| 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:55 |
| 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:56 |
| 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:57 |
| wallyworld | ah ok :-) | 23:58 |
| 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 | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!