davecheneythumper: re last nights discussion of pacakging00:35
davecheneycan you outline the concerns of the packager's ?00:36
thumperprimarily it seemed to be that we weren't following company standards00:36
davecheneyof that I have no doubt00:36
davecheneybut that isn't very actionable00:37
davecheneyi'm guessing the lack of build reproducability is a red flag00:38
thumperI'm going to email people concerned to see if we can get help from people who do know the standards00:38
thumperone guy is even in sydney, so pairing might be good if you are interested00:38
thumperI'll keep you in the email loop00:39
thumperdavecheney: can I get you to merge that packaging branch of mine? then I can move the card to done02:14
bigjoolsthumper: how was your foray into packaging?02:22
thumperbigjools: brief02:22
davecheneythumper: i have some questions about that change02:25
thumperdavecheney: shoot02:26
davecheneysee review02:26
thumperdavecheney: replied02:39
* thumper is EODing early due to overly long meeting last night02:39
* thumper will look back in later02:41
davecheneythumper: fair enough, that makes sense02:42
jtvAnyone else getting this build error?  go/src/launchpad.net/juju-core/cmd/juju/constraints.go:66: undefined: results03:25
jtvLooks like a :=/= typo.03:25
* thumper tries03:25
davecheneyjtv: which revno ?03:25
jtvThe latest.03:26
jtvI'm also getting this one, which I guess is good news:03:26
jtv    // A failure here indicates that goyaml bug lp:1132537 is fixed; please03:26
jtv    // delete this test and uncomment the flagged constraintsRoundtripTests.03:26
davecheney+1 for being unhelpful :)03:26
thumper+1 for getting that03:26
thumperdavecheney: I get it with tip03:26
thumperjtv: yes03:26
davecheneyconfirmed, # launchpad.net/juju-core/cmd/juju03:26
davecheneycmd/juju/constraints.go:66: undefined: results03:26
davecheneycmd/juju/constraints.go:66: cannot assign to results03:26
davecheneycmd/juju/constraints.go:67: undefined: results03:26
davecheneywill submit a fix03:26
jtvAh thanks.03:26
thumperplz fix03:26
davecheneywhat the fuck people03:26
thumperwe just talked about this last night03:27
thumperit worked on my commit03:27
thumperyep, last commit added that03:27
* davecheney sends a shittyram03:27
thumperjtv: perhaps he is sending it to mramm03:28
thumperor he is sending a pooey male sheep03:28
jtvthumper: that's why I asked :-P03:28
jtvConfucius say, man-sheep eat from curry tree, he be shitty ram for a day03:29
davecheneysorry, type03:29
davecheneyi meant shittyHAM03:29
jtvWhy didn't you SAY so03:29
davecheneyit's my accent03:29
thumperdavecheney: I think the whole = / := thing is horrible03:29
thumperand needs a rethink03:29
thumperso many problems come about from it03:29
jtvFWIW I guess you _can_ create this kind of bug just from concurrent commits.03:30
jtvThe shadowing is really horrible though.  Makes you wonder what's so horrible about a mere unused variable.03:30
jtvfoo, err := bar()03:30
jtvif err != nil { panic(err) }03:30
jtvif foo {03:31
jtvsklurg, err := szot()03:31
jtvOops now I've created a second err.03:31
thumperjtv: I don't think you have03:31
jtvThat's what I thought.03:32
jtvIt'll shadow quietly.03:32
thumperit isn't shadowed03:32
thumperit is using the same err03:32
thumpera new sklurg is created though03:32
thumperhmm, inside the if though, new scope and all...03:33
thumperI've not tested...03:33
thumperI don't actually know what is happening there03:33
thumperperhaps davecheney does03:33
davecheneydavecheney: knows what ?03:33
jtvthumper: I do know _this_ happens: http://play.golang.org/p/uV__rKVATq03:34
jtvWoo!  Test suite completed with just 1 compile error and 1 test failure.03:35
thumperdavecheney: you have a local packaging branch...?03:35
thumperdavecheney: https://code.launchpad.net/~thumper/juju-core/package/+merge/157020 has instructions at the top03:35
thumperdavecheney: bzr merge ...03:35
thumperdavecheney: bzr commit... bzr push03:35
thumperdavecheney: done03:35
davecheneythumper: oh, ok, it's that simple03:35
* davecheney wonders what lbox does ...03:35
thumperdavecheney: previously was referring to jtv's comment above about errors03:35
thumperdavecheney: lbox does that too, but slowly03:36
thumperand with extra checking03:36
thumperand many round trips03:36
davecheneythumper: are you talking about shadowing ?03:38
thumperdavecheney: yeah, is the err a shadow or using the one from the previous scope?03:38
davecheneyit is what it is03:38
davecheneyit can't be changed now, it's in the spec03:38
jtvthumper: just tried the sklurg/err thing and yes, it creates a new sklurg *and* a new err.03:41
davecheneylooks like others feel the same, https://twitter.com/GolangSucks/status/31999990986665164803:47
thumperjtv: is that you @GolangSucks ?03:48
jtvthumper: nope03:59
jtvBit strong for me...  You know what I'm like.  I'm trying to see the good things.  :)03:59
jtvBut I do notice a surprising similarity.  "for item := range x counts x. Unless it's a channel in which case it returns values."  Pretty much a literal match with something I wrote.04:01
jtvThe unnecessary choices are something I mentioned as well.  Lots of make-work.04:01
bigjoolsGetting a compile error from trunk: http://paste.ubuntu.com/5678748/05:00
davecheneybigjools: probably got versoin skew with openstack05:01
bigjoolsdavecheney: oh I assumed a go get -u would update that?05:01
davecheneytry go get -u launchpad.net/goose/...05:01
davecheneyit might05:01
bigjoolsas in, I did a get -u on juju-core05:02
davecheneyohhh, i wouldn't do that05:02
davecheneyit'll totally screw your bzr checkout05:02
davecheneygo get -u doesn't understand cobzr branches05:03
bigjoolsI don't use cobzr05:03
davecheneyfair enough05:03
bigjoolsthat's the least of its problems anyway, I aliased my branch command so it doesn't use trees05:03
bigjoolswhich breaks go get05:04
davecheneybigjools: probably easier to skip it and just to a bzr pull05:04
bigjoolsanyway - still got that compile error05:04
bigjoolsafter goose updated05:04
bigjoolsI am on raring if that makes any difference05:05
davecheneybigjools: pls hold05:07
bigjoolsdavecheney: hlding :)05:07
bigjoolsand holding05:07
davecheneybigjools: fastest way to a fix05:08
davecheneyrm -rf $GOPATH/src/launchpad.net/goose05:09
davecheneygo get launcpad.net/goose/...05:09
davecheneyor whatever method you want to use to manage your source05:09
bigjoolsoh did it move?05:09
davecheneyit did not move05:10
davecheneythat isn't strictly true05:10
bigjoolsquickest way to fix that is:05:10
bigjoolsbzr pull --remember lp:goose05:10
bigjoolsaaaand done.  Loads of updates, crikey.05:11
davecheneysomeone fucked a lot of things up when the kicked us all out of the ~gophers gropu05:11
bigjoolsok new errors05:11
bigjoolscanonical/GO/src/launchpad.net/juju-core/cmd/juju/constraints.go:66: undefined: results05:11
davecheneyfix proposed05:12
bigjoolsah is this what you were talking about jtv?05:12
bigjoolsdavecheney: given that it's blocking everyone, are you going to land it right away or do you have to wait for 2 pairs of eyes?05:15
davecheneybigjools: that is all the enchouragement I need :)05:16
bigjoolsdavecheney: that was the intention :)05:16
bigjoolsdavecheney: compiles \o/05:21
bigjoolsand the raring mongo seems to have ssl support \o/05:52
davecheneybigjools: where is the lp project for mongo ?05:54
davecheney~mongodb looks stale05:54
bigjoolsdavecheney: not sure there is one - I am just using the latest package, I think jamespage packaged it05:54
bigjoolsnow, I am getting loads of test failures :/05:54
bigjools[LOG] 92.72463 ERROR api: error receiving request: read tcp use of closed network connection05:55
bigjoolsall over05:55
davecheneybigjools: id suggest that ssl doesn't work05:55
bigjoolsare you on raring?05:56
bigjoolsdavecheney: I think raring's mongo (2.2.4?) is incompatible with the current juju06:21
bigjoolsit definitely has ssl support06:22
bigjoolsthumper: you're on raring?06:24
bigjoolshmm I have it working on a fresh raring VM.  I wonder what's up with my local env...06:31
rogpeppemornin' all!06:45
davecheneyerror: Failed to load data for branch bzr+ssh://bazaar.launchpad.net/~dave-cheney/juju-core/111-add-go-build-check-to-lbox/: Server returned 502 and body: <!DOCTYPE html>07:17
davecheney<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">07:17
davecheneythanks for coming in today, launchpad07:17
* fwereade_ -> shops, bbiab07:33
dimiternrogpeppe: ping09:07
rogpeppedimitern: pong09:07
* fwereade_ slept badly, and is more than usually stupid today, and is going to lie down for a bit09:07
dimiternrogpeppe: hey, let me pass an idea by09:08
rogpeppedimitern: ok09:08
dimiternrogpeppe: how about adding Id() to AgentState, so the jujud/agent can find out which entity id it's associated with and check if it's sane, and then set the password?09:08
* rogpeppe goes to look09:10
rogpeppedimitern: i'm not sure i understand the motivation.09:11
dimiternrogpeppe: it seems all implementations, except the upgrader are backed by a state object - machine or unit09:11
rogpeppedimitern: the code that sets the password first gets the entity09:11
rogpeppedimitern: there's already a method, Entity, to do that09:11
dimiternrogpeppe: ah!09:11
rogpeppedimitern: if we make the Entity code return an error if something about it is bogus, then you've got what you want, no?09:12
rogpeppedimitern: that's what i was suggesting last night09:12
dimiternrogpeppe: exactly09:12
dimiternrogpeppe: for example how is a machine taken as an entity?09:13
rogpeppedimitern: i'm not sure i understand the question09:13
dimiternrogpeppe: trying to find the Entity interface09:14
rogpeppedimitern: there's no Entity interface09:14
rogpeppedimitern: Entity returns an AgentState interface value09:14
dimiternrogpeppe: ok, but where's the Id()?09:14
dimiternrogpeppe: it's not in the AgentState09:14
rogpeppedimitern: Entity is implemented by the agent in question.09:15
dimiternrogpeppe: and all I get back is an AgentState - should I cast it to something to get its Id?09:15
rogpeppedimitern: so we have MachineAgent.Entity and UnitAgent.Entity09:15
rogpeppedimitern: each of those knows the id09:15
dimiternrogpeppe: ok, got you - you mean only return the entity if it's sane09:15
rogpeppedimitern: for instance, look at the implementation of MachineAgent.Entity09:15
rogpeppedimitern: yes09:16
rogpeppedimitern: there's already an error return09:16
dimiternrogpeppe: that should work, cheers09:16
rogpeppedimitern: cool09:16
rogpeppedimitern: you might find this quite cool: i've been trying to isolate a go runtime bug that occurs when running the uniter tests against go tip. i wanted to eliminate the possibility that it was a problem with our use of unsafe and cgo (goyaml uses cgo), so i mocked up goyaml to make rpc calls instead. here's the "mock" goyaml implementation: lp:~rogpeppe/+junk/yamlserver09:26
rogpeppedimitern: it worked - i got uniter tests passing when using it, and i'm still seeing the runtime crashes.09:26
dimiternrogpeppe: cool, I'll take a look09:27
rogpeppedimitern: i think it's nice that i can swap out an entire implementation like that and almost nothing needed to change in any of the other code (i just needed to change the uniter tests to register the expected types)09:28
dimiternrogpeppe: so it uses goyaml on the rpc server09:28
rogpeppedimitern: yeah09:29
dimiternrogpeppe: and the crash in uniter tests is no longer there?09:29
rogpeppedimitern: no, it still crashes09:29
rogpeppedimitern: which is what i was hoping09:29
dimiternrogpeppe: so it's not our code then09:29
rogpeppedimitern: because it means it's (almost certainly) not a problem with our code09:29
rogpeppedimitern: yeah09:29
dimiternrogpeppe: good to hear, but any idea is it in goyaml or is it go-tip related bug?09:30
rogpeppedimitern: it's a go-tip related bug. probably the new garbage collector.09:30
rogpeppedimitern: but perhaps the new scheduler.09:31
rogpeppedimitern: http://code.google.com/p/go/issues/detail?id=519309:31
dimiternrogpeppe: hmm.. it seems the closer we get to 1.1 release more nasty bugs appear09:31
rogpeppedimitern: they've shoved a lot of stuff in quite recently.09:31
rogpeppedimitern: i'm very concerned too.09:31
dimiternrogpeppe: it seems at least they're looking into it, it's not just collecting dust in the issues list09:34
rogpeppedimitern: yeah, they're concerned too09:34
dimiternrogpeppe: well, there are a lot of smart people there, so there's hope of fixing it09:40
rogpeppedimitern: yeah. it would be nice if i could replicate the problem with less than x00,000 lines of source code :-)09:40
dimiternrogpeppe: definitely :)09:41
rogpeppedimitern: unfortunately the problem is so intermittent, that's not easy09:42
dimiternrogpeppe: how often can you reproduce it?09:47
rogpeppedimitern: about once in every 3 or 4 runs09:47
rogpeppedimitern: it dies in quite a few different ways09:48
dimiternrogpeppe: bugger09:49
rogpeppedimitern: see the collection of panics in my last message on the above issue09:50
dimiternrogpeppe: in random places of the code or at least these are often the same?09:50
dimiternrogpeppe: ah, ok09:50
dimiternrogpeppe: nasty.. they're in at least 3 different places09:54
rogpeppedimitern: yeah. i'm pretty sure they're all symptomatic of something else (for instance a race or other bug in the garbage collector)09:55
rogpeppedimitern: i see other problems too which aren't panics, but are symptomatic of memory corruption09:55
dimiternrogpeppe: so you nailed it down to r1600809:56
dimitern(the link is broken there btw)09:56
rogpeppedimitern: i'm not sure.09:57
rogpeppedimitern: that might just be the change that made the garbage collector bug show up09:57
rogpeppedimitern: because 16008 is when the new scheduler was introduced09:57
dimiternrogpeppe: in can be in either the new scheduler or the GC, both nasty internals09:59
dimiternrogpeppe: oh well, we'll see how it goes09:59
rogpeppedimitern: yeah09:59
dimiternrogpeppe: so if a machine doesn't have instid, is it correct to call EnsureDead() before returning it from the Entity()? That way it'll be detected in openState and the worker will die properly10:08
fwereade_dimitern, hell no :)10:09
dimiternfwereade_: why?10:09
fwereade_dimitern, we don't want the machine to die -- there'll be another agent along soon to do the right thing10:09
fwereade_dimitern, we just want the bad agent to go away and not come back :)10:09
rogpeppefwereade_: exactly10:10
dimiternfwereade_: so we want the instance dead, not the machine10:10
fwereade_dimitern, yeah, exactly10:10
rogpeppedimitern: and the agent will die immediately anyway10:10
dimiternrogpeppe: not really10:10
fwereade_dimitern, sorry about the terminology, it is annoying ;)10:10
dimiternrogpeppe: it'll die (now) iff the machine is missing or dead10:10
rogpeppedimitern: because the run loop will terminate10:10
rogpeppedimitern: ah, you're right - it'll keep on trying10:11
fwereade_dimitern, rogpeppe: how would you feel about just writing a flag somewhere local saying "stopped" or something10:11
dimiternrogpeppe: I'll have to introduce a specific ErrUnprovisioned for example, and use that to amend the cases in which the worker dies10:11
rogpeppefwereade_: i really don't care about this case10:11
rogpeppefwereade_: it's a ghost machine10:12
rogpeppefwereade_: i don't care if it carries on trying10:12
rogpeppea ghost *instance*, rather10:12
dimiternrogpeppe: but it'll be nicer to finish off the rouge MA as early as possible in this case, isn't it?10:12
fwereade_rogpeppe, so long as we return nil from Run it'll only retry once per restart anyway I guess10:12
fwereade_rogpeppe, but wait10:12
rogpeppedimitern: i don't want to introduce more logic for this case which is actually vanishingly unlikely10:13
fwereade_rogpeppe, if we *don't* mark it stopped, then a badly-timed restart of its instance can fuck up the other machine, I think10:13
rogpeppefwereade_: how so?10:13
dimiternfwereade_, rogpeppe: how about that specific error (ErrUnprovisioned) or something?10:14
dimitern(returned from MA.Entity)10:14
dimiternor maybe (nastier) return NotFoundf(), although it's clearly a lie, but it'll only affect the agent10:15
fwereade_rogpeppe, agreed vanshingly unlikely, but: (1) bad machine logs in with old password (2) provisioner sets password for good machine and starts an instance (3) bad machine changes password and starts running10:15
fwereade_rogpeppe, (4) provisioner takes down bad machine (5) good machine can't run10:15
dimiternfwereade_: why is this at all so unlikely?10:16
fwereade_dimitern, it requires pathologically bad timing10:16
dimiternfwereade_: given infinite time and scenarios, everything is possible :)10:16
fwereade_dimitern, exactly so10:17
fwereade_dimitern, that is why I would prefer a little tweak to the agent running10:17
fwereade_dimitern, first thing an agent should do is check its conf for a stopped flag; last thing it should do if it's returning nil from Run is to set the stopped flag10:18
rogpeppefwereade_: why would the bad machine ever get as far as changing the password?10:18
dimiternrogpeppe: this happens first thing after running the MA, right?10:18
fwereade_rogpeppe, because the provisioner set one at the worst possible time... say during a 5-second t1.micro sleep on the bad machine10:18
rogpeppefwereade_: the bad machine doesn't care - it looks at the instance id and sees that it doesn't match its own instance10:19
rogpeppefwereade_: i don't see that there's any way it can get past that barrier10:19
fwereade_rogpeppe, how's it meant to find that out? we're planning to drop InstanceId from EnvironProvider10:19
dimiternrogpeppe: it cannot know it's own instance id, only the machine id10:19
dimiternrogpeppe: it can check is it "" or not10:19
rogpeppefwereade_: orly?10:20
fwereade_rogpeppe, yesrly, it's caused problems for both maas and openstack10:20
rogpeppefwereade_: so how do we deal with the "provisioner unprovisions itself" problem?10:20
fwereade_rogpeppe, and there's no reason for it except to prevent the provisioner from screwing itself10:20
fwereade_rogpeppe, so, we special-case in the provisioner instead10:21
dimiternfwereade_: I think MAAS guys figured it out, and in openstack it can be done through the storage10:21
fwereade_dimitern, I know it can be done10:21
rogpeppefwereade_: how does that work?10:21
fwereade_dimitern, but there is a different shitty wa of doing it in each case10:21
fwereade_rogpeppe, if there's only one machine in state, grab the instance id from provider state... done. right?10:22
fwereade_dimitern, note that the put-it-in-environ-state works for the bootstrap machine only -- it won't let us get an InstanceId on an arbitrary machine10:23
* dimitern keeps waiting for an answer on errUnprovisioned (locally in jujud only)10:23
rogpeppefwereade_: assuming the machine doesn't come up really fast, i suppose :-)10:23
rogpeppedimitern: we're getting there :-)10:24
fwereade_rogpeppe, even if it does, we fail out and get restarted in 5s10:24
rogpeppefwereade_: no, if it does, we unprovision ourselves10:24
rogpeppefwereade_: or... no, i see10:24
fwereade_rogpeppe, this would happen before we do anything else, so I think it's safe10:25
rogpeppefwereade_: i'm not sure you can guarantee it happens before we do anything else10:25
rogpeppefwereade_: the provisioner is just another API client, right?10:26
fwereade_rogpeppe, what's affected by this other than the provisioner?10:26
rogpeppefwereade_: someone external could jump in before the provisioner and add another machine10:26
fwereade_rogpeppe, ah!10:26
fwereade_rogpeppe, ok, so we special-case it to be machine 0 then ;p10:26
dimiternfwereade_: how about HA state then?10:27
rogpeppedimitern: HA state is fine - we only have this problem at bootstrap10:27
fwereade_dimitern, HA state is orthogonal, I still plan to bootstrap one machine and add HA later10:28
dimiternah, ok then10:28
fwereade_dimitern, even if it's part of jujud bootstrap itself to add N more machines to state directly10:28
rogpeppefwereade_: ah, i've got an idea10:28
rogpeppefwereade_: the bootstrap init code marks machine 0 with an instance-id "pending"10:29
rogpeppefwereade_: when the provisioner comes up, if it sees machine 0 with "pending" instance id, it fetches it from the provider10:29
rogpeppefwereade_: otherwise i don't see how the machine-0 special casing works10:30
fwereade_rogpeppe, not sure about a magic value in that field, I'd kinda rather it be separate10:31
* dimitern still doesn't quite get why we need to special-case machine-010:32
rogpeppedimitern: because machine-0 is the *only* place that haven't got the state to write the new instance id to10:32
fwereade_dimitern, we can't always know machine 0's instance id before we provision it10:32
fwereade_dimitern, and we can't write it to state just after we provisioned it, because there's no state yet10:32
rogpeppefwereade_: yeah, an extra field in Machine would work as well10:33
dimiternoh, I see now10:33
fwereade_rogpeppe, I'm wondering whether it's actually a property of the environment10:33
rogpeppefwereade_: oh, doh!10:34
fwereade_rogpeppe, Environment.BootstrapComplete() (bool, error) ?10:34
rogpeppefwereade_: it's something that can be done by bootstrap-init10:34
rogpeppefwereade_: the provisioner doesn't need to know anything about it10:34
fwereade_rogpeppe, hmm10:35
rogpeppefwereade_: i mean bootstrap-state, of course10:35
fwereade_rogpeppe, yeah, that's right10:35
fwereade_rogpeppe, oh no wait10:35
fwereade_rogpeppe, we need the user to have connected once before we have the keys that let us find out the instance id10:36
* dimitern goes for a run until the argument settles, bbi30m10:36
fwereade_rogpeppe, that doesn't matter now10:36
rogpeppefwereade_: hmm, i suppose so. unless the provider can make it available to anyone10:37
fwereade_rogpeppe, but might it if the CLI were doing an API connection?10:37
fwereade_rogpeppe, I think it's ok to put it in the provisioner anyway -- the locality of reference would help, I think10:38
rogpeppefwereade_: the code would be much simpler if it could go in bootstrap-state, but i'd forgotten about the provisioner keys issue10:38
fwereade_rogpeppe, I know it would :(10:39
fwereade_rogpeppe, but I honestly think it's not too bad10:39
fwereade_rogpeppe, it exists to fix the provisioner10:39
rogpeppefwereade_: yeah, but the muck spreads into quite a few places10:39
rogpeppefwereade_: perhaps we change Machine.InstanceId to return, instead of a bool, one of InstanceIdNotSet, InstanceIdOk or InstanceIdPending, rather than add another bool.10:40
rogpeppefwereade_: then if the provisioner finds machine 0 with InstanceIdPending, it knows where to look10:41
rogpeppefwereade_: and it can then fill in the instance id and carry on as normal10:41
fwereade_rogpeppe, it feels quite localized to me anyway though10:42
rogpeppefwereade_: well, it affects the provisioners and the state as well as the provisioner, so not *very* localized :-)10:42
fwereade_rogpeppe, I don't think it even has to affect the providers10:43
rogpeppefwereade_: don't they have to be able to return the instance id for machine 0 ?10:43
fwereade_rogpeppe, they all have to list instances on demand, and those instances have an InstanceId method10:44
mattywfwereade_, rogpeppe sorry to interupt, I'm still having problems connecting to the api https://pastebin.canonical.com/8850410:44
rogpeppefwereade_: that doesn't help10:44
fwereade_rogpeppe, meaning that we only have to worry about a single mechanism for determining instance id10:44
rogpeppefwereade_: we need to know which of those instances corresponds to machine 0's instance10:45
rogpeppemattyw: OpenID discovery error: HTTP Response status from identity URL host is not 200. Got status 50310:45
fwereade_rogpeppe, how are those other instances going to get started without a provisioner? :)10:45
rogpeppefwereade_: bootstrap10:45
rogpeppefwereade_: and possibly by a previous environment10:45
rogpeppefwereade_: if we could assume that we'd only ever get one instance after bootstrap, the problem would be much easier10:47
fwereade_rogpeppe, do you mean a previous environment that has not been properly shut down, or just an eventual-consistency phantom?10:47
rogpeppefwereade_: the former.10:47
fwereade_rogpeppe, screw that, user error10:47
fwereade_rogpeppe, if destroy-environment failed, run it again until it succeeds10:47
rogpeppefwereade_: but eventual consistency could make a properly shut down environment show the same symptom10:48
TheMueaaaaaargh (facepalm)10:48
TheMuefound the failure, only had to move three loc a bit. didn't seen which value differs otherwise in all that debugging output. (hmpf)10:50
rogpeppefwereade_: the scenario that concerns me is when, somehow, the token that signifies that an environment has been bootstrapped has gone (perhaps someone removed all their buckets), so we bootstrap and find all the previous instances.10:52
fwereade_rogpeppe, to be utterly explicit, this is when the env reports one terminated machine to be running and one running machine not to exist, right?10:52
rogpeppefwereade_: no, it's when the env reports more than one running instance, only one of which is the bootstrapped instance.10:53
mattywfwereade_, rogpeppe https://pastebin.canonical.com/88504/ should be working again now - it's trouble connecting to the api10:53
fwereade_rogpeppe, meh, fail out when len(instances) != 110:53
rogpeppefwereade_: that is a possibility, yeah. we could mark the bootstrap machine with an error status in that case.10:54
fwereade_rogpeppe, not even that10:55
fwereade_rogpeppe, just try again later10:55
rogpeppefwereade_: the user needs at least some feedback, otherwise they'd have no idea why "juju deploy" isn't working.10:56
rogpeppefwereade_: or... just don't unprovision any machines in that case, i guess10:56
fwereade_rogpeppe, but wait I'm being stupid10:57
fwereade_rogpeppe, we *always* set the instance id in the provider storage anyway10:57
rogpeppefwereade_: yes10:57
fwereade_rogpeppe, provisioner, as soon as it has keys, calls ensureBootstrapComplete()10:57
rogpeppefwereade_: well, in the existing providers10:57
rogpeppefwereade_: we don't make it available though10:58
fwereade_rogpeppe, agreed that without storage we will have a hard time10:58
rogpeppefwereade_: we had plans to move away from that storage in ec210:58
rogpeppefwereade_: and use instance tags instead10:58
rogpeppefwereade_: ah!10:58
fwereade_rogpeppe, well, that would be an alternative solution10:58
rogpeppefwereade_: this is a good reason why generating the env UUID on the client side would be good10:59
fwereade_rogpeppe, tagging instances with uuid? yeah10:59
rogpeppefwereade_: yeah10:59
fwereade_rogpeppe, it feels to me like storage is the tool we have available now though10:59
rogpeppefwereade_: that's true. but i'd like our solution to work with tags too.11:00
rogpeppemattyw: looking11:00
fwereade_rogpeppe, something funny about `invalid entity name ""` given the `info.Tag = "user-admin"`11:01
rogpeppefwereade_: that's what i'm thinking11:01
rogpeppemattyw: is this on go trunk?11:01
rogpeppemattyw: i mean juju-core tip!11:02
mattywrogpeppe, no, it's 2 or 3 days old11:03
rogpeppemattyw: what revid?11:03
mattywrogpeppe, 1060 I think *double checks*11:03
mattywrogpeppe, just to make things worse, it's actually of our own branch https://code.launchpad.net/~cloud-green/juju-core/trunk so we had a stationary target to work on11:04
rogpeppemattyw: that's ok - i'll branch that and have a look11:05
fwereade_rogpeppe, anyway, for maximum paranoia we can wait until there is a running instance with an id that matches the one in storage11:07
rogpeppefwereade_: i think just st.Machine("0").SetInstanceId(environ.InstanceIdOfBootstrapMachine()) should be fine11:08
fwereade_rogpeppe, ok, sgtm :)11:09
rogpeppemattyw: ok, looks like you're on 1091. have you made any of your own changes to that branch?11:11
rogpeppemattyw: (if not, i'm not quite sure why you merged trunk rather than just pulling it)11:11
fwereade_rogpeppe, btw did the terminated-flag-in-conf idea seem to hold water?11:12
rogpeppefwereade_: oh yes, i was meaning to get back to that11:12
rogpeppefwereade_: i'm not sure it eliminates the race entirely11:12
fwereade_rogpeppe, hmm. "if there's an instance ID but an independent attempt to log in with my original credentials fails, it's not my instance id, so bomb out with errTerminating; otherwise go ahead and change it"?11:16
rogpeppefwereade_: sounds plausible11:17
rogpeppefwereade_: although a bit more heavyweight than i'd like11:18
rogpeppefwereade_: a simple nonce might be better11:19
fwereade_rogpeppe, chosen by provisioner, set on machine, passed through StartInstance?11:20
rogpeppefwereade_: yup11:20
fwereade_rogpeppe, nice11:20
rogpeppefwereade_: perhaps have it returned by Machine.SetInstanceId11:21
rogpeppefwereade_: which would increment it11:21
fwereade_rogpeppe, we need to pick a nonce before we start the machine11:21
rogpeppefwereade_: doh!11:21
rogpeppefwereade_: just use a random number.11:23
rogpeppefwereade_: if we get the provisioner to pick a random number n when it starts, then n.seqno actually provides useful information about which provisioner (in an HA world) provisioned the machine.11:25
rogpeppefwereade_: and it's a nice consistent link between new machine and the state. i *think* i like it.11:27
rogpeppes/new machine/new instance/ of course11:27
fwereade_rogpeppe, I don't follow the "seqno" bit, but if you mean $(provisioner-id)-$(random-number) I'm +111:28
rogpeppefwereade_: hmm, mind you, the initial password is a pretty good link too11:28
fwereade_rogpeppe, a prefix on the random password? that feels a bit off to me11:29
rogpeppefwereade_: i mean $(random-provisioner-id)-$(sequentially-incrementing-number)11:29
rogpeppefwereade_: no, i mean that if you manage to log in with the initial password, you've a pretty good idea that you're talking to the right state.11:29
fwereade_rogpeppe, damn lunch11:32
rogpeppefwereade_: enjoy11:32
* dimitern is back11:33
mattywrogpeppe, no changes, the only reason I haven't pulled is my inexperience with bzr11:33
rogpeppemattyw: sorry, i'll take another look in a mo11:34
rogpeppedimitern: there's already a way of telling the run loop to terminate11:36
rogpeppedimitern: see isFatal in agent.go11:36
rogpeppedimitern: if you return errUnprovisioned and add errUnprovisioned to the isFatal cases, it should all just work.11:37
rogpeppedimitern: ha!11:37
rogpeppedimitern: in fact, just return &fatalError{"machine was not provisioned"}11:38
* rogpeppe feels a little bit smug11:38
rogpeppemattyw: are you bootstrapping with the same version as you're compiling your client against?11:39
dimiternjust finished reading through  the log11:39
dimiternrogpeppe: sgtm, nicer11:40
dimiternrogpeppe: unfortunately it doesn't work11:41
rogpeppedimitern: oh?11:42
dimiternrogpeppe: isFatal is checked after we're in runLoop11:42
rogpeppedimitern: and?11:43
dimiternrogpeppe: so it won't solve the issue about messing up the password by a bad machine11:43
rogpeppedimitern: the password is changed inside runOnce11:43
rogpeppedimitern: erm... i think :-)11:43
rogpeppedimitern: yeah11:44
dimiternrogpeppe: istm it's in openState:SetMongoPassword11:44
rogpeppedimitern: yup11:44
rogpeppedimitern: and what's openState called by?11:44
dimiternrogpeppe: by RunLoop's callback in runagentloop11:44
rogpeppedimitern: which is called?11:45
dimiternrogpeppe: so it's too late then11:45
rogpeppedimitern: i don't think so11:45
rogpeppedimitern: runOnce calls openState11:45
dimiternrogpeppe: why not just have Entity return a fatalError instead and add it to the conditions?11:45
rogpeppedimitern: what conditions?11:46
dimiternif state.IsNotFound(err) || err == nil && entity.Life() == state.Dead { err = worker.ErrDead }11:46
dimiternin openState11:47
dimiternmake that || isFatal(err) || ... and all done11:47
rogpeppedimitern: there's no point in doing that11:47
rogpeppedimitern: if the err is not nil, it returns in11:48
rogpeppedimitern: on the next line11:48
dimiternrogpeppe: that's ater11:48
dimiternrogpeppe: yeah, the first if is what i'm aiming for11:48
rogpeppedimitern: you want openState to return ErrDead?11:48
dimiternrogpeppe: it already does, doesn't it?11:49
rogpeppedimitern: yeah, or a fatalError if Entity returns that11:49
dimiternrogpeppe: ok, it seems I'm not explaining myself well enough11:49
dimiternrogpeppe: err = worker.ErrDead is what I want to be the case, just like the other conditions, where the machine is missing or dead11:50
dimiternrogpeppe: this causes the next if to return ErrDead11:50
dimiternrogpeppe: and kills the worker before setting the password or anything bad happens11:50
rogpeppedimitern: even if you return a fatalError, the worker will be killed before that11:50
rogpeppedimitern: but...11:51
rogpeppedimitern: there's something to be said for returning ErrDead, because then the agent exits and never restarts11:51
dimiternrogpeppe: yeah, actually.. but the test fails11:51
dimiternrogpeppe: look at this http://paste.ubuntu.com/5679464/11:52
rogpeppedimitern: are you saying you're returning a *fatalError from Entity and the agent isn't exiting?11:52
dimiternrogpeppe: this test succeeds when I used errUnprovisioned and checked for it in the first if, with fatalError it fails, saying it's still running11:52
rogpeppedimitern: could you paste your Entity implementation, please?11:52
dimiternrogpeppe: http://paste.ubuntu.com/5679472/11:54
dimiternrogpeppe: and now I'm getting "timed out waiting for agent to finish; stop error: <nil>" in the assert after runAgentTimeout11:55
mattywrogpeppe, no, I'm not bootstrapping using the same version11:55
rogpeppedimitern: hmm, that's weird.11:55
rogpeppedimitern: it *should* work. could you push your branch?11:56
dimiternrogpeppe: ok, just a sec11:56
rogpeppemattyw: are you bootstrapping with a newer version, by any chance?11:56
rogpeppemattyw: the API has recently changed in a backwardly incompatible way11:56
mattywrogpeppe, I don't think so. My juju command is 1.9.12-precise-amd6411:57
rogpeppemattyw: in general, while we're developing stuff fast, you need to run exactly the same version of client and server.11:57
mattywrogpeppe, ok. I'll try that11:57
rogpeppemattyw: you can use juju bootstrap --upload-tools11:57
rogpeppemattyw: but currently any charms you deploy have to be from the same series as your local machine11:58
mattywrogpeppe, I forget, I've had a lot of problems trying to bootstrap at all recently11:58
rogpeppemattyw: every time you see a problem, add a bug!11:58
mattywrogpeppe, they're the same series11:58
mattywrogpeppe, I do :)11:58
dimiternrogpeppe: lp:~dimitern/+junk/machine-agent-status11:59
rogpeppemattyw: (or add to an existing bug if it seems the same)11:59
rogpeppedimitern: ta11:59
mattywrogpeppe, is there a version of the repo you'd recomend running against at the moment?12:00
rogpeppemattyw: i'd try and keep in sync with tip, though it's a pain12:02
dimiternrogpeppe: hmm.. for some reason the old change got pushed, not the one with fatalError12:03
rogpeppedimitern: perhaps you didn't commit12:03
dimiternrogpeppe: ouch :( haven't saved it in emacs12:03
dimiternrogpeppe: sorry, let me try again12:03
rogpeppedimitern: ah, that might have something to do with it!12:04
dimiternrogpeppe: now it works with fatalError, sorry for the noise12:05
rogpeppedimitern: np. i was a bit confused :-)12:05
rogpeppedimitern: but...12:05
dimiternrogpeppe: the only change is I needed to change the assert to check for the error after runAgentTimeout12:05
rogpeppedimitern: having said that...12:05
rogpeppedimitern: i'm wondering if you should return ErrDead anyway12:06
dimiternrogpeppe: maybe upstart will mess it up if it's not ErrDead and restart it?12:06
rogpeppedimitern: exactly12:06
fwereade_rogpeppe, mattyw: FWIW I landed --fake-series yesterday12:07
fwereade_rogpeppe, mattyw: it seemed to work for me12:07
rogpeppedimitern: in fact, that's precisely the behaviour we *want* when upgrading12:07
rogpeppefwereade_: \o/12:07
dimiternrogpeppe: ok, ErrDead it is then - directly from the Entity()12:07
fwereade_rogpeppe, mattyw: `juju bootstrap --upload-tools --fake-series precise,quantal`12:07
dimiternrogpeppe: or fatalError + isFatal() in the if -> ErrDead?12:07
rogpeppedimitern: i think so. although it's kind of a lie.12:08
fwereade_rogpeppe, I'm -1 on reusing ErrDead12:08
rogpeppedimitern: nononono12:08
fwereade_rogpeppe, it means something specific12:08
mattywrogpeppe, ok thanks, what does --upload-tools actually do?12:08
rogpeppefwereade_: yes12:08
dimiternso we're back on errUnprovisioned :)12:08
rogpeppefwereade_: i think we're just using ErrDead as a convenience12:09
rogpeppefwereade_: i think we can change all instances of ErrDead to ErrDeadOrInvalid12:09
fwereade_rogpeppe, dimitern: what's wrong with errTerminating? the top-level Run can set the conf to terminated and return nil; other conditions that indicate we should stop, like ErrDead, can do the same12:09
fwereade_rogpeppe, call it ErrTerminateAgent and I'm fine12:10
rogpeppefwereade_: what do you mean by "set the conf to terminated"?12:10
* dimitern an there goes another 20m for 2 lines of code ;)12:10
fwereade_rogpeppe, I mean, set a flag meaning "don't even try" that we can check on startup12:11
rogpeppefwereade_: why bother?12:11
dimiternI'm fine to call it errTerminated instead of errUnprovisioned, although the former seems not quite right - do you intend to reuse it for something else?12:11
rogpeppedimitern: i think i'd have type terminalError {err string}12:12
rogpeppedimitern: then the message printed by the agent when exiting can be accurate12:12
dimiternrogpeppe: lethalCase{} ? :D12:12
fwereade_dimitern, I'm saying that Run at the top level should have a single error that it treats as "return nil, causing upstart to stop bothering"12:12
rogpeppefwereade_: agreed. currently that's just ErrDead.12:13
dimiternfwereade_: we don't even have to get to run12:13
fwereade_rogpeppe, maybe we don't, let me reload state12:13
rogpeppefwereade_:  but we'd change that to check for *terminalError12:13
rogpeppefwereade_: ah! it does need to check for ErrDead too12:14
rogpeppefwereade_: because that's what the workers return if the machine is dead.12:15
fwereade_rogpeppe, yeah, exactly12:15
rogpeppefwereade_: but tbh why not just reuse it?12:15
rogpeppefwereade_: we're adding more code for a marginal case12:15
fwereade_rogpeppe, I'm fine having the workers who need to return ErrTerminateAgent, I'm not so fine having an unprovisioned machine return ErrDead12:15
rogpeppefwereade_: the machine might as well be dead as far as the agent is concerned12:16
rogpeppefwereade_: ok, i'd be happy with that12:16
* rogpeppe has lunch12:16
fwereade_rogpeppe, cool, cheers12:16
dimiternfwereade_: how about having a isDeadOrTerminated(err) checking for both ErrDead and ErrTerminated and is it in place of err == ErrDead checks?12:18
fwereade_dimitern, I'm just saying s/ErrDead/ErrTerminateAgent/ throughout12:20
dimiternfwereade_: actually, I can just add ErrTerminateAgent and return in from Entity, make it work like ErrDead wrt upstart and draw the line there, changing all the workers can be a follow-up12:21
fwereade_dimitern, forget ErrDead12:21
fwereade_dimitern, a single magic value meaning "stop and don't come back" is all I'm really thinking about here12:21
fwereade_dimitern, ErrDead exists, and is perfect, except it has the wrong name12:21
dimiternfwereade_: i'm not comfortable changing something without knowing the repercussions12:21
fwereade_dimitern, fix the name and we're fine :)12:22
dimiternfwereade_: what is ErrDead used for - in all cases?12:22
dimitern(for workers)12:22
fwereade_dimitern, it is used, in all cases, to indicate that the running agent should exit 012:22
dimiternfwereade_: well, if it's only that, the renaming makes sens12:23
fwereade_dimitern, cool12:24
fwereade_sorry, I definitely didn't communicate correctly12:24
dimiternfwereade_: I'll do it as a prereq though, since it's trivial12:24
fwereade_dimitern, sgtm -- but just to sync up, can we have a quick hangout?12:25
mattywrogpeppe, that seems to be working now thanks :)12:28
rogpeppemattyw: cool. sorry about that - it's been a problem for loads of people, because we're changing things in a backwardly incompatible way all the time without changing the major version number12:29
rogpeppemattyw: but that's dev versions for ya!12:29
mattywrogpeppe, one quick odd question12:36
rogpeppemattyw: ok12:36
mattywwhen a unit gets removed we get a unitinfo back with all the status about that unit? Is the plan to stay like that? The reason I ask is that in the pyjuju 'api' when a unit is removed we only get told the unit name of the removed unit12:37
rogpeppemattyw: yeah, it should stay like that.12:38
rogpeppemattyw: although it's kind of an unintended consequence of the way things are implemented12:39
mattywrogpeppe, ok thanks, some of my favourite consequences are the unintended ones12:39
mattywrogpeppe, once the api ships I guess any change we'll get told about?12:40
rogpeppemattyw: yup12:40
mattywrogpeppe, that's fine for me. When vds and I have a few more tests It would be good if you could take a look, will probably be next week though12:40
=== jam1 is now known as jam
rogpeppemattyw: sure12:42
=== BradCrittenden is now known as bac
rogpeppefwereade_: do you know the PPA/name of the SSL-capable mongod package, by any chance?13:44
fwereade_rogpeppe, sorry, I'm afraid not -- I always just grabbed it from the bucket and stuck in on $PATH13:45
rogpeppefwereade_: yeah, i did too. i'm trying to put some instructions together so that people can try to reproduce the go-runtime errors i've been seeing13:46
rogpeppefwereade_: and i seemed to remember someone had packaged mongod13:46
rogpeppefwereade_: i guess i'll just say "grab the bucket contents"13:46
rogpeppeniemeyer: hiya13:46
dimiternfwereade_, rogpeppe: ErrDead -> ErrTerminateAgent https://codereview.appspot.com/841604313:49
fwereade_dimitern, LGTM trivial14:00
dimiternfwereade_: cheers14:01
rogpeppeniemeyer: https://codereview.appspot.com/8367044 (not that it helped, in fact, but worth doing anyway)14:07
teknicorogpeppe, you're possibly looking for https://launchpad.net/~julian-edwards/+archive/mongodb/+packages14:40
rogpeppeteknico: thanks14:40
rogpeppeteknico: though i've just gone with directing people to the public bucket for now14:40
rogpeppeteknico: (with an accompanying sha1 sum)14:40
niemeyerrogpeppe: Rob spent a good while last night looking at the issue14:55
niemeyerrogpeppe: I've helped them setting up a juju environment so they could reproduce the issue14:55
niemeyerrogpeppe: He wrote down notes by the end of his day, and I suppose Dmitry moved it on14:56
rogpeppeniemeyer: thanks for that15:03
niemeyerrogpeppe: np, let's hope good stuff comes out of it15:05
* rogpeppe crosses his fingers15:05
dimiterncloudinit tests are a proper pain to change15:07
rogpeppedimitern: if you can think of a better way...15:07
rogpeppedimitern: they're better than they used to be15:07
rogpeppedimitern: a little script could probably help actually15:08
dimitern:) i'm not sticking my finger in there any more than I need to for now15:09
rogpeppemgz: hey mr on-call :-) a quick trivial branch for you (code move only) https://codereview.appspot.com/842204315:16
mgzmega over usage :)15:18
mgzare file-level comments a thing in go?15:18
rogpeppemgz: well, yeah15:18
rogpeppemgz: yes, we can do that though we tend not to15:18
mgzlike, triple quote at top of module saying roughly what it's about?15:18
mgzor is the pre-function ///15:19
rogpeppemgz: usually the type declaration at the top gives a good idea15:19
mgz// funcName15:19
mgzthe only special magic that the doc generation picks up on?15:19
rogpeppemgz: file-level comments are ignored by the doc tools15:19
rogpeppemgz: that and comments just before the package decl15:20
rogpeppemgz: the doc tool picks up comments attached directly to top level declarations, essentially15:20
rogpeppemgz: ta!15:22
mgzdimitern: ^if you have a moment and if rog wants two reviews15:22
rogpeppemgz: i'm not sure that i can say much in a file-level comment that the type decl and doc comment at the top don't already say15:23
rogpeppemgz: i think i'll leave it as is if that's ok15:24
mgzthat's a fair enough answer for one-struct files I think15:24
mgzwhich this isn't quite, but nearly :)15:25
dimiternmgz: oh, the reviewer showed up ;)15:26
mgzI was expecting pokes15:27
dimiternlet me read through the log15:27
mgzbrowsing randomly for stuff to do is painful with rietveld...15:27
mgzdimitern: just click the codereview link, it's pretty clear15:27
dimiternmgz: it is, but fortunately fwereade is keeping +activereviews neat now15:28
dimiternlooking, but it's submitted already15:29
mgzthat answers clause #2 then :)15:29
dimiternwhat's clause #2?15:29
mgzif rog wanted another review15:30
rogpeppemgz, dimitern: here's another fairly trivial one: https://codereview.appspot.com/8294044/15:31
dimiternrogpeppe: just opened it while you're writing :)15:32
mgzleading underscore?15:32
rogpeppemgz: python-style :-)15:33
mgzso, as a hint internal to the package past the caps-vs-nocaps exposure?15:33
rogpeppemgz: yeah15:33
mgzI guess that seems reasonable15:34
rogpeppefwereade: what do you think?15:34
dimiternrogpeppe: why do you need this?15:34
fwereaderogpeppe, that sounds sensible to me15:34
fwereadedimitern, because package-level-only encapsulation is scary as hell once you're past... say... 1 file per package ;)15:35
rogpeppedimitern: because i'm essentially exposing the allInfo type as an API to the backing type. some methods are not supposed to be called by the backing.15:35
dimiternrogpeppe: sure, but what will this solve?15:35
rogpeppedimitern: it just makes it easy to verify that the backing is only calling expected methods15:36
rogpeppedimitern: and if you call a _method from outside the type itself, you'll probably think twice15:37
dimiternrogpeppe: how about putting really internal stuff in a subpackage?15:37
rogpeppedimitern: i'm not sure. it seems very intertwined with the allWatcher implementation15:39
rogpeppedimitern: and that need to be internal to state, i *think*.15:39
dimiternrogpeppe: this *could* indicate code smell, but otherwise I'm fine with it15:40
rogpeppedimitern: actually, i think you're perhaps right - the allWatcher can trivially be moved into its own package.15:44
rogpeppedimitern: while the backing implementation remains inside state15:44
dimiternrogpeppe: sgtm15:44
dimiternfwereade, rogpeppe: machine nonce introduced https://codereview.appspot.com/8247045/16:04
rogpeppedimitern: cool; will look soon.16:04
rogpeppedimitern: actually, i'm not sure, peer reviews need doing by the end of day16:04
rogpeppedimitern: and i haven't done any yet16:05
dimiternrogpeppe: what do you mean?16:05
rogpeppedimitern: the allhands stuff16:05
dimiternrogpeppe: ah, yeah - I've done mine on the day16:06
rogpeppedimitern: v sensible!16:06
dimiternrogpeppe: I know myself how forgetful I can get, so I try to cheat and do it right away, so I can forget it comfortably :)16:06
rogpeppedimitern: that's not cheating, that's the right thing to do!16:07
dimiternrogpeppe: yeah, i meant cheating the lazyness :)16:08
=== deryck is now known as deryck[lunch]
fwereadeTheMue, reviewed16:53
fwereadedimitern, why is MachineNonce required in Conf?16:58
dimiternfwereade: that's what I asked earlier - should it be or not16:58
dimiternfwereade: but I think it's better to be required and add tests for it16:59
dimiternnext one coming - state support for nonce provisioning - https://codereview.appspot.com/8429044/16:59
fwereadedimitern, sorry, I thought you meant in state16:59
fwereadedimitern, but it shouldn't be required for agent.Conf -- that has to work for unit agents too17:00
dimiternfwereade: oops, I mean *environs*17:00
fwereadedimitern, cool17:00
dimiternfwereade: so no tests then? only in cloudinit perhaps\17:00
fwereadedimitern, in cloudinit sounds good17:01
dimiternfwereade: this will simplify things a lot17:01
fwereadedimitern, and you should at least check that a Conf can be read/written both with and without a MachineNonce17:01
dimiternfwereade: wasn't quite sure it'll be enough, but it's better to cut than to add ;)17:01
dimiternfwereade: you mean add a no-error test with machinenonce in?17:02
dimiternfwereade: and one without17:02
fwereadedimitern, yeah, so really leave the rest and just add a single test that includes it I think17:03
dimiternfwereade: deal17:04
dimiternfwereade: i'm starting on state now17:04
rogpeppedimitern: allwatcher now factored into its own package. the tests weren't entirely trivial, but altogether fairly painless17:08
dimiternrogpeppe: great to hear!17:09
niemeyerrogpeppe: You mentioned yesterday that you reproduced the bug on socket.Acquire even after the fix was made, right?17:11
rogpeppeniemeyer: yes17:12
niemeyerrogpeppe: Okay, cool17:12
niemeyerrogpeppe: Just want to make sure I'm not on crac17:12
rogpeppeniemeyer: all the panics in my last post on that issue were with that fix made17:12
rogpeppeniemeyer: yeah, i've spent a fair amount of time trying to convince myself of the same thing17:12
niemeyerrogpeppe: Oh, might be worth mentioning it in that thread17:12
niemeyerrogpeppe: Were the tracebacks made with the rpc trick too?17:15
rogpeppeniemeyer: yes17:16
rogpeppeniemeyer: (i did mention that actually)17:16
niemeyerrogpeppe: Yeah, it was Rob that seemed unaware in the thread17:16
fwereadedimitern, reviewed17:21
dimiternfwereade: tyvm17:26
niemeyerfwereade: FYI, juju publish will be up for review in a few17:37
fwereadeniemeyer, awesome, sorry I've been lagging a bit on your reviews17:37
niemeyerfwereade: It's fine.. I have all reviewed right now17:37
niemeyerfwereade: Just have to submit17:37
niemeyerfwereade: And the main piece is coming up in a moment17:37
fwereadeniemeyer, ah, fantastic :)17:37
niemeyerfwereade: I still want to add a few features, but I want to get the core in before that17:38
fwereadeniemeyer, +1 :)17:38
dimiternfwereade: why should CheckProvisioned(nonce) return an err ever? m.doc.Nonce == nonce should be enough, I think17:39
fwereadedimitern, ha, yes, I'm on crack17:39
dimiternfwereade: and it's because it can't be ever changed it'll either be that or emtpy17:39
dimiternfwereade: so, perhaps m.doc.Nonce == nonce && nonce != ""17:40
fwereadedimitern, but do check that InstanceId != "" as well, or make sure there's no way for it not to be (did we agree to panic on SetProvisioned with "" id?)17:40
dimiternfwereade: yeah, already taken care of in SetProvisioned17:40
fwereadedimitern, but you're absolutely right that we don't need to hit the network for that, cheers17:41
dimiternfwereade: notTheSame := D{{"instanceid", D{{"$nin", []InstanceId{"", id}}}}}17:41
dimiternfwereade: that's the extra assert in SetProvisioned17:41
dimiternfwereade: and it has corresponding bunch of errAborted handling (i'm proud :) )17:42
fwereadedimitern, I look forward to seeing it :)17:42
fwereadealthough, everybody, I'm off for a bit now17:42
fwereadeI'll probably be back at some stage, I'm up to my elbows in environs again, but I'm out of go-juice ATM17:42
fwereadehappy weekends to those I miss17:43
dimiternfwereade: you too!17:43
=== deryck[lunch] is now known as deryck
rogpeppeit's that time of day. happy weekend to all and sundry17:47
TheMuefwereade, rogpeppe: have a nice weekend17:48
TheMuefwereade: will propose the latest changes later17:48
dimiternrogpeppe, TheMue: both of you too ;)17:48
TheMuedimitern: enjoy the spare time to recreate17:49
dimiternTheMue: oh, i do, especially lately17:58
TheMueso, final propose is in, time for the weekend. bye19:28

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