User1: User2 scp & scp reproposed, some nice reuse in there
User2: User1 Sweet, will have a quick look
User2: User1 http://play.golang.org/
User2: Erm
User2: User1 http://play.golang.org/p/cPscJ6RuoX
User1: User2 yes, i had to check that as well
User2: User1 Sorry, I don't get it?
User2: User1 "This will panic if len(c.Args) == 1. I've redone the logic to be less crackful."
User1: conshttp://play.golang.org/p/okbxoy-UI2
User2: User1 c.Target, c.Args = c.Args[0], c.Args[1:]
User1: User2 http://play.golang.org/p/TObeRIa8wL
User1: yes, you are right
User1: but I dont understand why http://play.golang.org/p/Fd_jNi6mVe
User2: User1 Because that's out of bounds
User2: User1 s[1:] works when len(s) == 1 for the same reason that s[:1] works.
User1: i dont' think that is correct
User1: but this isn't the right place to argue about it
User1: i'll fix my code
User1: ok, someone explained it too me in the channel
User2: User1 Stepping out for the night.. have a good day!
User1: enjoy
User3: davecheney, User4 mornin'
User1: User3 howdy
User4: davecheney, User3 heyhey
User3: User4 it'd be great if you could have a glance at the uniter upgrade branch, if you have a moment sometime this morning: https://codereview.appspot.com/6561063/
User4: rogpeppe, sure
User4: rogpeppe, (still thinking)
User3: User4 np
User4: rogpeppe, ISTM that it would be simpler (not to mention less conflicty, and mildly kinder to the network) to get the Unit only when you actually need to run an upgrade
User4: rogpeppe, doing that removes the need to change runOnce and Uniter
User4: rogpeppe, is there a deeper motivation in play than "I need a unit"?
 * rogpeppe has a look to remind himself
User3: User4 we need to unit immediately
User3: User4 because we use it to announce the current agent version
User3: User4 but that's not the reason we change runOnce and Uniter
User3: User4 the reason for that is that we want to upgrader to be as independent as possible of uniter bugs
User3: User4 so we do the absolute minimum necessary before starting the upgrader
User3: User4 hence it's important that the uniter factory method doesn't return an error - early errors should not take down the upgrader.
User3: s/need to unit/need the unit/
User3: User4 does that make sense?
User4: rogpeppe, hmm, apart from the ones which should, but I see it's tricky
User3: User4 which errors *should* take down the upgrader?
User3: User5 last thing i was was [07:23:38] <fwereade_> rogpeppe, hmm, apart from the ones which should, but I see it's tricky
User3: s/was/saw/
User5: rogpeppe, cheers
User5: rogpeppe, I'm not quite convinced that deferring the error helps you much though, 1 mo
User5: rogpeppe, surely an early error return from the Uniter will hose the upgrader just as badly, because runTasks will terminate it
User3: User5 no, because we've got some special case logic in the upgrader for just such an eventuality
User3: User5 if it's killed early on, it waits until it has at least had a squizz at the proposed version
User3: User5 and if that's changed, it doesn't exit until it has actually downloaded the upgrade
User3: User5 (well, with some timeout too)
User3: User5 it's given 5 minutes
 * fwereade continues to think
User5: rogpeppe, something about all that does make me a little queasy... I feel that if I ask an upgrader to stop, it should jolly well stop, by criminy
User3: User5 this was discussed at the time
User3: User5 i think the upgrader is special
User3: User5 because it's the only way we can escape bad s/w
User5: rogpeppe, I agree, but IMO that means it should have control of the tasks rather than being antisocial :)
User3: User5 it *will* stop... when it's made good and sure that someone is not trying to upgrade us
User3: User5 well, as you know, that's how i started and that was deemed incorrect
User3: User5 so this is what we're doing. and it doesn't seem bad to me.
User5: rogpeppe, I do feel your pain there... but I think I need to talk to niemeyer about this
User3: User5 ok
User5: rogpeppe, sorry :(
User3: User5 if we change this now, BTW, it knocks everything off
User3: User5 because everything is built in this way currently.
User3: User5 the upgrader is "just" another task
User3: User5 so, given that we're not swimming in free time, and this architecture will work for the time being, perhaps we could move forward as we are and consider a change later?
User5: rogpeppe, the collision with my own uniter changes is not entirely trivial
User5: rogpeppe, and there is a value I return from NewUniter that really should block upgrades
User3: User5 which is?
User5: ErrUnitDead
User3: User5 i don't think it matters too much tbh. it's an edge case - it doesn't matter much if we do upgrade in that case, it's just an extra 5s delay
User6: morning
User3: User5 FWIW i think i'd probably structure it in a similar way even if the upgrader was in control - we want the upgrader to be independent right up until the moment it decides that now is the time to upgrade.
User3: User6 hiya
User6: User3 hiya
User3: User5 if you like, i'll merge your most recent uniter branch and include it as a prerequisite
User5: rogpeppe, surely the right thing to do there is to have the upgrader in change of runTasks? the wait & retry business could be handled internally, surely?
User5: rogpeppe, it's not the code conflict that bothers me so much as the lack of clarity
User3: User5 it's not *that* straightforward. you want to start the upgrader independently, regardless of what the other tasks are doing. then the wait and retry isn't logic that's in the outer loop (because it's dependendent on what gets downloaded) but in a separate goroutine.
User3: User5 so the structure starts to look similar to what we've currently got
User3: User5 i think the notion of a task being able to delay shutdown until it's done what it needs to do is a reasonable one, and one of the reasons we use tombs like we do.
User5: rogpeppe, yeah, I see that
User3: User5 last seen: [07:53:09] <fwereade> rogpeppe, yeah, I see that
User5: rogpeppe, I *think* I'm convinced, although I still don't quite entirely like something about it
User3: User5 i understand.
User5: rogpeppe, that said, I think I am coming around to your perspective that something outside the uniter should handle death-watching
User3: User5 wasn't there a load of mode-specific stuff that you needed to do when killed?
User5: rogpeppe, the uniter's Dying response is not interesting from your perspective, I think (although there's little point upgrading a dying unit)
User5: rogpeppe, I think you probably *should* be watching for Dead and crash-stopping though
User3: User5 "me" being...?
User3: User5 ah, i see
User3: User5 you mean i should crash-stop when something returns ErrDead
User5: rogpeppe, almost certainly yes
User3: User5 maybe. although i think it's a very rare edge case tbh
User5: rogpeppe, but what I was trying to say is that the upgrader itself should know not to bother watching a unit once it's dying
User3: User5 the upgrader doesn't watch a unit
User5: rogpeppe, I submit that it should, so that binary upgrades work within the same framework as anything else -- I don't think they transcend entity lifetime ;)
User3: User5 there's always a delay between entity being killed and entity actually dying. this just makes it a little longer in some edge cases.
User3: User5 if you upgrade a system, there's a possibility that some dying units might linger for a few seconds more as they download a new version. i don't think that's too bad a price.
User5: rogpeppe, well, it means that it continues to act alive -- in some, but not all, respects -- for arbitrarily longer
User3: User5 does it?
User5: rogpeppe, it may be that this is actually correct behaviour
User3: User5 what is the upgrader keeping alive?
User3: User5 after all, the Unit is marked dead
User5: rogpeppe, wait, I'm talking about bad behaviour on Dying more than ugliness on Dead here
User5: rogpeppe, I don't like the ugliness on Dead but I could live with it
User3: User5 i'm not sure it changes the Dying behaviour at all, does it?
User5: rogpeppe, the question that currently exercises me is "should Dying entities upgrade their code?"
User5: rogpeppe, maybe, actually, they should
User3: User5 i think so
User5: rogpeppe, yeah, I can imagine a bug blocking clean shutdown that can be resolved by an upgrade
User3: User5 i think it's nice to have upgrading totally divorced from any of the other logic
User5: rogpeppe, the fact that they stop watching for charm upgrades on Dying then becomes potentially problematic
User3: User5 i think charm upgrades are a different thing - they're at a higher level.
User3: User5 and they really do relate directly to the unit
User3: User5 so it makes sense not to upgrade a charm when the unit is dying
User5: rogpeppe, hm, the bug-blocking-clean-shutdown I guess does *not*apply because the user can already use juju resolved
User3: User5 for charm upgrade, yeah - we're providing the always-available layer on top of a charm.
User5: rogpeppe, ok, all sounds reasonable, I'll finish the review :)
User3: User5 tyvm
User3: User5 there's also this fairly trivial: https://codereview.appspot.com/6564063/
User3: everything in my window system has just start breaking. i'm gonna reboot
User3: started
User7: anyone here use multiple monitors under ubuntu?
User5: rog, ok, I have another thought
User5: rog, sorry no
User7: User5 ok, listening
User5: rog, I wil be comfortable with this is there is some very basic life handling at the top level
User5: rog, I think we need:
User5: rog, 1) drop the unit return from runOnce, because it's potentially panic-inducing anyway
User5: rog, 2) when we get the unit in runOnce, return a special error on NotFound, and that same special error if the unit exists but is Dead
User5: rog, 3) if we see that error in Run, return nil
User5: rog, 4) return the UpgradedError when we're doing an upgrade restart
User5: rog, 5) add a "normal exit 0" stanza to the upstart conf
User5: rog, and I think that's it
User5: rog, that I think conveys my intent as best I can
User5: rog, the precise details of implementation are ofc approximate
User1: the disparity between *ConfigNode.Map() and Charm.Config().Option is a pain in the balls
User5: rog, does that sound sane
User5: davecheney, ouch, I bet
User1: map[string]string vs map[string]interface{}
User1: mix in some json or yaml and it's a royal pain
User7: User5 i don't know what you mean about dropping the unit return from runOnce - i don't think it can induce panic
User7: User5 if we can't get the unit in runOnce, we'll never start the upgrader
User5: rog, bah, true
User7: User5 we could also check for dead if you like.
User7: User5 that would be trivial
 * rog wishes we used map[string]string throughout
User7: bbs
User5: rog, leaving unit stuff aside for a moment, what is your opinion of the "normal exit 0" thing?
User5: rog, (when you return ofc0
User7: User5 maybe the agent should just remove its own upstart conf
User5: rog, that feels icky to me
User7: User5 who else is going to remove it?
User5: rog, the machine agent?
User7: User5 ... or the principal unit agent, right?
User5: rog, and I think it's fine to have a unit agent run on startup and exit immediately without error to indicate that it's done all it has to do
User5: rog, yeah, whoever deployed it
User5: rog, or a machine agent for that matter
User7: User5 yeah, i think you're right
User7: User5 presumably it *is* possible to get upstart to never start something again after it's exited ok
User7: User5 i'm slightly dubious though - if we reboot, surely it'll start again anyway
User5: rog, AIUI the stanza "normal exit 0" should be sufficient
User7: User5 ok, sounds fine.
User5: rog, it will; and the UA will cleanly observe its deadness, exit without error, and never trouble the machine again that run
User7: User5 interesting point: currently if one of the workers dies without an error, runTasks will just let it die, but continue on. i wonder if actually it should kill everything as usual in that case, so the tasks are always tied together.
User5: rog, hmmmmmmmmmm
User7: User5 then the unit agent can exit with a nil error when the unit is dead
User7: User5 and everything gets shut down cleanly.
User5: rog, I think that in code I would prefer an explicit error return
User7: User5 a nil error seems a fine way of saying "i'm shutting down with no error" to me
User5: rog, an an error seems to me to be a fine way to signal "what you asked me to do ain't gonna happen"
User7: User5 it's done *exactly* what we asked it to do, surely?
User7: User5 but... given that it wants other things to die along with it, perhaps ErrDead is good.
User5: rog, gaah, sorry
User7: User5 last thing i saw was: [08:52:16] <fwereade> rog, an an error seems to me to be a fine way to signal "what you asked me to do ain't gonna happen"
User5: rog, well, we get to choose how we define it -- I see the condition of lacking a unit as fundamentally an error condition for the uniter, because it means it can't do anything
User5: rog, the client code may be in a position to handle that specific error in a different way
User7: User5 zero is a ok value :-)
User7: User5 last thing i said before you went:
User7: [08:53:45]  User5 but... given that it wants other things to die along with it, perhaps ErrDead is good.
User5: rog, a Uniter *needs* a viable Unit to do its job -- if it cannot get the unit, or the unit is dead, that is an error :)
User5: rog, ah, I missed that, sorry
User5: rog, +1 on ErrDead causing nil return from Run
User7: User5 ok, will do
User5: rog, btw, would you put it somewhere easily accessible, like state, so I can also return it from the Uniter please?
User7: User5 it could even go in the Uniter if we wanted
User7: User5 i mean, in worker/uniter
User5: rog, +1
User5: rog, ok, sorry to keep banging on about the unit return, but it really doesn't feel right
User5: rog, and it's a little goroutine-icky, even if not technically unsafe
User7: User5 it's never used two goroutines simultaneously
User5: rog, but it crosses my mind that the *upgrader* already has the unit, and can surely send down its PathKey in the UpgradedError itself?
User5: rog, that's why I said not unsafe
User7: User5 i'm not sure i see the issue
User7: User5 runOnce only returns when none of its sub-tasks are running
User7: User5 therefore there can be no problem with goroutine ickiness
User7: User5 i may be on crack
User5: rog, I guess I just don't like a return value that is present-but-useless when no error, or most errors; present-and-useful on one specific error, and not-present on some other errors
User7: User5 it's always present :-)
User7: oh no it's not
User5: rog, so you somehow return a unit when a unit is not found?
User7: indeed
User5: rog, ISTM that the PathKey is the important thing, not the context, and that that would be a fine thing to send down with the error
User5: rog, sorry, "not the context" is kinda meaningless
User7: User5 that does assume that the agent is *always* going to be named after the PathKey.
User7: User5 otherwise the upgrader wouldn't be able to send it
User7: User5 i know what you mean about the ickiness though
User5: rog, hmm, AIUI that was the only point of PathKey in the first place
User5: rog, does Machine have one too?
User7: User5 yeah i know - that's why i wanted to call it "AgentName" ...
User7: User5 it does
User5: rog, feels to me like the way to go
User7: User5 the ironic thing is that we already have the means to make the agent name, without involving unit or machine
User5: rog, ha!
User5: rog, but then, meh, we (currently) always need the unit anyway, we may as well ask it what it thinks
User5: rog, +1 on AgentName
User7: User5 been there, it was wrong
User5: rog, understood, but it may just have been an idea before its time :)
User7: User5 maybe. perhaps i'll start with PathKey and see whether the incongruence might change niemeyer's mind
User5: rog, SGTM
User7: User5 the other thing that occurred to me is the UpgradedError could contain a closure which would do the upgrade (i.e. call ChangeAgentTools)
User7: User5 but that's probably a bit icky
User5: rog, the only thing that stopped me proposing that was a feeling that UpgradedError was weird enough already
User5: rog, but I think it would probably actually be the cleanest solution
User7: [09:23:16]  User5 but that's probably a bit icky
User5: rog, not sure -- it's already not quite right, because it's not an upgrade*d* error
User5: rog, it's actually an UpgradeReadyError
User7: User5 i dunno - it's "someone has upgraded me"
User5: rog, and if you think of it like that, a RunUpgrade method mades sense
User5: rog, the actual upgrade of the agent does not take place until the agent calls ChangeAgentTools though
User7: User5 UpgradeReadyError doesn't sound much like an error
User5: rog, in what way is it worse than UpgradedError>
User7: User5 i agree
User7: User5 you're probably right, it's just about the same
User5: rog, indeed -- the whole error idea is what still feels mildly abusive
User5: rog, but I think at least it's worth a try
User5: rog, https://codereview.appspot.com/6561063/ reviewed anyway
User5: rog, https://codereview.appspot.com/6564063/ LGTM
User7: User5 thanks
User7: User5 dammit, the upgrader can't do the actual upgrade easily
User7: User5 it doesn't have agent.Conf.DataDir
User5: rog, really? blast
User7: User5 and i'm slightly reluctant to pass that in just for this
User5: rog, doesn't it use DataDir somehow to figure out the place to put the real tools dir in the first place?
User5: rog, I *thought* the upgrader put the tools, and then just told the client that they can now symlink to the tools' known lcation
User7: User5 yes, it does indeed have dataDir around, you're right
User5: rog, cool
User7: User5 ok, np then
User8: hello.
User7: User8 mornin'
User6: User8 moin
User7: User6 hiya
User6: User7 didn't we already seen 3h ago? ;)
User7: User6 i thought maybe i hadn't :-)
User6: User7 it's ok, we both are 40+. so i know that. :P
User7: User5 do you think *all* upstart scripts we produce should have the "normal exit 0" stanza?
User7: User6 :-)
User5: rog, hmm, it demands a similar change in the machine agent, but -- for our purposes at least -- that might be a sensible way to go
User5: rog, it will not be unbearably hard to make it configurable later if we want to
User7: User5 i've already made the change to the machine agent
User5: rog, well then go for it :)
User6: release meeting time
 * davecheney waits
User6: i ping the boss
User8: aah, meeting.
User8: I forgot about this.
User6: hmm, no answer
User7: User5 this is still WIP, but hopefully is closer to what you'd like: https://codereview.appspot.com/6561063
User9: pong
User6: ah, the master
User9: hahah
User6: just wanted to collect the state of all to send it to you, but now you're here. fine.
User7: davecheney, User5 invites are out
User8: User1 User5 https://plus.google.com/hangouts/_/9dde8fe32d77795910d89838149fce4ccddea664?authuser=1&hl=en
User7: i lost conn
User7: now "we're having trouble connecting with the plugin"
User7: User5 presumably your net connection isn't good enough for a hangout this morning?
User5: rog, oh *hell* I completely forgot and was eating lunch
User5: rog, joining
User7: User5 https://plus.google.com/hangouts/_/9dde8fe32d77795910d89838149fce4ccddea664?authuser=1&hl=en
User1: all: http://paste.ubuntu.com/1247363/
User1: ^ hook, sadness
User6: User5 is the current ServiceRelationWatcher what you expected with issue #1032539?
User5: TheMue, just a mo, let me take a look; if it has Added/Removed, no
User7: User1 have you looked at the log output?
User7: User1 that's where the hook output will go, which should be diagnostic
User7: bbs
User7: User5 done, i think: https://codereview.appspot.com/6561063
User7: (live tests pass)
User5: rog, awesome, I'll take a look in a sec
User7: User5 thanks
User7: User5 trivial? https://codereview.appspot.com/6568064
User7: lunch
User5: rog, LGTM on UpgradeReadyError... oh wait there was something I meant to check
User5: rog, what's the deal with changing the arge to the machine agent?
User7: User5 hmm, let me check
User7: User5 i'm not sure what you mean. which argument?
User5: rog, I can't find it, I may be remembering from an older version
User5: rog, yeah, it doesn't exist -- sorry
User5: rog, https://codereview.appspot.com/6568064/ looks trivial to me
User7: User5 cool, will submit
User7: User5 (thanks!)
User2: Hello all!
User2: Sorry, my phone warned me about the early meeting a bit late.. did we have one?
User8: we did
User2: User8 Nice
User2: User8 Have you seen the machines watcher I've pushed?
User8: User2 still wip, found a bug and making final adjustments now.
User8: User2 no, I'll take a look.
User2: User8 https://codereview.appspot.com/6566066/
User2: User8 Thanks
User2: User8 Since we're both working on that stuff, it's good to cross-review
User2: User8 Btw, I've talked about a similar pattern to the watcher you're pushing with William in the sprint, and now it occurred to me that you were not around in the conversation
User2: User8 is it using a single goroutine, or a goroutine per unit?
User8: a goroutine for the machine plus one goroutine for each principal unit (not for subordinates).
User2: User8 I don't think we need more than a single goroutine
User2: User8 The logic ends up simpler rather than more complex with a single goroutine
User2: User8 Please have a look at RelationUnitsWatcher
User2: User8 It has a similar problem in which it has to decide what to watch as it goes
User8: User2 yeah, it can be done with only one goroutine. I'll change it after I fix this one bug. I did it this way because it matched the way I thought of the problem, but it's easy to change now.
User2: User8 It started with the same design of one-per-subcontext, and then after a conversation it got refactored to be a single goroutine
User2: User8 Yeah, I totally understand that.. it feels "right", which is why I figured I should ask
User7: User2 yo!
User2: User7 Yo!
User7: User2 after discussion with william this morning, i made quite a few changes to the uniter upgrade branch
User7: User2 i hope you find them ok...
User7: User2 https://codereview.appspot.com/6561063/
User7: User2 am just running the first live test on the machine agent with provisioner and firewaller in-built
User7: User2 if that works, we're basically there for upgrading
User2: User7 It looks like you've made changes to the upgrader itself?
User7: User2 as requested, yes
User2: User7 Why did it affect the branch, specifically?
User7: User2 because fwereade was not happy about the fact that runOnce returned the unit
User7: User2 because it was only valid sometimes
User7: User2 so the fix was to make the upgrader responsible for doing the actual upgrade itself.
User2: User7 Yeah, I'm not against it for sure.. sounds like a great idea
User2: User7 I just wish this was done by itself
User2: User7 Rather than on top of another big branch
User7: User2 yes, perhaps i should have done that. it seemed like i'd have got into a twisty mess, but it may have been ok
User2: User7 I think that's where I am right now :)
User7: User2 ok, i'll back out the changes and create another branch
User2: User7 It's fine, I'm already on it
User2: User7 if err == uniter.ErrDead {?
User2: User7 Was this merged with William's change?
User7: User2 no, this was another thing that william suggested
User7: User2 perhaps i should have left that for later too
User2: User7 How's that related to upgrading?
User2: 82         if state.IsNotFound(err) || err == nil && unit.Life() == state.Dead {
User2: ?
User2: User7 Gosh man..
User7: User2 it's related to the logic around upgrading.
User2: User7 That's related to unit lifecycle
User7: User2 william asked me to change it, so i did. i'll change it back, np
User2: User7 Which is not handled now, and as far as I understand is completely unrelated to upgrading
User2: User7 Okay, can we please go back to applying the upgrading pattern that was already in place to the unit?
User2: User7 Or, optionally, refactor the upgrading out *without* adding it to the unit
User2: User7 and then add it to the unit in a separate step
User2: User7 The branch as it is seems somewhat out of scope in whichever angle we look
User7: User2 if i just take out the lifecycle changes, would that be enough?
User2: User7 I have no idea.. right now I have a branch that has at least: 1) Add upgrade support to unit; 2) Refactor the upgrade machanism; 3) Add lifecycle support to the unit
User2: User7 I'd do them as (2), (1), and keep (3) out entirely.. William is working on it
User7: User2 ok
User2: User7 I'd also be fine with (1) + (2), though, as that's the order you did and might be easier to get back onto that state
User7: User2 that would indeed be much easier, as i did the lifecycle thing last
User2: User7 Right
User7: User2 thanks
User7: User2 i should know better when to stop what i'm doing and make a new independent branch
User2: User7 I have a hard time understanding it, to be honest
User2: User7 It's so pleasing to get small branches in..
User2: User7 Fast, painless
User2: User7 Easy for people to look at and "Hah, of course we want that"
User7: User2 yeah, but in this case, the incentive was "if i just make these few small changes, then william will be happy"
User2: User7 You can always make William happy in another branch :-)
User7: User2 yeah
User2: User7 Even more when the change is *already* huge
User6: so, late lunchtime today, afterwards an appointment outside
User8: User2 you got a review.
User6: User2 maybe i come back to you in the evening regarding the security groups, but most is already clear
User2: User8 Thanks!
User8: User2 I'll mark my branch as WIP and do some more changes.
User2: User6 Super, that branch Aram reviewed has your test, btw
User2: User8 Cool
User6: User2 yes, i've revied it too and like it
User6: reviewed
User2: User8 Can you clarify this bit: "to check that when we get an event, the lifecycle of the entity is really what we expect it to be."
User8: User2 we get []int{2,3,5}, it would be nice if we could check that 5 is alive because it was just added, 2 is dead and 3 is dying.
User2: User8 Hmm.. what are we testing?
User2: User8 This feels like testing the test itself? I mean, it's the test itself that is putting the unit to Dead
User8: the test puts the unit to dead, but perhaps the watcher misfired for whatever wrong reason, stil delivering the correct event. by checking that the unit is dead we make sure that the watcher fired for the correct reason.
User2: User8 Sorry, I don't think that's the case. The test is saying "u.EnsureDead()".. it doesn't make sense for this specific test to ask "Is u actually dead?"..
User2: User8 We have tests for EnsureDead elsewhere
User2: User8 Regarding this: "I pondered about this. If we use Select anyway, why don't we use the real document, machineDoc in this case, and ignore the fields we don't care about?"
User2: User8 I was on the fence..
User2: User8 I think you're right.. we should just use the machine doc
User7: User2 i hope this is more digestible: https://codereview.appspot.com/6561063
User2: User7 Thanks very much, looking
User7: User5 i've taken out the life-cycle-related changes - they'll go in another CL
User5: rog, ok, SGTM, so long as they come soon I won't fret :)
User2: User7 At these times I <3 Rietveld
User7: User2 yeah, being able to diff against the different stages is invaluable
User2: User7 Yeah, and diffing across the back and forth produces clean diffs
User7: User2 ah yes
User8: at previous job we had the worst review tool ever.
User8: custom made in house, probably 15 years ago.
User7: fwereade, User2 factored-out branch: https://codereview.appspot.com/6567067/
User2: User7 Reviewed
User2: User7 The first one, that is
User7: User2 tyvm
User2: User8 Nasty. I think I never had anything close to Rietveld either, to be honest
User2: So much lifetime wasted :-)
User7: User2 the cloudinit change
User7: is necessary because the upgrader uses the machine's PathKey for the agent
User7: name. I could roll that back and explicitly pass in an agent name to NewUpgrader
User7: instead, if you like.
User8: dy = -dx/(x + c1)
User8: y = -I[dx/(x + c1)]
User8: y = -ln|x + c1| + c2' = -ln|x + c1| + ln(exp(c2')) = -ln(exp(c2')|(x + c1)|) = -ln|c2(x + c1)|
User8: meh, sorry, not here.
User8: damn paste.
User2: User7 No, sounds sensible then, thank you
User7: User2 cool.
User2: User8 Curious
User7: User2 BTW for the nil error, i think maybe it would be best as: if err == nil {err = fmt.Errorf("tasks finished with no error") } or something
User2: User7 "uniter error: %v" feels very terse and clear
User7: User2 rather than "uniter error: nil" which is still not great
User7: User2 ok, will do
User7: User2 seems funny saying "error" when there's no error, that's all
User8: User2 I found an awesome hacky way of solving the not so complex differential equation x * y' + 1 = exp(y).
User2: User8 What does the equation mean?
User8: can't remember where I first encountered it, particle physics for sure. A new solution just sprang (?) in my mind randomly and I had to write it down.
User2: User7 Fair enough regarding the error message, you're right
User2: User8 That's how it generally goes :-)
User7: User2 ok, i'll go with my suggestion above, thanks
User2: User7 Thank you
User7: ah, this will be why my combined machine and provsioning agent isn't working!
User7: 2012/09/28 14:47:51 JUJU loaded invalid environment configuration: no registered provider for "ec2"
User7: not too hard to fix :-)
User8: I must specify that I've probably encountered this equation more than 7 years ago though, heh.
User8: at a particular physics olympiad.
User2: User7 Follow up reviewed
User7: User2 thanks!
User7: User2 i wondered about worker.ErrDead but considered that it might be useful for a client to be able to distinguish which worker had given the ErrDead. but maybe that's overthinking it.
User2: User8 Differential equations are surprisingly useful.. I wish I could keep the background math in my head for longer.. but haven't really ever used them in anger, so it remains just as a spare tool
User2: User7 The only agent that can be dead is the one that is running
User7: User2 ok.
User2: User7 e.g. we can't have a dead provisioning worker without a dead machine
User8: I have to step out early today guys, but I'll finish work in the weekend on the watcher.
User7: User8 have fun!
User2: User8 Have a pleasant EOD
User8: thanks.
User7: machiner/provisioner/firewaller upgrade worked live, yay!
User2: User7 WOAH
User2: I guess we're closing September in pretty good shape
User7: User2 in worker: var ErrDead = errors.New("agent object is dead") ?
User7: pwd
User7: User2 better than we were a week ago, no question :-)
User2: User7 s/object/entity/, otherwise LGTM
User7: User2 cool
User7: User2 https://codereview.appspot.com/6570063/
User7: User2 the final piece of the puzzle
User2: User7 Looking
User7: User5 https://codereview.appspot.com/6570063/
User5: rog, ack
User5: rog, that is awesome
User5: rog, LGTM
User7: User5 cheers!
User7: User5 i need to do another branch to pass the state into the Machiner directly, i think.
User7: User5 or maybe niemeyer will call that out in this branch
User7: User5 actually, nope, it's not a problem, cool.
User2: User7 Done
User7: User2 thanks!
User2: User7 Very cool
User7: User2 it was very easy and *almost* worked first time...
User2: User7 Great stuff
User2: Lunch is calling
User2: biab
User7: right upgrades are GO!
User7: User9 ^
User9: User7 awesome
User7: User9 the only thing we don't have currently is the --bump-version functionality.
User7: User2 stage 1 of --bump-version: https://codereview.appspot.com/6560066
User2: User7 Looking
User2: User7 Done
User7: User2 thanks!
User2: User7 My pleasure
User7: User2 BTW, about --bump-version:
User7: User2 i'm wondering when we *don't* want it enabled
User7: User2 if we're uploading tools to private storage
User2: User7 Any time checking out a release and uploading it
User7: User2 why would we want to do that without bumping the build version? nothing will see the new release.
User2: User7 Bumping the version is a hack.. uploading tools to private storage is not
User2: User7 People can upload tools to private storage in their own cloud, with a real juju release
User7: User2 ok. should we make it an error if the tools already exist in the storage then?
User2: User7 How's that connected to the above?
User7: User2 because otherwise it's likely to be a mistake, and people might be surprised when nothing happens.
User2: User7 When you copy a file to a location that already contains the same file, nothing happens other than the same file being in the same place
User2: User7 That's not surprising
User7: User2 i'm not sure. in this case, it may well feel like we're asking juju to use a particular version.
User7: User2 which it will be, just not the version we've just uploaded
User2: User7 It will be the version we just uploaded if that's the version that is being used
User2: User7 I don't see the problem, really
User2: User7 --upload-tools uploads tools.. that's all
User7: User2 well, the problem there is we've got two "versions" of the s/w with the same version number
User2: User7 If people expect more than this, they'll be wrong
User7: User2 it's actually "upgrade-juju --upload-tools" and the problem i see is in the first word there.
User2: User7 That means you've changed the release
User7: User2 yeah - we've changed the release but the version number hasn't changed. that feels a bit like a mistake to me.
User7: User2 i'm trying to imagine a situation where it's useful to overwrite the existing tools for a given version
User2: User7 If you checkout 2.2.0, that's 2.2.0
User2: User7 Not 91.69.162.6, not 91.69.162.61
User7: User2 ok, that's fine.
User2: User7 We're doing development, and we have tools to do what we want when we have to
User2: User7 Thus --bump-version
User7: User2 if someone has already checked out and uploaded 2.2.0, is it useful to them to be able to overwrite the existing 2.2.0 in storage?
User2: User7 Don't know.. doesn't look like a problem we should worry about right now
User7: User2 (i'm not suggesting automatic --bump-version BTW)
User7: User2 ok
User2: User7 If people ask to upload, and it uploads, sounds fine
User2: User7 We can fine tune that behavior over time too.. anything we think right now will likely be wrong in the real world
User7: User2 ok, seems reasonable
User7: User2 submitted. ... and with that, it's time for me to call it a week. oh, i might sign off a few bugs first :-)
User2: User7 Super, thanks for the great stuff this week
User7: User2 got there in the end :-)
User2: User7 Yeah
User7: fwereade, Aram, niemeyer, everyone else: have a great weekend!
User5: rog, and yourself :)
User2: User7 Thanks, you too!
User5: niemeyer, btw, I just put a couple of trivials in the queue
User2: User5 I'll look right now
User2: User5 Done
User2: User5 Done
User5: niemeyer, lovely, thanks
User2: User5 My pleasure.. have a couple of questions on the second one
User5: niemeyer, the bits you're asking about are basically the same
User5: niemeyer, if you do a plain waitHooks{}, it means, "Iexpect no further hooks to have been run"
User5: niemeyer, the initial StartSync is to poke the uniter into responding to events that might cause spurious events to happen
User5: niemeyer, the subsequent ones are to make sure that it responds to state changes in a timely way
User5: niemeyer, sound sensible?
User2: User5 Ah, yeah, thanks.. I suggest dropping it down to, say, 100ms then, since this is the happy case
User5: niemeyer, SGTM
User2: User5 We're getting a second every 5 of those, otherwise
User5: niemeyer, indeed, dropping it will not hurt at all
User5: niemeyer, ok to submit with that change?
User2: User5 LGTM
User5: niemeyer, cheers
User5: hmm, got to go out in a mo, actually, but they should be in soon enough :)
User2: User5 Have a good weekend, and thanks for the hard work too.. feels like we're on the runway for a fully working implementation
 * niemeyer steps out for a while
juju-dev
Ubuntu IRC