/srv/irclogs.ubuntu.com/2012/09/26/#juju-dev.txt

niemeyerdavecheney: Yo!01:24
niemeyerdavecheney: Are you staying around for a bit?01:24
davecheneyniemeyer: yy01:25
niemeyerdavecheney: I guess the answer is no.. :)01:29
davecheneyyeah, i'm arround01:29
niemeyerdavecheney: Ah, hey :)01:29
niemeyerdavecheney: I'm about to get live tests working, and have a set of changes in the pipeline.. if you'll be around for a bit, we can quickly interact on those01:30
davecheneyniemeyer: excellent01:30
niemeyerdavecheney: It's all pretty agreeable stuff01:30
davecheneylet me know what you need me too do01:30
niemeyerdavecheney: Btw, I have a connected RPi turning a led on and off here :-)01:30
davecheneynice01:31
davecheneyniemeyer: http://dave.cheney.net/2012/09/25/installing-go-on-the-raspberry-pi01:31
niemeyerBoards with GPIO <301:31
davecheneyfinally wrote it up last night01:31
niemeyerdavecheney: Oh, sweet01:31
niemeyerdavecheney: Very cool01:33
niemeyerdavecheney: I got Go working yesterday too01:33
davecheneyodessa(~/devel/src/launchpad.net/juju-core/testing) % go test ../cmd/juju01:33
davecheney--- FAIL: TestPackage (0.00 seconds)01:33
davecheneymgo.go:65: exec: "mongod": executable file not found in $PATH FAIL01:33
davecheneyFAILlaunchpad.net/juju-core/cmd/juju0.116s01:33
davecheney^ with your review comments applied01:33
niemeyer?01:33
niemeyerdavecheney: I don't understand.. I didn't suggest any changes to the path or whatever?01:34
davecheneyniemeyer: nah, this is just responding to your comments in ,https://codereview.appspot.com/656004501:34
niemeyerdavecheney: Ah, ok, just showing how it looks like.. super, thanks01:34
niemeyerdavecheney: Phew.. I thought it was all broken :)01:34
davecheneynah, just testing that it does work as expected if you don't have mgo installed01:35
niemeyerdavecheney: Looks good01:36
davecheneyty01:38
niemeyerAmazon is taking ages to allocate a machine apparently.. and S3 is failing.. not a good night to test it live01:55
niemeyerAnyway, I'll break the CLs down01:55
niemeyerhttps://codereview.appspot.com/657205002:00
davecheneyniemeyer: Proposal: https://code.launchpad.net/~dave-cheney/goetveld/002-add-darwin-termios-support/+merge/12636202:02
davecheneyfor some reason goetveld doesn't create CL's, only LP reviews02:02
davecheneyniemeyer: try another zone ?02:03
davecheneyi ofter use southeast-102:03
niemeyerhttps://codereview.appspot.com/657305002:04
niemeyerhttps://codereview.appspot.com/657904302:09
niemeyererror: Failed to update merge proposal log: EOF02:25
niemeyerMaybe it's my connection that is in a bad state02:25
niemeyerhttps://codereview.appspot.com/656704802:25
niemeyerdavecheney: Last for the day ^02:25
davecheneyniemeyer: i haen't seen that one for a while, the EOF02:26
davecheneyreviewing last CL now02:26
niemeyerShower.. will pass by in a bit..02:37
niemeyerdavecheney: Thanks for the reviews!02:37
davecheneykk02:39
davecheneyniemeyer: no worries02:39
niemeyerdavecheney: Okay, I guess I'll merge this stuff02:50
niemeyerdavecheney: So people can make progress tomorrow on top of it02:50
niemeyerdavecheney: How's config-get going there?02:50
davecheneyniemeyer: dunno, i should pull that branch and see if it compiles02:51
niemeyerdavecheney: I mean the stuff that you were pushing forward since the sprint02:51
niemeyerdavecheney: Or what have you been up to?02:51
davecheneyi've been on swap days since the sprint02:53
niemeyerToday as well?02:53
niemeyerdavecheney: ?02:54
davecheneyniemeyer: getting yelled at from downstairs, back in a while02:56
=== davecheney is now known as davecheney-a6faf
TheMuemorning05:56
davecheney-a6fafmorning05:57
=== davecheney-a6faf is now known as davecheney-bb1ad
fwereademornings06:57
davecheney-bb1adfwereade: http://paste.ubuntu.com/1228005/07:01
davecheney-bb1addid I just break transactions ?07:01
fwereadedavecheney-bb1ad, huh, not seen that before07:02
fwereadedavecheney-bb1ad, er, maybe? :)07:02
davecheney-bb1adfwereade: shitter07:02
fwereadedavecheney-bb1ad, but I actually have no idea07:02
fwereadedavecheney-bb1ad, what did you do? :)07:02
davecheney-bb1adjust what I typed07:03
davecheney-bb1adthat is trunk07:03
TheMuefwereade: morning07:03
fwereadeTheMue, heyhey07:03
=== davecheney-bb1ad is now known as davecheney
fwereadedavecheney, bah, I'll try myself07:03
davecheneyhttps://bugs.launchpad.net/juju-core/+bug/105664207:05
davecheneyfor posterity07:05
fwereademroning rogpeppe07:21
rogpeppefwereade: hiya07:22
TheMuerogpeppe: morning07:22
davecheneyfwereade: running those commands in a loop07:24
davecheneyfwereade: another failure07:52
fwereadedavecheney, oh dear :( same one?07:52
davecheneydifferent07:52
davecheneyhttp://paste.ubuntu.com/1228059/07:52
fwereadedavecheney, hmm, is that just ec2 taking too long?07:53
davecheneypossibly, but we should be able to cope with that07:54
davecheneylucky(~/src/launchpad.net/juju-core/cmd/juju) % juju destroy-environment07:55
davecheneylucky(~/src/launchpad.net/juju-core/cmd/juju) % bash -x stress.bash07:55
davecheney+ set -e07:55
davecheney+ true07:55
davecheney+ juju bootstrap --upload-tools07:55
davecheney+ juju deploy mongodb07:55
davecheneyerror: no instances found07:55
davecheneythat happened in 2 minutes07:55
davecheneyless07:55
davecheneyour timeout is 10 minutes07:55
davecheneylucky(~/src/launchpad.net/juju-core/cmd/juju) % bash -x stress.bash08:01
davecheney+ set -e08:01
davecheney+ true08:01
davecheney+ juju bootstrap --upload-tools08:01
davecheney+ juju deploy mongodb08:01
davecheney+ juju status08:01
davecheneyerror: instance i-bbd62fc6 for machine 1 not found08:01
davecheneyhttps://bugs.launchpad.net/juju-core/+bug/105667908:02
fwereadebizarre :(08:08
TheMueha!08:12
TheMuecatched the service bug in firewaller, phew08:12
TheMuehmm, still sometimes have a race *sigh*09:07
davecheneyrogpeppe: http://paste.ubuntu.com/1228172/09:49
davecheney^ evil, or very evil ?09:49
Arammoin09:50
rogpeppedavecheney: why?09:50
davecheneythe py tests fro juju ssh were using this weird mocking thing09:51
davecheneyI needed a way to catch the ssh arguments, without running the command09:51
rogpeppedavecheney: the way i was doing that before was by changing $PATH09:52
rogpeppedavecheney: is that not reasonable? (it doesn't involve changing the actual code at all)09:52
davecheneyi guess I want to test that the code is generating the correct invovation09:53
rogpeppedavecheney: i realise it's more work though09:53
davecheneyso I need the args09:53
TheMueAram: hi09:53
rogpeppedavecheney: you can get the args. just make the ssh into a shell script that saves the args.09:53
davecheneyrogpeppe: sure, that would work, but then I have to write that stuff to disk, blah blah09:54
rogpeppedavecheney: i thought we were doing something like this anyway09:54
rogpeppedavecheney: we do, in TestSSHErrors09:55
davecheneyfair point09:55
rogpeppedavecheney: i don't think it's much more hassle to get the script to save its args.09:56
rogpeppedavecheney: or just use a different script template09:56
rogpeppedavecheney: i'm not that keen on mocking Command.09:56
davecheneyfair enough'09:57
rogpeppedavecheney: just change the script to do something like this: for i in "$@"; do echo $i >> {{.Dir}}/args; done09:59
rogpeppefwereade: do you find the "JUJU:DEBUG watcher: loading new events from changelog collection..." messages useful?10:34
rogpeppefwereade: i'm inclined to delete them - they're so noisy.10:34
fwereaderogpeppe, sometimes; I would be quite keen on a package-specific debug flag that defaults to false though10:34
rogpeppefwereade: how would you turn it on?10:35
Aramrogpeppe: good catch about nil != empty on golang-dev. I was like "wtf is this" but didn't say anything because I assumed I missed some subtlety and I was wrong.10:35
fwereaderogpeppe, watcher.SetDebug(true) in the setups of tests that I thought it would help in debugging10:35
Aram:)10:35
Aramyes, delete the damn verbosity.10:36
rogpeppefwereade: or even just watcher.Debug = true :-)10:37
fwereaderogpeppe, indeed :)10:37
fwereadeAram, btw, what's the priority on the confignode read-missing change?10:41
Aramfwereade: ?10:42
AramI'm missing something :)10:42
Aramwhat's wrong10:42
fwereadeAram, sorry, I thought I overheard you discussing it with niemeyer with a vague view to doing it soon10:42
fwereadeAram, I mean changing CondfigNode such that an attempt to read an empty one returns an error10:43
fwereadeAram, sorry, a *missing* one10:43
fwereadeAram, empty is fine10:43
Aramhmm.10:43
AramI'll take a look.10:44
Aramthe behavior used to be like you described.10:44
fwereadeAram, honestly I'm easy either way, I think it would be a good change but not an overwhelmingly awesome one10:44
fwereadeAram, I just wanted to know the plan because I'm about to fiddle around with  one of the clients10:44
Aramthe behavior should be like you described, if it is not, I'll do it after I do the machine units watcher.10:45
Aramwhich I'll start after lunch.10:46
Aramwhich is now10:46
Aram:L)10:46
rogpeppefwereade: what's the current status of the uniter w.r.t. lifecycle? does it watch for Dying?10:59
fwereaderogpeppe, nope, no handling at all10:59
fwereaderogpeppe, that would be a good thing to add, actually :)10:59
TheMuelunchtime (did i already say i hate testing for races *grmpf*)11:00
rogpeppefwereade: where would you fit the watcher in? Uniter.loop is sooo neat currently...11:00
rogpeppefwereade: hmm, i suppose it could just be a separate thing that kills the watcher, perhaps with a known error.11:01
fwereaderogpeppe, I was expecting all the steady state modes to watch for it11:01
fwereaderogpeppe, maybe I can have occasional checks at transition time too11:01
* fwereade shrugs11:02
rogpeppefwereade: is there anything mode-specific that needs to be done at shutdown time?11:03
fwereaderogpeppe, yes, all sorts of watcher stops, generally11:03
rogpeppefwereade: those will happen anyway when the uniter tomb is killed, no?11:03
fwereaderogpeppe, s/anyway/assuming the existence of a new entity whose responsibility it is to do that on unit death/11:05
rogpeppefwereade: ah, of course, the watchers are independent of the modes11:05
fwereaderogpeppe, sorry I think I am miscommunicating11:05
fwereaderogpeppe, yes, when we return from a mode, its watchers will be cleaned up; there are no watchers outside modes at the moment11:06
rogpeppefwereade: that's what i thought originally11:06
rogpeppefwereade: so...11:06
fwereaderogpeppe, but 2/3 of the steady state modes already watch the unit11:06
rogpeppefwereade: why do we need a new entity to stop watchers on unit death?11:07
fwereaderogpeppe, we don't11:07
fwereaderogpeppe, we just need to use unit watchers in the modes11:07
fwereaderogpeppe, no call at all to mess around with uniter.loop IMO11:07
rogpeppefwereade: i'm wondering if there's any reason to clutter up every single mode with dying logic11:07
fwereaderogpeppe, well, yes11:08
fwereaderogpeppe, usually Dying is of no concern at all11:08
rogpeppefwereade: when we can have an entirely separate entity that just kills the uniter when life changes to state.Dying11:08
fwereaderogpeppe, whoa how do yo propose to do that?11:08
fwereaderogpeppe, we don;t want to *kill* the uniter11:08
rogpeppefwereade: no?11:08
fwereaderogpeppe, we enter an extended and detailed shutdown sequence in which lots of things happen11:08
fwereaderogpeppe, when we hit Dead is a different matter11:09
rogpeppefwereade: ah, ok. that's what i meant by "is there anything mode-specific that needs to be done at shutdown time?"11:09
fwereaderogpeppe, but if you're in a hook error state it matters not one whit whether you're alive or dying11:09
fwereaderogpeppe, you wait for that user resolution, like it or not11:09
rogpeppefwereade: does that mean we can't remove units that are in an error state?11:09
fwereaderogpeppe, there will be mode-specific subtleties though11:10
fwereaderogpeppe, yes11:10
fwereaderogpeppe, unless you -force11:10
fwereaderogpeppe, eg, a hook error state should probably just ignore charm upgrades while dying11:10
rogpeppefwereade: ok, this all makes sense. not a trivial change then :-)11:10
fwereaderogpeppe, afraid not :)11:11
fwereaderogpeppe, I am slightly worried that all the lifecycle logic will rather dirty up the nice clean uniter11:12
rogpeppefwereade: i *think* that it'll just result in another couple of modes11:13
rogpeppefwereade: and some logic in the existing modes to make that transition11:13
fwereaderogpeppe, I'm talking more about responses to life checks in the various watchers within the modes11:13
fwereaderogpeppe, I worry that they will obscure the rest ;)11:14
rogpeppefwereade: i guess we'll find out...11:14
fwereaderogpeppe,  yeah :)11:15
rogpeppefwereade: do you think it's reasonable that Refresh return a *NotFoundError when the entity has been removed?11:31
fwereaderogpeppe, hmm, yeah, I think so11:32
fwereaderogpeppe, we'll see how it is in practice11:32
Aramfwereade: rogpeppe: since now we are only returning Ids, what do you say about the idea of only returning two kinds of changes, one for ints and one for strings, instead of a custom change type for each watcher.12:47
* fwereade kinda has a sad about this12:47
fwereadegenerally speaking, when I get a change I really *like* being able to do something useful with it12:48
Aramso you don't like that we are only returning ids?12:48
fwereadeAram, I guess I need more context; which watchers?12:49
Aramall of them.12:49
Aram:)12:49
Aramlook at how machines watcher is now.12:49
Aramit returns12:50
Aramtype MachinesChange struct {12:50
AramAlive []int12:50
AramDead  []int12:50
Aram}12:50
Aramall the others will return struct { Alive, Dead []string }12:50
fwereadeAram, yeah, I do appreciate the new structure... but, hmm, nothing needs Dying?12:50
Aramother watchers sure need Dying.12:51
Aramunsure about this.12:51
Aramniemeyer wrote it12:51
fwereadeAram, I thought we agreed a while back that we'd be supplying a single list of those entities whose life status had changed?12:51
fwereadeAram, I'm not 100% sure that's right, but I'm sketchy on just having Alive/Dead fields12:52
Aramyeah, I remember we agreed on that long ago, but I also remember we agreed on this more recently.12:52
Aram"agreed"12:53
fwereadeAram, blast, I missed that conversation12:53
fwereadeAram, (sure, I am aware that agreement is a fluid that changes its nature as it flows within a group)12:53
Aramif it were my choice, I'd just return map[Life]Id, because we might add more life states in the future12:53
Arammap[Life][]Id12:54
Aramso they are grouped by life12:54
Aramand you can have as many life states as you want12:54
fwereadeAram, that sounds good to me12:54
Arambut it's not my choice :).12:54
fwereadeAram, ...although...12:54
fwereadeAram, I kinda feel that reporting by status is somewhat against the spirit of the ids-only change12:55
fwereadeAram, really just `Changed []Id` seems best12:55
AramI agree in principle.12:56
fwereadeAram, ok back to concrete :)12:56
fwereadeAram, I want to watch a service's relations, and know when they are dying12:56
fwereadeAram, am I now expected to set up an individual Dying watch, per relation that I detect alive?12:57
Aramno12:57
fwereadeAram, how can I avoid this?12:57
Aramyou'll get a ServiceRelationWatcher which will return struct { Alive, Dying []string }12:58
fwereadeAram, ok, so each change type is tailored to its clients, and the Alive/Dead thing is not a brook-no-exceptions Agreement12:59
fwereadeAram, that SGTM then :)12:59
Arambut struct { Alive, Dying []string } is hardly tailored to its client, almost all watchers will use it.13:00
fwereadeAram, I still don't like the amount of reloading I'll be doing on doc watches...13:01
Aramyeah, I agree in principle. simpler watcher are good, but when they put a burden on the client the fact that they are simple means nothing.13:02
fwereadeAram, on the basis that the ids change will be really ugly and inconvenient for the doc watchers' only clients... as it will be I think for ConfigNode watches... would you hold off on doing them until you've changed the other ones?13:02
Aramsure.13:02
fwereadeAram, I will try to remember to bring it up with niemeyer this pm13:02
fwereadeAram, cheers13:03
fwereadeAram, and for the relations, I should be thinking in terms of lists of relation IDs only?13:03
fwereadeAram, (grouped as Alive/Dying ofc)13:03
Aramwhatever you want, but the primary key is the string formed from the endpoints and that has the advantage that you can extract information from it without having to load the document.13:04
fwereadeAram, hmm; do we have a Relation accessor on state that accepts that key?13:05
fwereadeAram, I don't think we do...13:06
Aramsurprisingly no, though State.Relation is close in spirit13:07
Aramsince the key is the stringified endpoints13:07
fwereadeAram, I think we have some thinking do do13:07
fwereadeAram, *lossily* stringified endpoints, I think13:08
fwereadeAram, we can't reconstruct the originals without hitting state13:08
Aramyeah, we can't ATM13:08
fwereadeAram, and that will involve figuring stuff out by getting information from the charms13:08
fwereadeAram, which I'm pretty sure we don't want to do because it will break all our tests13:09
fwereadeAram, (because most tests just slap a dummy charm into state, which doesn't declare any relations)13:10
niemeyerHello all!13:11
Aramhi13:11
fwereadeniemeyer, heyhey13:11
TheMueniemeyer: hiya13:13
niemeyerHow's the day looking?13:15
niemeyerGOod stuff?13:15
niemeyer> db.mycoll.insert({_id: 1})13:16
niemeyerduplicate key insert for unique index of capped collection13:16
niemeyer> db.foo.insert({_id: 1})13:16
niemeyerE11000 duplicate key error index: test.foo.$_id_  dup key: { : 1.0 }13:16
niemeyerIt's unfortunate that these are different errors13:17
* niemeyer reports upstream13:18
* TheMue still fights with a race condition13:30
Aramniemeyer: do we return dying machines in the initial event of the machine units watcher?13:40
Aramor do we now.13:40
Arams/now/not/13:40
Aramreturning dying machines makes it consistent with Machine.Units13:40
niemeyerAram: Dying, yes13:40
niemeyerAram: The only thing that cares about an Alive => Dying transition is the entity agent itself13:41
niemeyerAram: For all other purposes, the thing is still alive13:41
Aramniemeyer: so you're fine with UnitsChange being13:42
Aramtype UnitsChange struct {13:42
AramAlive []string13:42
AramDying []string13:42
Aram}13:42
Aram?13:42
niemeyerAram: s/Dying/Dead/?13:42
fwereadeAram, surely that's an Alive/Dead one13:42
niemeyerYeah13:42
fwereadeAram, ServiceRelations is Alive/Dying, because while the watching entity is not istelf a relation, it *is* responsible for responding to the  relation's lifecycle changes13:43
fwereadeAram, sorry that was not a helpful sentence13:44
Aramhow does one get Alive -> Dying if we don't deliver this event here? one watcher per each unit?13:44
fwereadeAram, I think the question here is entirely situational, and dependent on what the client needs to use13:44
fwereadeAram, the MPW only needs to about Alive (to deploy a container) and Dead (to destroy it)13:45
Aramok, that seems sensible.13:45
fwereadeAram, the Uniter is responsible for watching the unit for Dying, and then shutting itself down in an orderly fashion before making itself Dead13:46
niemeyer_Weird.. that was an abrupt "disconnection by peer"13:46
fwereadeAram, the Uniter is also responsible for handling everything about relations, and itself sets Dead, so it should only need Alive/Dying13:46
fwereadeAram, from SRW13:47
niemeyer_fwereade: That said, hmm13:47
fwereadeniemeyer_, I personally would prefer just []ChangedLife13:47
fwereadeniemeyer_, and handle it appropriately at the various call sites13:48
niemeyer_fwereade: Wouldn't it be weird.. let's say.. for a machine watcher to report a unit as alive when it's dying..13:48
fwereadeniemeyer_, which will I suspect contain a number of subtle differences13:48
niemeyer_fwereade: just so it starts up, and dies13:48
Aramit seems kind of wrong to return Dying units inside the Alive field of UnitsChange though.13:48
fwereadeniemeyer_, so a unit that is initiall seen to be Dying should be ignored, and should not generate a Dead13:49
niemeyer_fwereade: Kind of..13:49
niemeyer_fwereade: That's even more incorrect13:49
niemeyer_fwereade: imagine the same situation, but the unit is actually still running13:49
fwereadeniemeyer_, ha13:50
rogpeppeniemeyer_, fwereade: fairly trivial: https://codereview.appspot.com/656405413:50
niemeyer_rogpeppe: Thanks13:50
fwereadeniemeyer_, wait, surely the machine agent *knows* what is running anyway?13:50
niemeyer_fwereade: I think something along your ChangedLife idea might be plausible13:50
niemeyer_fwereade: Forces acknowledgement of the possibilities13:50
fwereadeniemeyer_, but regardless, yeah, I feel that's the way to go for now13:50
niemeyer_fwereade: Or even Alive/Dying/Dead fields13:51
niemeyer_fwereade: Which makes the need for handling even more explicit13:51
fwereadeniemeyer_, I'd prefer to keep it just as "changed!", rather than risk a lie (when the id is loaded and revealed to be in a different state)13:51
niemeyer_fwereade: Good point13:52
fwereadeniemeyer_, on a related note: ids from watchers13:52
niemeyer_fwereade: ok13:52
Aramso Changed  is the consesnsus?13:53
fwereadeniemeyer_, I like it in principle, and (I think) in practice for the collection watchers; I feel that it's going to be unhelpful when applied to the document watchers13:53
niemeyer_Aram: It may be just a slice of ids, I guess, but let's wait until the end of the conversation13:53
fwereadeniemeyer_, as the only client so far, every time I get a changed service or unit, I want it refreshed13:53
niemeyer_fwereade: There are a few different details that have been going unperceived13:54
fwereadeniemeyer_, can I handwave single document watchers to be "different enough" as to be sent as objects?13:54
fwereadeniemeyer_, go on13:54
niemeyer_fwereade: 1) We're faking the data for the entity; since the entity might not be there by the time we try to fetch it, we change its field to Dead and send a cached version13:55
niemeyer_fwereade: Which is quite wrong.. the unit may have died in an entirely different state13:56
niemeyer_fwereade: and such a Dead + arbitrary state may never have happened.. the justification for the death is in the unit that we didn't see (unit being just an example here)13:56
niemeyer_fwereade: 2) If we get 100 changes between the last time we've observed the unit, and the next time we're able to handle the change because e.g. the hook returned,13:57
niemeyer_fwereade: we're reloading the unit 100 times within the watcher, for absolutely no reason13:57
fwereadeniemeyer_, re (1), I dunno -- for what clients is the distinction important?13:58
niemeyer_fwereade: I don't know.. we'll likely find out when something explodes in our face13:58
fwereadeniemeyer_, re (2), hmm, true13:58
niemeyer_fwereade: 3) The cost of loading the unit is minimal in all cases I've ported13:58
fwereadeniemeyer_, I mean, I've been thinking about it, and I can't see any -- doesn't Dead mean "the only safe way to interact with this document is to destroy it"?13:59
fwereadeniemeyer_, yeah, it is only a few lines of code13:59
niemeyer_fwereade: and, interesting, in some cases it cleaned up..13:59
niemeyer_fwereade: The firewaller, for example, didn't really care much about that machine object13:59
niemeyer_fwereade: It ended up not using it at all in most cases13:59
niemeyer_fwereade: It was using it just because it was being handed off anyway13:59
fwereadeniemeyer_, I'm 100% behind it on the collections13:59
fwereadeniemeyer_, and I think I'm now convinced on the documents side14:00
niemeyer_fwereade: 4) It makes the watchers simple (!) :-)14:00
fwereadeniemeyer_, ;)14:00
niemeyer_fwereade, Aram: So, slice of ids?14:02
fwereadeniemeyer_, +114:02
AramChanged []string?14:02
niemeyer_Aram: []string14:02
* rogpeppe wishes that machine ids were strings too14:02
Aramhmm14:02
niemeyer_Aram: <-chan []string14:03
Aramrogpeppe++14:03
Aramniemeyer_: ok, that seems fine14:03
niemeyer_rogpeppe: We should talk about that someday when we're not in the middle of a big change14:05
rogpeppeniemeyer_: yeah14:05
niemeyer_rogpeppe: Reviewed14:10
=== niemeyer_ is now known as niemeyer
rogpeppeniemeyer: thanks14:10
rogpeppeniemeyer: i chose FitsTypeOf because then if the test fails it'll say what the failing error actually is14:11
niemeyerrogpeppe: If the test fails it's trivial to find out what's wrong14:11
rogpeppeniemeyer: ok14:11
niemeyerrogpeppe: That's how it's being done in the rest of the code already14:11
niemeyerrogpeppe: We have a test for IsNotFound that verifies it actually works as intended, and the rest is relying on it workng14:12
rogpeppeniemeyer: that's fine. i generally to try to make test failures as informative as i can, but i'm happy to use IsNotFound too14:12
niemeyerrogpeppe: That's a great approach, but there's value in testing the semantics we want.. we'll generally be using IsNotFound in client code, and it should work14:14
rogpeppeniemeyer: ok, but i thought that's what the IsNotFound test is for.14:14
rogpeppeniemeyer: anyway, i've changed it already14:14
niemeyerrogpeppe: Nevermind. Thank you!14:15
rogpeppeand one other occurrence too14:15
TheMuehave to leave due to an emergency, bbl14:23
niemeyerTheMue: Ouch.. hope it's all good there14:27
jamespageniemeyer, hey! available for a chat about versions of mongodb in quantal?  I understand 2.2.0 is desired....15:06
fwereadeniemeyer, whoops, I need to leave early today... I might have a CL for you later, but nothing yet I'm afraid15:06
niemeyerjamespage: Yo, here15:07
niemeyerfwereade: np, have a pleasant time there15:07
fwereadeniemeyer, Aram: ooo, one important thought: to make the ids change work with RelationUnitsChange et al, we will need a map[unit-name]txn-revno15:08
niemeyerfwereade: Hmm15:08
fwereadeAram, niemeyer, actually, I don;t think we do15:08
jamespageniemeyer, I had a request to up the version of mongodb in quantal to 2.2.0 to support go-juju15:08
niemeyerfwereade: That's good I guess, because I have no idea about what you have in mind yet :D15:09
fwereadeniemeyer, we just clear the settings out when we get a change15:09
jamespageniemeyer, currently we will be shipping 2.0.6 (maybe 2.0.7 if I get time)15:09
niemeyerjamespage: Col15:09
niemeyerjamespage: Cool15:09
niemeyerjamespage: 2.2.0 would be good indeed15:09
fwereadeniemeyer, I was thinking we'd need to keep track of revnos at the watcher level because they're used by the relation context level15:09
niemeyerjamespage: Very good, in fact15:09
jamespageniemeyer, at the moment I'm pushing back - its very late in the cycle15:09
fwereadeniemeyer, in persistent relation state storage15:09
jamespageniemeyer, I'm assuming that this will be required to support usage on precise as well15:10
niemeyerfwereade: Right, I think that's sensible15:10
niemeyerjamespage: In principle, no15:10
jamespageniemeyer, ??15:10
niemeyerjamespage: Well, it depends a bit on what you mean by that15:10
niemeyerjamespage: Support usage in which sense?15:11
niemeyerfwereade: Let's have a chat once you have some more time15:11
jamespageniemeyer, say if someone want to use go-juju on 12.0415:11
fwereadeniemeyer, but we don't: we can treat change notifications as clear-your-settings; and if we return a txnRevno from ReadSettings, we can persist *that*15:11
niemeyerfwereade: I'd appreciate the insight there15:11
jamespagerather than 12.10 ++15:11
fwereadeniemeyer, although it may require unhelpful extra plumbing15:11
fwereadeniemeyer, I need to thing it through a bit more15:11
fwereadeanyway gtg, take care & have fun15:12
niemeyerfwereade: Sounds sensible I think15:12
niemeyerfwereade: Cool, ttyl15:12
niemeyerjamespage: Hm15:12
niemeyerjamespage: SO,15:12
niemeyerjamespage: So,15:12
niemeyerjamespage: There are tons of very attractive changes on 2.2.015:13
niemeyerjamespage: Things like improved concurrency support, an aggregation framework that enables queries that are impossible otherwise, speed ups, etc etc15:13
niemeyerjamespage: We want to benefit from those15:13
niemeyerjamespage: That said,15:14
niemeyerjamespage: We can benefit from those either way, in the server side15:14
niemeyerjamespage: and we have other reasons to be working around the stock packaging in those cases15:14
jamespageniemeyer, sounds like you have issues with the distro packaging then?15:15
jamespageother than the version...15:15
niemeyerjamespage: No, it's really the version15:15
niemeyerjamespage: The problem is that we have to support N releases of Ubuntu with the same code15:16
niemeyerjamespage: Back and forth15:16
jamespageniemeyer, my suggestion is that we take the same approach as you guys did with zookeeper before15:16
jamespagei.e. ship it in PPA alongside go-juju15:16
niemeyerjamespage: not only that, but we need to make sure that upgrades don't happen without our explicit approval15:16
niemeyerjamespage: Since an upgrade of the core MongoDB behind juju's back might kill the whole deployment15:17
niemeyerjamespage: Well, sure.. if we have to workaround it, we can do that by several means15:18
niemeyerjamespage: The point, though, is that we'd benefit from having the new version on the distro15:18
niemeyerjamespage: So that people can more easily deploy juju *locally*15:18
niemeyerjamespage: For development, testing, etc15:19
niemeyerjamespage: Without having to go around the distro packages15:19
jamespageniemeyer, I would agree with that IF go-juju was going to land in the distro for quantal15:19
niemeyerjamespage: If it's not doable, I'll understand, and will start thinking of plan-b15:19
jamespageniemeyer, I'm happy to push time at upgrading mongodb to 2.2.0 next cycle15:20
niemeyerjamespage: If it's not in Quantal, it'll be there a bit afterwards15:20
jamespageyes15:20
niemeyerjamespage: It'll still come to life during Quantal's lifetime15:20
niemeyerjamespage: That said, we don't have local support yet, so perhaps you're right and it doesn't matter for this cycle15:20
jamespageniemeyer, I just think the overall risk this late in cycle is to great for a number of reasons15:21
jamespage1) none of the server team have much experience with mongodb or its packaging (it being in universe)15:21
niemeyerjamespage: Understood, sounds good.. I'd probably be causing you pain for no good reason15:21
jamespage2) limited time with charm sprints and ODS in the next two weeks...15:22
jamespageniemeyer, OK - I'll keep you notified of any work that happens15:22
jamespageand will make sure its back-portable to precise/quantal15:22
niemeyerjamespage: Thanks, and sorry for the half-false alarm15:22
jamespageniemeyer, no problemo15:23
niemeyerjamespage: Can we put this on the agenda for the next release?15:23
jamespageniemeyer, absolutely - if mongodb is going to support juju going forward that it need to be MIR'ed and more owned in ubuntu anyway15:23
jamespageowned/loved....15:24
niemeyerjamespage: +115:24
niemeyerjamespage: Meanwhile, FYI we've built a version of 2.2.0 and are shipping alongside juju in a bare-bones fashion (bins only)15:24
niemeyerjamespage: This means we're able to deploy in all releases of Ubuntu we want to support15:25
jamespagecoolio15:25
niemeyerjamespage: Precise, Quantal, Quantal+1, etc15:25
jamespageniemeyer, all archs as well?15:25
niemeyerjamespage: Yep, if/when needed15:26
jamespageniemeyer, good-oh15:26
niemeyerjamespage: tools/mongo-2.2.0-precise-amd64.tgz15:26
jamespageI know quite a bit of testing has been happening using juju/maas on arm for example15:26
niemeyerjamespage: I don't have any experience with mongo on arm, though15:27
niemeyer"juju-core has no active code reviews."15:38
niemeyerIt's been a while since I've seen that message..15:38
niemeyerBtw, we've topped almost 6k lines in during the sprint15:40
niemeyerand 11k lines *out*15:40
niemeyer(thanks to zk's departure)15:41
niemeyerDidn't take long, though: https://codereview.appspot.com/657404915:48
Aramfwereade: "15:53
Aram<fwereade> niemeyer, Aram: ooo, one important thought: to make the ids change work with RelationUnitsChange et al, we will need a map[unit-name]txn-revno"15:53
Aramdo we need this?15:53
niemeyerAram: I don't know.. have to talk to William later about details15:53
niemeyerAram: Does it make a difference for you now?15:53
AramI think it would be easier to adapt later if we need to.15:54
niemeyerAram: Are you working on RelationUnitsChange?15:54
Aramno.15:55
AramI was thinking in general.15:55
niemeyerAram: Okay, so that's not needed. RelationUnitsChange is very special in that regard15:55
Aramabout this.15:55
Aramok.15:55
Aramsuits me.15:55
niemeyerAram: :-)15:55
niemeyerrogpeppe: Have you evolved/tried live tests today?15:59
rogpeppeniemeyer: yeah, i've got a branch that is stubbornly refusing to work live. each time it's something a little bit different.16:00
niemeyerrogpeppe: :(16:00
niemeyerrogpeppe: We need to polish those live tests.. the organization right now is pretty confusing, there are bits in ec2 that should be in jujutest, unrelated tests are being done together, etc16:01
rogpeppeniemeyer: i agree16:01
rogpeppeniemeyer: which bits are you thinking can move from ec2 to jujutest, BTW?16:01
rogpeppeniemeyer: i'm starting to think that my policy of always returning a non-nil Tools is not a good one16:05
rogpeppeniemeyer: i think that if the tools haven't been set, AgentTools should return nil and NotFoundError16:06
rogpeppeniemeyer: this is an error i get when calling juju status on the current test environment:16:06
rogpeppe2012/09/26 16:59:33 JUJU:DEBUG juju status command failed: cannot get all machines: invalid binary version ""16:07
rogpeppeerror: cannot get all machines: invalid binary version ""16:07
niemeyerrogpeppe: That's what I'd expect too16:07
niemeyerrogpeppe: (as a client of that API)16:07
niemeyerrogpeppe: Hmm.. are you missing an update from trunk?16:07
niemeyerrogpeppe: I'm running an env here.. status works fie16:08
niemeyerfine16:08
rogpeppeniemeyer: i don't *think* so, unless it's a very recent one16:08
niemeyerrogpeppe: Last evening16:08
niemeyerrogpeppe: I've fixed the marshalling of tools16:08
rogpeppeniemeyer: no, i've merged trunk16:09
niemeyerrogpeppe: Ok, so worth investigating..16:09
niemeyerrogpeppe: Hmm.. wouldn't that happen if the machine was retrieved before it got an agent set?16:10
rogpeppeniemeyer: that's what i'm thinking16:10
niemeyerrogpeppe: the machine sets its own agent on startup16:10
rogpeppeniemeyer: yup16:10
niemeyerrogpeppe: So that must be it16:10
rogpeppeniemeyer: that's why i thought moving to using a nil tools might help16:10
rogpeppeniemeyer: but maybe it'd call SetBSON on the toolsDoc.Version anyway?16:11
niemeyerrogpeppe: What does "using a nil tools" mean?16:11
rogpeppeniemeyer: hmm, now i look, i'm not sure :-)16:13
rogpeppeniemeyer: it seems to me that the tools *will* be nil in a newly created machine doc.16:13
niemeyerrogpeppe: I think Tools is just missing the SetZero return if the value is bson's null16:13
rogpeppeniemeyer: ah, you're probably right.16:14
niemeyerrogpeppe: I'm always probably right.. I'm often probably wrong too.. :-)16:14
niemeyerrogpeppe: I thought you were referring to this logic earlier:16:14
niemeyer        if m.doc.Tools == nil {16:15
niemeyer                return &Tools{}, nil16:15
niemeyer        }16:15
niemeyerThis is  bogus16:15
niemeyerIt should return nil, notFound("tools for machine %v", m.doc.Id)16:15
rogpeppeniemeyer: yes, that's what i was saying above16:15
niemeyerrogpeppe: Cool, so two bugs16:15
niemeyerBtw, I found out why it's taking a bit to init mongo.. it's preallocing the journal16:16
niemeyerI'll find a way to work aroudn it16:16
rogpeppeniemeyer: that would be great16:17
rogpeppeniemeyer: how do you find if a bson.Raw is a null?16:18
niemeyerrogpeppe: Compare raw.Kind against a constant.. I don16:18
niemeyerrogpeppe: 't recall which, but I think we're already doing this elsewhere..16:18
niemeyerrogpeppe: Otherwise,16:18
niemeyerrogpeppe: bsonspec.org16:18
niemeyerLunch!16:18
rogpeppeniemeyer: enjoy!16:18
rogpeppeniemeyer: ah! i see why the state tests passed now...16:33
rogpeppec.Skip("Marshalling of agent tools is currently broken")16:33
rogpeppehmm, no, it passes16:34
niemeyerrogpeppe: Try to Marshal, Unmarhsal, re-Marshal, and re-Unmarshal16:44
niemeyerrogpeppe: I suspect the third will fail16:44
niemeyerrogpeppe: Sorry, the fourth oe16:45
niemeyerrogpeppe: The third will save the bogus empty tools16:45
niemeyerrogpeppe: Fourth will blow up16:45
rogpeppeniemeyer: how does that sequence happen in our current code? i'm not sure we ever Marshal the whole machine doc16:47
rogpeppeniemeyer: i'm trying to write a test that currently fails16:47
niemeyerrogpeppe: The suggested sequence will fail, I suspect16:48
rogpeppeniemeyer: Marshal isn't available externally to state16:48
niemeyerrogpeppe: It marshals when it writes to the database16:48
niemeyerrogpeppe: Ah, it's probably GetBSON that is broken too16:49
rogpeppeniemeyer: i don't *think* it marshals when it writes to the db16:49
niemeyerHmm, or maybe not..16:49
niemeyerrogpeppe: Huh?16:49
niemeyerrogpeppe: There's no way for mgo to send anything to the database without marshaling16:50
rogpeppeniemeyer: well, it marshals the agent tools when they're set, but given that they're set, it should all work ok16:50
niemeyerrogpeppe: That's where the sequence above comes from.. due to the lack of SetZero on SetBSON, it will marshal empty tools16:51
rogpeppeniemeyer: even if the tools aren't empty?16:51
niemeyerrogpeppe: It's the same thing we just talked above16:51
niemeyer<niemeyer> rogpeppe: I think Tools is just missing the SetZero return if the value is bson's null16:51
rogpeppeniemeyer: i can see how we get an invalid tools in the machineDoc, but i'm not sure how we can get that marshalled again.16:52
rogpeppeniemeyer: because AFAICS we never marshal from the machine doc's Tools16:53
rogpeppeniemeyer: obviously you're right in one sense, but i don't quite see yet exactly how it happens. i'm probably being stupid, sorry.16:54
niemeyerrogpeppe: You're not.. I don't know either16:55
niemeyerrogpeppe: let's see the code that failed16:55
niemeyerrogpeppe: Well, actually, can you push the WIP branch?16:56
niemeyerrogpeppe: Otherwise I have no idea about what *actually* failed16:56
rogpeppeniemeyer: ok, i'll do that16:56
rogpeppeniemeyer: https://codereview.appspot.com/656605816:58
niemeyerCheers16:58
niemeyers16:58
AramI'm preparing some roast pork, but I'll be back after all the stuff is in the oven16:59
rogpeppeniemeyer: i'm running the live tests again to check if i get the same symptom17:03
rogpeppeAram: my mouth waters17:04
niemeyerrogpeppe: If you get the error, see if you can look at the logs17:04
rogpeppeniemeyer: i looked at the logs before and saw nothing unusual17:04
niemeyerrogpeppe: That test environment, are you sure you bootstrapped it with up-to-date tools?17:05
rogpeppeniemeyer: i'm doing go test -amazon in environs/ec217:05
rogpeppeniemeyer: that should be sufficient17:05
niemeyerrogpeppe: Because that's very similar to the bug I fixed yesterday17:06
rogpeppeniemeyer: i'm pretty sure i'm using the up to date tools17:07
rogpeppeniemeyer: well, the tools from the source in the CL above17:07
niemeyerOk17:07
niemeyerrogpeppe: Ahh, I think the mystery is solved17:13
* rogpeppe is all ears, having just reproduced the problem17:13
niemeyerrogpeppe: txn has to re-marshal the doc on insertion to inject the key17:13
niemeyerrogpeppe: Well, actually.. hmm17:13
niemeyerrogpeppe: Nevermind.. makes no sense17:13
rogpeppedarn17:14
niemeyerrogpeppe: The remarshalling will not use the type17:14
niemeyerrogpeppe: Do you have a live env with it?17:14
rogpeppeniemeyer: yes17:14
niemeyerrogpeppe: Can I get access?17:14
rogpeppeniemeyer: yeah, one mo17:14
niemeyerrogpeppe: There's a handy tool to import ssh keys from lp17:15
rogpeppeniemeyer: don't i need to give you an ssh key?17:15
niemeyerrogpeppe: ssh-import-id17:15
niemeyerrogpeppe: I have one :)17:15
rogpeppeniemeyer: yeah, but the environment won't be authorized for your key17:16
niemeyerrogpeppe: ssh-import-id17:16
rogpeppeniemeyer: or... can i add keys dynamically to a machine in the environment?17:17
rogpeppeniemeyer: is that what you're suggesting?17:17
niemeyerrogpeppe: Yes, you can log into the machine, and run ssh-import-id17:17
niemeyerrogpeppe: ssh-import-id niemeyer, more precisely17:17
rogpeppeniemeyer: ok, i'll try that17:17
rogpeppeniemeyer: try now: ec2-23-20-216-248.compute-1.amazonaws.com17:19
niemeyerI'm in17:19
niemeyerrogpeppe: Tools versions in the env look fine17:20
niemeyerrogpeppe: Is a "juju status" against it breaking?17:20
rogpeppeniemeyer: yeah17:20
niemeyerrogpeppe: Hah, nice.. that's going to be easy17:20
rogpeppeniemeyer: but all juju status does is call AllMachines17:21
niemeyerrogpeppe: How do you call juju status?17:21
rogpeppeniemeyer:  juju status --debug -e sample-b765066ea6aab3bb17:22
niemeyerrogpeppe: COol17:22
rogpeppeniemeyer: (it helps that i log the environment public attrs in the test now)17:23
rogpeppeniemeyer: out of interest, what mongo command did you use to inspect the db?17:25
niemeyerrogpeppe: "mongo"17:25
niemeyerrogpeppe: "mongo localhost:37017/juju"17:26
niemeyerrogpeppe: More precisely17:26
rogpeppeniemeyer: i got that far.17:26
rogpeppeniemeyer: ah, got it. it's a literal "db".17:28
niemeyerrogpeppe: db.machines.find()17:28
rogpeppeniemeyer: i thought i had to type juju.machines.find(017:28
rogpeppe)17:28
niemeyerrogpeppe: I can't reproduce the error without some further hacking17:29
niemeyerrogpeppe: Can you please enable the mgo logs, and run juju status with them on17:29
niemeyerrogpeppe: mgo.SetDebug(true); mgo.SetLogger(log.Target)17:29
rogpeppeniemeyer: ok17:31
niemeyerrogpeppe: Sent a few comments on https://codereview.appspot.com/6566058/17:33
rogpeppeniemeyer: ah! you caught the bug i was actually trying to fix. thanks!17:34
rogpeppeniemeyer: i'm so f*!#ing blind17:34
rogpeppeniemeyer: i'm glad we found this in the process though17:35
niemeyerrogpeppe: +1, and don't blame yourself too much.. it's a single char.. very easy to miss17:35
rogpeppeniemeyer: ok, so i'm an idiot as suspected17:36
rogpeppeniemeyer: and you were exactly right. i was conflating two problems and assuming they were linked17:37
rogpeppeniemeyer: the "juju" i was running to run status was not the current juju. doh!17:37
rogpeppeniemeyer: and v sorry to waste your time17:37
rogpeppeniemeyer: it all works17:37
niemeyerrogpeppe: Woohay!17:38
rogpeppepwd17:38
niemeyer/home/rog17:38
rogpepperm -rf *17:38
niemeyerrm: command not found17:39
rogpeppegive up in disgust17:39
niemeyerrogpeppe: :-)17:39
niemeyerrogpeppe: Those debug sessions are nice because we get some intimacy with the whole process17:42
rogpeppeniemeyer: that's a nice slant to put on it :-)17:42
rogpeppeniemeyer: right, i'm running the live tests again, and i hope and trust they will work this time17:42
rogpeppeniemeyer: i've got to go and cook keema muttar curry :-)17:43
niemeyerrogpeppe: Have good fun there17:43
rogpeppeniemeyer: the "does nothing" channel range loop BTW is so that in a subsequent branch i can just slot in a Unit and watch it in the same way17:44
rogpeppeniemeyer: that was the reason for my "i wish machine ids were strings" remark earlier17:44
niemeyerrogpeppe: Ok, thanks17:44
rogpeppeniemeyer: i'll pop back in 10 minutes or so and see if the test succeeded17:45
niemeyerrogpeppe: Super, fingers crossed17:45
niemeyerrogpeppe: Btw, I've put your review fixes here: https://codereview.appspot.com/657404917:46
TheMueso, back, will join full in a few moments18:07
rogpeppeniemeyer:18:20
rogpeppe08.43.171 PASS18:20
rogpeppe08.43.176 ok  launchpad.net/juju-core/environs/ec2521.154s18:20
niemeyer!!!18:20
rogpeppe:-)18:20
* niemeyer dances the funky chicken18:20
niemeyerrogpeppe: https://codereview.appspot.com/656906018:21
niemeyerrogpeppe: This will speed the next one up18:21
* niemeyer loves "watcher was stopped cleanly" panics19:15
niemeyerSo many background activity in tests caught up on it19:15
niemeyerok      launchpad.net/juju-core/cmd/jujud       13.860s19:38
* fwereade cheers at niemeyer19:49
niemeyerfwereade: Hey! Good timing..19:49
fwereadeniemeyer, heyhey19:49
niemeyerfwereade: With this change, the only tests that seem broken here are the uniter ones19:49
niemeyerfwereade: But I thought you had fixed those.. is the change pending?19:49
fwereadeniemeyer, oh? which ones?19:49
fwereadeniemeyer, would you paste them to me, it should all work...19:50
niemeyerfwereade: Cool19:50
niemeyerfwereade: http://paste.ubuntu.com/1229166/19:50
niemeyerfwereade: I haven't investigated, so could be something trivial19:51
fwereadeniemeyer, that is odd, it looks kinda familiar, but I forget what was involved19:51
fwereadeniemeyer, I'll grab a fresh trunk and try to repro19:51
niemeyerfwereade: Not sure if it matters, but this was part of a full round run19:52
niemeyer2012/09/26 16:56:23 JUJU Unit status is "started"20:04
niemeyer2012/09/26 16:56:23 JUJU Built files published at http://ec2-177-71-232-16.sa-east-1.compute.amazonaws.com20:04
niemeyer2012/09/26 16:56:23 JUJU Remember to destroy the environment when you're done...20:04
niemeyererror: Failed to update merge proposal log: EOF20:12
niemeyerWhy do you hate me LP20:12
niemeyerOne more change up: https://codereview.appspot.com/656505220:13
niemeyerAnother one: https://codereview.appspot.com/656705420:19
niemeyerI guess I'll just bundle them up in a mail later.. the interactive reviewing isn't working today :)20:20
fwereadeniemeyer, https://codereview.appspot.com/6565052/ LGTM20:28
fwereadeniemeyer, and the other too20:32
niemeyerfwereade: Danke!20:34
fwereadeniemeyer, bitte!20:34
fwereadeniemeyer, btw, what's your current thinking on the necessity of a corrective agent?20:37
fwereadeniemeyer, I'm feeling like I want one :)20:38
niemeyerfwereade: Seems unnecessary20:38
niemeyerfwereade: What are you missing?20:38
fwereadeniemeyer, ok, the situation that makes me want it is as follows20:38
fwereadeniemeyer, the Uniter has detected a Dying unit and discharged all its responsibilities20:38
fwereadeniemeyer, it calls EnsureDead() on the unit20:38
fwereadeniemeyer, at this point, all bets are off20:39
niemeyerfwereade: In which sense?20:39
fwereadeniemeyer, in an ideal world, the uniter would smoothly delete the unit, and also the service (if it's dying and this was the last unit)20:39
fwereadeniemeyer, in the pessimistic case, it will be half way through doing that when the machine agent nukes the container and the unit agent with it20:40
fwereadeniemeyer, I think *something* needs to be capable of cleaning up after that20:40
niemeyerfwereade: The uniter should never delete a unit20:41
niemeyerfwereade: In my view so far, at least20:41
fwereadeniemeyer, ah, ok then, that's for the machiner to do once the unit is dead?20:41
niemeyerfwereade: It's work ends at EnsureDead20:41
fwereadeniemeyer, that makes sense to me20:41
niemeyerfwereade: Yes, once the machiner gets rid of the unit container, it deletes the unit20:41
fwereadeniemeyer, and once the machiner has cleaned up the container, it completely trashes the unit and possibly the srvice20:42
fwereadeniemeyer, cool20:42
niemeyerfwereade: As we've talked a few times, Dead is always handled by the "lifecycle manager" so to speak20:42
fwereadeniemeyer, what happens when the machiner is AWOL?20:42
niemeyerfwereade: The machiner<>provisioner relationship is the same20:42
niemeyerfwereade: The same as usual20:43
fwereadeniemeyer, that sounds sensible20:43
niemeyerfwereade: When the machiner handles the dead unit, it may also handle the service transparently20:43
niemeyerfwereade: We should probably put that logic within RemoveUnit itself20:44
fwereadeniemeyer, +1, but worrying about races20:44
fwereadeniemeyer, I expect we can make it work, we just need to be careful20:45
niemeyerfwereade: I think the idea we discussed at the sprint, with a refcount, works well20:45
fwereadeniemeyer, (and, to clarify: when a machiner is down it never removes the unit, and we sit and wait until someone fixes it?)20:45
niemeyerfwereade: yep20:45
fwereadeniemeyer, ok, cool20:46
niemeyerfwereade: Things should work in expected ways during shutdown.. juju will not kill resources up without user consent20:46
fwereadeniemeyer, permanently blocking a terminate-machine because of a dead unit (that's not being picked up due to a screwed machiner) seems a touch user-hostile if they don;t have the tools to resolve it20:47
fwereadeniemeyer, a Dying unit is one thing, they can resolve that20:47
niemeyerfwereade: We're not blocking because of a dead unit.. we're blocking because of a non-responding machiner20:49
niemeyerfwereade: I want to know why the machiner is not responding, rather than swiping that kind of error under the covers20:50
fwereadeniemeyer, yeah, good justification20:50
* fwereade is happy he doesn't have to worry directly about cleaning up services too :)20:51
fwereadenot right now, anyway ;)20:51
niemeyerfwereade: yeah :)20:51
niemeyerReview queue is owned today: https://code.launchpad.net/juju-core/+activereviews20:53
niemeyerfwereade: I'll have to step out for an errand.. back later, but hopefully you'll be resting by then.. have a good time there20:54
fwereadeniemeyer, I'll almost certainly have a WIP branch I'd appreciate a look at... I'll mail you if I manage it20:54
fwereadeniemeyer, cheers20:54
fwereadeniemeyer, https://codereview.appspot.com/6575053 is (I think) reasonably complete, and does still pass all existing tests21:39
fwereadeniemeyer, still WIP, but I intend to write actual tests tomorrow morning and submit something pretty close, so preliminary comments would be much appreciated21:41
niemeyerfwereade: Awesome!23:01
niemeyerfwereade: Will have a look23:01

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!