[13:20] wrtp: did you see my mail about the machineswatcher ? [13:21] sorry, firsly, how was your flight ? [13:21] davecheney: no i didn't. i've just arrived back. will have a look. [13:21] i've managed to blow up my cobzr repo [13:21] so i've lost all my old branches [13:21] but i'll get them back and port the machines watcher tests [13:21] davecheney: fine. a bit crowded at lisbon. got to the front of an hour long queue wait and they said "you could have used the priority queue because you checked in online". grrr. [13:22] davecheney: oops. how did you manage that? [13:22] davecheney: are you still in lisbon BTW? [13:22] yeah, don't leave for hours [13:22] eating stale chips and a crap club sandwitch [13:23] wrtp: bzr branch url:/// [13:23] not lp:... [13:23] will do a remote branch [13:23] and it screwes up cobzr [13:23] i did it yesterday, have a look in the channel log for the gorey details [13:23] davecheney: oops [13:23] davecheney: did you post to juju-dev ? [13:23] y [13:23] lucky(~/devel/juju-core) % bzr switch 111-util-quango [13:23] bzr: ERROR: Not a branch: "/home/dfc/devel/juju-core/.bzr/cobzr/000-foo/ [13:23] /". [13:24] ^ i'm tyring to fix it, but the .bzr/branch/location file cannot have a \n at the end of the line ... [13:24] davecheney: ah, found it [13:24] davecheney: yeah, definitely needs more tests! [13:24] i'm concerned the machines watcher doens't work properly [13:24] which is sorta important for the PA :) [13:25] davecheney: mere technicalities! [13:26] i know [13:26] such trivialities [13:29] oh brilliant, there were no tests for the old machines watcher ... [13:30] * davecheney le sigh [13:38] davecheney: have you looked in watcher_test.go [13:38] ? [13:39] wrtp: maybe i missed it, but there are no tests that call WatchMachines [13:39] davecheney: state_test.go:/WatchMachines [13:40] in the old codebase or the new ? [13:40] davecheney: old codebase [13:40] davecheney: it's true they're fairly poxy tests [13:41] ahh yes [13:41] i did miss it [13:41] well, i'll start with that [13:41] davecheney: i'm not sure the table-driven test adds much there tbh [13:41] and indeed that is present in the new codebase [13:41] I wonder if it's actually testing anything [13:42] davecheney: doesn't look implausible, but it could do with doing many more machines. [13:42] davecheney: it occurs to me that a StopSync might be useful [13:43] davecheney: then we can easily simulate multiple changes arriving at once. [13:43] what does Start/Stop sync do ? [13:43] davecheney: StartSync triggers a db poll [13:43] does it disconnect the watcher from the underlying even stream ? [13:43] so stopSync, change some stuff, start sync == big change [13:43] davecheney: yup [13:44] ok, i'll keep looking [13:44] davecheney: we *can* do that now, but it's non-deterministic because the sync might happen while we're making the changes [13:44] the PA is just acting like it never sees any machines changes at all [13:44] davecheney: have you put a log statement in? [13:44] davecheney: just to make sure [13:44] they were there from the odl days [13:44] but it's been a while [13:45] i'll keep digging [13:45] hmm, no logging in the normal path [13:45] only the error path [13:45] that would be a good place for me to start [13:46] wrtp: how was customs ? [13:46] should I get a move on [13:46] davecheney: fine, but quite slow again [13:46] davecheney: depends very much on how many people are going through at that time, of course [13:47] i should probably go then [13:57] wrtp: interesting [13:57] [LOG] 97.37511 JUJU:DEBUG watcher: got request: watcher.reqWatch{key:watcher.watchKey{c:"machines", id:interface {}(nil)}, info:watcher.watchInfo{ch:(chan<- watcher.Change)(0xf840151eb0), revno:0}} [13:57] [LOG] 97.37554 JUJU rcvd machines change: &{[] []} [13:57] davecheney: just thought - does it call AllMachines? [13:58] davecheney: 'cos AllMachines is horribly broken [13:58] wrtp: yes, but only after actioning machine additions [13:59] davecheney: AllMachines only fills in the machine id, nothing else. [13:59] wrtp: ok, i'll be careful of this [13:59] but this is happening before then [14:00] in worker/provisioner:loop() [14:00] we wait on the watcher [14:00] and this is the change that is received [14:03] oops, just almost ran out of battery [14:06] davecheney: the WatchMachines test in the new state seems much better than the old one [14:06] davecheney: though it doesn't check what happens when there's an initial machine. [14:07] wrtp: yeah, there are a few bugs like that [14:07] expecting addmachine to give you machine 1, not machine 0 [14:07] i think we talked about this yesterday [14:07] because of the bootstrap rquirements [14:08] davecheney: it looks like it works though [14:09] davecheney: yeah we did [14:09] davecheney: but after chatting about it with gustavo, we decided to keep things the way they were [14:09] davecheney: i.e. the dummy provider bootstrap does not allocate a machine and instance [14:12] wrtp: sounds sane [14:13] and predictable [14:13] davecheney: i dunno. given that every real provider will have (at least) machine 0 allocated at bootstrap time, i sway towards the thought that dummy should too [14:14] davecheney: but it does require quite a few tests to be fixed [14:16] wrtp: i fear that is mocking too much [14:16] davecheney: i definitely see that POV too [14:22] the magic that happens behind the scenes in jujud bootstrap is unfortunate [14:22] because we either have to ignore it in the tests [14:22] or mock it [14:22] which is brittle [14:27] davecheney: gotta go now. hope you survive your flights ok. [14:28] davecheney: see you whenever you manage to surface! [14:29] wrtp: cool [14:29] i'll keep working on this [14:29] btw,m should MachinesWatcher ever return a change with empty Added and Removed ?