[06:18] <TheMue> Morning
[06:39] <fwereade_> morning TheMue, davecheney, rog
[06:39] <davecheney> howdy
[06:42]  * fwereade_ looks back at davecheney's conversation
[06:43] <fwereade_> davecheney, interesting, sorry to lead you astray on /initialized
[06:43] <davecheney> fwereade_: no harm done
[06:43] <fwereade_> davecheney, I always saw it as meaning "if this node does not exist, do not trust the state, because it is wrong"
[06:43] <davecheney> two commits today, so ain't nothing going to bring me down :)
[06:43] <fwereade_> davecheney, sweet :)
[06:55]  * TheMue would like to take his whole family with him to Lisbon. It's raining cats and dogs here today.
[06:56] <TheMue> Maybe I should try a summer2012 *= -1
[07:25] <rog> TheMue, fwereade_: mornin'!
[07:27] <TheMue> rog: Heya
[07:39] <fwereade_> rog, heyhey
[09:14] <davecheney> rog: can I ask you a question about gnu flags ?
[09:14] <rog> davecheney: ask away
[09:14] <davecheney> i'll describe what I want to do, not what I am currently doing
[09:14] <davecheney> to support multple output formatters
[09:14] <davecheney> I have a map[string]func(io.Writer)
[09:14] <davecheney> which maps format names to a function that will format the output
[09:15] <rog> davecheney: ok
[09:16] <davecheney> but i'm having trouble satistfying the interface for whatever fs.Var takes
[09:16] <davecheney> which needs Set() and String()
[09:16] <davecheney> ideally i'd like to avoid having to make a struct { name string render func() } for each map entry
[09:16] <rog> davecheney: you want a flag that selects the format?
[09:16] <davecheney> just so we knew it's name
[09:16] <davecheney> yes
[09:17] <davecheney> that defaults to yaml
[09:17] <davecheney> rog: please hold, pasting
[09:17] <davecheney> rog: http://paste.ubuntu.com/1097972/
[09:17] <davecheney> this may not work exactly, i've been hacking on it
[09:18] <Aram> moin.
[09:18]  * davecheney waves
[09:19] <rog> davecheney: so your difficulty is that you don't know how to define the String method on the formatterVar?
[09:20] <davecheney> rog: not without having to pass a struct with the name of the formatter around
[09:20] <davecheney> I was trying to keep it lean using only a function
[09:20] <rog> davecheney: why not just do: type formatterVar string; and then index into the formatter map at a later stage?
[09:21] <rog> davecheney: or have another method on formatterVar that returns the function.
[09:21] <davecheney> the latter might be good
[09:21] <rog> davecheney: by looking up into the map.
[09:21] <fwereade_> davecheney, take a look at cmd/jujuc/server/output.go
[09:21] <rog> davecheney: all this stuff is identical to the standard flag package BTW
[09:21] <davecheney> yeah, i've been looking in cmd/cmd and cmd/jujud for hints
[09:22] <davecheney> give me a sec to play
[09:22] <davecheney> thnks for your suggestions
[09:22] <fwereade_> davecheney, the file I suggested should have ~exactly what you need already implemented
[09:22] <davecheney> fwereade_: sweeeeeeeeeeeet
[09:22] <rog> fwereade_: i *thought* i'd seen it before!
[09:23] <fwereade_> :D
[09:23] <davecheney> fwereade_: that is _exactly_ what I want
[09:23]  * davecheney smells refactoring
[09:23] <fwereade_> davecheney, awesome :)
[09:24] <TheMue> Aram: Moin
[09:24] <davecheney> fwereade_: can I move this to cmd/ ?
[09:24] <fwereade_> davecheney, please do :)
[09:24] <davecheney> as it will be used by several cmds ?
[09:25] <fwereade_> davecheney, SGTM, you may want to write a couple of explicit tests for it
[09:25] <davecheney> fwereade_: will do
[09:27] <fwereade_> davecheney, beware the --test flag, which we probably don;t want on juju status
[09:28] <fwereade_> davecheney, but you should probably keep --output, pointless though it may be, in the service of interface stability
[09:36] <davecheney> fwereade_: in that case, may I move c.out.testmode out of the formatter
[09:37] <davecheney> as test mode is only used by the jujuc/server types
[09:39] <Aram> fwereade_: there's a problem with multiple testing suites in mstate :).
[09:40] <Aram> starting a replicated mongod takes 30 seconds, so tests take 200 seconds just to bootstrap!
[09:40] <Aram> I tried to make mongod start faster but ti can't be done.
[09:40] <fwereade_> Aram, whoa :)
[09:40] <fwereade_> davecheney, SGTM
[09:41] <fwereade_> Aram, you *should* be able to do something like we do in testing
[09:41] <fwereade_> Aram, so at least you share a mongo between tests in every package
[09:41] <Aram> that's the plan I had in mind.
[09:41] <davecheney> lbox propose over celular connection will probably be a fail
[09:41] <fwereade_> Aram, and surely you can run most tests without a replicated mongod?
[09:42] <Aram> that's another plan I had in mind.
[09:43] <fwereade_> Aram, STM that running most of them without replication, but the actual mstate suite with, would be pretty plausible
[09:45] <davecheney> fwereade_: rog: https://codereview.appspot.com/6426043
[09:46] <fwereade_> davecheney, could we not have testMode on ClientContext please?
[09:46] <davecheney> actaully, please hold, still massaging
[09:47] <davecheney> fwereade_: indeed, i moved it there
[09:47] <fwereade_> davecheney, emphasis on "not"
[09:47] <davecheney> it's in server.go, i'll move it into context.go
[09:47] <fwereade_> davecheney, the commands do embed a ClientContext but I don't think the ClientContext is "commandy"
[09:48] <davecheney> fwereade_: ahh, my mistake, i misinterprited you
[09:48] <fwereade_> davecheney, and I *think* that not every ClientContext-embedder actually wants that flag
[09:48] <davecheney> only config-get and unit-get care about this
[09:48] <davecheney> i'll add it explicitly to them
[09:48] <fwereade_> davecheney, yeah, there will be more but you don't need to worry about them yet
[09:49] <fwereade_> davecheney, I'm worrying about how I'm going to make all these things work enough for everybody ;p
[09:54] <davecheney> fwereade_: PTAL
[09:57] <fwereade_> davecheney, LGTM modulo tests :)
[09:58] <davecheney> adding some testage to TestCommand now
[10:45] <davecheney> fwereade_: rog aram: comments most welcome on tests "{\"Juju\":1,\"Puppet\":false}\n"
[10:45] <davecheney> err, "{\"Juju\":1,\"Puppet\":false}\n"
[10:45] <davecheney> https://codereview.appspot.com/6426043
[10:48] <rog> davecheney: does that test pass consistently?
[10:48] <davecheney> rog: yup
[10:48] <davecheney> i use a struct for the test data so I don't get screwed by map ordering
[10:48] <rog> davecheney: oh yeah, it's struct not a map, duh
[10:48] <davecheney> rog: don't worry, i made that mistake
[10:49] <davecheney> i added a comment to warn othrs
[10:49] <davecheney> others
[10:50] <rog> davecheney: i wonder whether instead of specifying the entire error output, it might be better to make a regexp that just includes the most important bits.
[10:51] <davecheney> rog: yeah, i'll make that change
[10:51] <rog> davecheney: e.g. `usage.*invalid value \"xml\" for flag.*`
[10:51] <davecheney> that was overkill
[10:51] <rog> davecheney: actually i think it may have to be (.|\n)*
[10:52] <davecheney> Assert( Equals ) is already a regex right ?)
[10:52] <rog> davecheney: also, perhaps use a more-obviously unrecognised name than "xml"?
[10:52] <rog> davecheney: nope, use Assert(Matches)
[10:52] <rog> davecheney: it's just plausible we may add xml support in the future, i guess
[10:53] <davecheney> rog: i can invent a more implausible output format
[10:53] <rog> davecheney: i'm sure you can :-)
[11:06] <rog> davecheney: you've got a review
[11:08] <davecheney> thanks rog
[11:13] <davecheney> rog: Assert Matche is having trouble with a multi line message
[11:13] <davecheney> matches
[11:14] <rog> davecheney: did you use (.|\n)* ?
[11:14] <davecheney> no ...
[11:14] <rog> davecheney: i think you need to, as . doesn't match \n
[11:15] <rog> TheMue: i wonder if another way of testing the internals of the firewaller might be to hook into the logger.
[11:15] <davecheney> that go it
[11:15] <davecheney> thanks
[11:16] <rog> TheMue: you could make a little type with an Output method that grepped for certain strings.
[11:16] <rog> TheMue: then there would be no need to have unsafe access to firewaller internals
[11:26] <davecheney> rog: holy crap
[11:26] <davecheney> if you include a CL url in the body of lbox commit message
[11:27] <davecheney> it appends it to the old review
[11:27] <davecheney> farq!
[11:27] <rog> davecheney: you mean lbox submit ?
[11:27] <davecheney> no, look what happened here
[11:27] <davecheney> https://codereview.appspot.com/6405046#msg14
[11:28] <rog> *sigh* random emails are going missing, and i don't know why
[11:29] <rog> davecheney: in particular, i never saw the Submitted email, so i thought i was replying to an active review
[11:29] <davecheney> yeah, that happens a lot
[11:29] <rog> davecheney: i wonder why that message came out yellow
[11:30] <davecheney> but I use my gmail for reviews
[11:30] <davecheney> which is more reliable
[11:30] <rog> davecheney: me too
[11:30] <rog> davecheney: i use gmail for everything
[11:30] <rog> davecheney: and it worked fine before
[11:30] <rog> davecheney: i verified that it wasn't any of my filters misbehaving, and it's not going into Spam.
[11:30] <davecheney> rog: https://codereview.appspot.com/6416044/
[11:31] <davecheney> rog: email totally crapped out last week, thursday/friday
[11:31] <davecheney> 24+ hours delays
[11:31] <davecheney> maybe this is a continuation
[11:31] <rog> davecheney: email from launchpad was always very slow, but it got there in the end.
[11:31] <rog> davecheney: these days, i'm not sure i'm seeing any email from launchpad at all.
[11:33] <davecheney> weird
[11:50] <TheMue> rog: Instead of logging I also could add a chan of interface{} and send testing data to a goroutine where it's simply added to a slice. Additionally I can send funcs to that goroutine which iterate and type switch over the collected data.
[11:57] <rog> TheMue: i like the logging approach because it doesn't make the core code do anything unusual
[11:57] <rog> TheMue: there's no need to be sending to a chan in the normal code flow
[11:58] <rog> TheMue: and having a log.Debugf call would look quite normal
[11:58] <rog> TheMue: you could parse the messages trivially, with fmt.Scanf
[12:02] <TheMue> rog: Have to think about. I'm not such a big fan of string-stream-data-fetching. ;)
[12:28] <TheMue> rog: Thx for your review. How do we handle ZK outages in other parts?
[12:29] <rog> TheMue: i think some tests close the underlying zk and test the behaviour
[12:29] <rog> TheMue: or perhaps the underlying state.
[12:29] <rog> TheMue: it's not easy, but i think it's worth doing as much of the logic is about the error paths, so it's important that we get it right.
[12:30] <TheMue> rog: Btw, the better handling of a dying machine units watcher is in the next branch (goes in today). It hasn't been complete so far.
[12:31] <rog> TheMue: i'm thinking about the handling of a dying machine watcher in this case
[12:31] <TheMue> rog: I can follow your motivation but I'm not sure how you would do it.
[12:32] <rog> TheMue: here's a thought: currently the firewaller opens its own state. how about we change the signature to NewFirewaller(*state.State)?
[12:33] <rog> TheMue: then we can pass in a state that we have a handle to, and can close that and see what happens
[12:33] <rog> TheMue: in fact, that enables both the PA and the firewaller to use the same state object, which is probably a good thing.
[12:34] <TheMue> rog: Would be ok for me, yes.
[12:35] <TheMue> rog: Could you add this as a review comment so that Gustavo could take a look at that idea too?
[12:36] <rog> TheMue: done
[12:36] <TheMue> rog: thx
[12:37] <TheMue> rog: I'll now finalize the follow-up branch as proposal and then return to the initial one
[12:37] <rog> TheMue: sounds good
[12:57] <rog> TheMue: here's one way of doing the log-based tests. i think it works quite naturally, but it may be crack! http://paste.ubuntu.com/1098250/
[12:57] <rog> fwereade: what do you think of the above approach?
[12:57]  * fwereade reads
[13:00]  * fwereade isn't sure
[13:01] <fwereade> rog, I think it's decent scaffolding but bad testing if you see what I mean
[13:01] <fwereade> rog, it will help us verify behaviour as we build up the firewaller
[13:01] <rog> fwereade: that's the idea
[13:01] <fwereade> rog, but I think that once we can test the whole thing in terms of dummy Operations we should trash it
[13:01] <rog> fwereade: i agree
[13:02] <fwereade> rog, I think we're on the same page then :)
[13:02] <rog> fwereade: i just wanted something that was unintrusive but reliable
[13:02] <rog> fwereade: the current approach of inspecting the firewaller's locals from another goroutine seems icky to me
[13:03] <rog> TheMue: hey, i just saw your CloseState method again
[13:03] <rog> TheMue: that approach would be fine, i think.
[13:04] <fwereade> rog, yep, definitely
[13:05] <rog> TheMue: maybe we don't need more error checking currently in fact
[13:06] <TheMue> rog: Eh, sorry, somehow I'm lost.
[13:07] <rog> TheMue: lost where?
[13:07] <TheMue> rog: From the paste above (which I'm reading now and so maybe answered too quick) and the last statement about CloseState and "don't need more".
[13:08] <rog> TheMue: the paste above is orthogonal to the error testing issue that i was just referring to
[13:09] <rog> TheMue: sorry, two things without sufficient disambiguation!
[13:09] <rog> TheMue: i'm suggesting the paste above as a way to avoid the need for the thread-unsafe AllMachines methods.
[13:09] <TheMue> rog: ah, ok
[13:10] <rog> TheMue: it's the kind of thing i was referring to earlier when i talked about hooking into the log messages
[13:13] <TheMue> rog: yes, ic, your approach is nice. i only have to add some more debug statements then.
[13:13] <rog> TheMue: exactly
[13:13] <rog> TheMue: cool, glad you like it
[13:14] <rog> TheMue: if you like, i'll submit it as another CL after yours is done.
[13:14] <rog> TheMue: you could leave a "TODO: thread safe testing" or somesuch comment lying around
[13:15] <TheMue> rog: can do so, but I can also add it myself and update the proposal
[13:15] <rog> TheMue: ok, that's probably easier tbh
[13:16] <rog> biab
[13:16] <TheMue> rog: I'm just testing the follow-up branch, then I can start with it
[13:16] <TheMue> niemeyer: morning
[13:19] <niemeyer> Gooood mornings
[13:19] <rog> niemeyer: yo!
[13:21] <niemeyer> rog: Heya!
[13:40] <rog> niemeyer: in https://bugs.launchpad.net/bugs/1017732, you say "The two first parameters are only useful for InferCharm" but there's not currently a function by that name. did you mean InferURL?
[13:42] <niemeyer> rog: Yeah
[13:43] <rog> niemeyer: ok. next branch up, i'll propose a fix. i want to make the argument to juju.Conn.Deploy a charm.URL
[13:44] <rog> niemeyer: which means i want InferRepository as you suggest it
[13:44] <niemeyer> rog: Sounds great, cheers
[13:45] <rog> fwereade: sound ok to you? i'll assign the bug to me if you've not already got a fix in progress.
[13:47] <niemeyer> TheMue: ping
[13:53] <fwereade> rog, sorry, was having a bite to eat, forgot lunch, reading back
[13:53] <fwereade> niemeyer, heyhey
[13:53] <rog> fwereade: np
[13:54] <fwereade> rog, go for it :)
[13:54] <rog> fwereade: done
[13:55]  * fwereade cheers
[13:55] <TheMue> niemeyer: pong
[13:55] <TheMue> niemeyer: seen your +1 an will re-propose it later
[13:55] <niemeyer> TheMue: I'm just sending a few other notes
[13:55] <niemeyer> TheMue: Can you please see if you'd like to talk about something there?
[13:55] <TheMue> niemeyer: thx
[13:56] <niemeyer> fwereade: Heya!
[13:58] <fwereade> niemeyer, heyhey
[13:58] <fwereade> niemeyer, how's it going?
[14:01] <niemeyer> fwereade: Smoothly!
[14:01] <niemeyer> :)
[14:01] <fwereade> niemeyer, sweet :D
[14:03] <TheMue> niemeyer: Only one question regarding your first comment. You would prefer a stopping of the firewall using the typical way?
[14:04] <niemeyer> TheMue: Hmm
[14:04] <niemeyer> TheMue: You know what, I think the implementation is right
[14:04] <niemeyer> TheMue: It's the comment that is wrong
[14:05] <niemeyer> TheMue: "can't stop machine tracker: %v"
[14:05] <niemeyer> TheMue: The machine tracker *was* stopped, if we got there
[14:06] <niemeyer> TheMue: Ah, and it should also delete the machine from the map
[14:10] <TheMue> niemeyer: this is done below with delete. the error is the the result of a stopping error. that's why I log "can't stop the tracker". the machine has been removed, but the tracker showed an error during stopping.
[14:11] <TheMue> niemeyer: maybe better "stopping machine tracker %d showed errer %v"
[14:11] <niemeyer> TheMue: See the comment in the CL
[14:11] <TheMue> s/errer/error/
[14:11] <niemeyer> TheMue: If you 'continue', the delete is never reached
[14:12] <niemeyer> TheMue: Yes, but "can't stop" is incorrect
[14:12] <niemeyer> TheMue: It won't get there unless it is stopped
[14:13] <TheMue> niemeyer: iiirks, the continue, you're right
[14:15] <TheMue> niemeyer: any good idea for the logging message? or shall i take that "stopping machine tracker %d: %v" thingy?
[14:16] <niemeyer> TheMue: Have you seen the CL comments?
[14:16] <niemeyer> TheMue: There's a suggestion there
[14:17] <TheMue> niemeyer: didn't refreshed screen while we're typing here
[14:18] <TheMue> niemeyer: rietveld needs auto-refresh. ;)
[14:18] <niemeyer> TheMue: Yeah :)
[14:20] <niemeyer> TheMue: What about the channel closing?
[14:22] <TheMue> niemeyer: you've seen rogs comment? it's the firewallers channel, only passed to the tracker. I could move it up. it can only be closed by the firewaller. so my fw.finish() has to take care to first stop the machine trackers befor closing that channel.
[14:23] <niemeyer> TheMue: Hold on tight there
[14:23] <niemeyer> :)
[14:23] <niemeyer> TheMue: The question was pretty simple
[14:23] <niemeyer> TheMue: What happens if the channel is closed?
[14:23] <niemeyer> TheMue: The sentence should start with "When it is closed, ..." :-)
[14:24] <TheMue> niemeyer: "When it is closed, no tracker should be running."
[14:24] <niemeyer> TheMue: Okay, that sounds perfect, thanks
[14:24] <TheMue> niemeyer: ;)
[14:25] <niemeyer> TheMue: We need to have that idea clear in there
[14:26] <TheMue> niemeyer: Yes, and I have an idea how to ensure it, based on rogs suggestion
[14:27] <TheMue> niemeyer: the next CL will have it, it's a simple one
[14:27] <TheMue> niemeyer: btw, maybe you could help me
[14:27] <niemeyer> TheMue: Right now it's a puzzle that is being carefully put together, not easy to assess whether it is right, and very easy to screw up in the next change
[14:27] <niemeyer> TheMue: If nothing else, we must document how to not screw up
[14:27] <TheMue> niemeyer: I know change sets and their acronym CS, but what is the L in CL?
[14:28] <niemeyer> TheMue: change list, I don't know where it comes from, to be honest
[14:28] <TheMue> niemeyer: ah, thought so but not have been sure
[14:29] <niemeyer> TheMue: In fact, I think we have a problem in there
[14:29] <TheMue> niemeyer: I think the next propose will it make more clear
[14:30] <TheMue> niemeyer: fw.loop() has a defer fw.finish()
[14:30] <TheMue> niemeyer: there first all trackers are stopped
[14:30] <niemeyer> TheMue: Ah, no, the problem I imagined just now doesn't exist
[14:30] <TheMue> niemeyer: and then the channels closed (possible by making them to firewaller fields).
[14:32] <niemeyer> TheMue: Btw, why do we have to wait for the firewaller.tomb.Dying within the machineTracker?
[14:32] <TheMue> niemeyer: I think that's a clear tear down. and by referencing fw.xyzChan in the trackers it is more clear that the change is sent back to be handled there
[14:33] <niemeyer> TheMue: Ah, yes, that sounds good
[14:33] <niemeyer> TheMue: You mean removing the defers and moving them into the finish method, right?
[14:33] <TheMue> niemeyer: yes
[14:33] <niemeyer> TheMue:+1!
[14:34] <TheMue> niemeyer: which line you're referring to with the wait?
[14:34] <niemeyer> TheMue: Hmm..
[14:34] <niemeyer> TheMue: So why do we even close the machineUnitsChanges channel?
[14:35] <niemeyer> TheMue: I don't think we have to close it.. there's no benefit, apparently
[14:35] <TheMue> niemeyer: you mean it's ok to let the GC close them?
[14:35] <niemeyer> TheMue: It's just introducing a race we have to be aware of
[14:35] <niemeyer> TheMue: No, the GC doesn't close channels
[14:35] <niemeyer> TheMue: Ever
[14:35] <niemeyer> TheMue: The GC just collects them, once they are unused
[14:36] <niemeyer> TheMue: It's fine to leave a channel unclosed
[14:36] <niemeyer> TheMue: If it makes sense for the intended workflow
[14:37] <TheMue> niemeyer: yeah, thought a bit about it.
[14:38] <TheMue> niemeyer: most important is a proper closing of the trackers bottom up.
[14:38] <niemeyer> TheMue: So, +1 on moving stuff onto finish, +1 on dropping the unnecessary close of the channel too
[14:39] <niemeyer> TheMue: So, going back a bit, the line I was referring to is 132 and 140
[14:43] <fwereade> niemeyer, quickly, please confirm: a hook is allowed to set the running unit's relation state for any relation the unit is in... right?
[14:43] <fwereade> niemeyer, s/running/local/ is perhaps clearer
[14:44] <niemeyer> fwereade: Yes
[14:44] <fwereade> niemeyer, jolly good, thanks :)
[14:44] <niemeyer> fwereade: There are defaults, but yes, it is allowed
[14:44] <fwereade> niemeyer, yep
[14:44] <TheMue> niemeyer: thinking about it. don't know anymore *argh*
[14:44] <fwereade> niemeyer, just wanted to check I was following the python correctly
[14:44] <TheMue> niemeyer: currently see now reason
[15:00] <niemeyer> fwereade: Now that I think of it, I'm not sure if Python ever did this correctly, since we figured those details as we went through it
[15:01] <niemeyer> fwereade: I've recently seen bug reports addressing precisely this issue, IIRC
[15:01] <niemeyer> fwereade: May have been fixed by now
[15:01] <fwereade> niemeyer, hmmmmm :/
[15:01] <fwereade> niemeyer, ok, I'll take a look :)
[15:01] <niemeyer> fwereade: Nothing to worry about, I think.. it was just not something we focused a lot on as we got the basic semantics right in the first go
[15:02] <fwereade> niemeyer, ah, jolly good
[15:02] <niemeyer> fwereade: Your feeling of "all hooks should be able to see and touch this" is the right one
[15:02] <fwereade> niemeyer, still worth double checking what went wrong so I don;t make the same mistake
[15:02] <niemeyer> fwereade: Good point
[15:02] <fwereade> niemeyer, all my mistakes will be new and fresh :)
[15:03] <niemeyer> fwereade: LOL
[15:03] <niemeyer> fwereade: Fresh mistakes just leaving the oven are always so tasty, though!
[15:04]  * fwereade runs his tummy
[15:04] <fwereade> er, rubs
[15:06] <fwereade> niemeyer, cool, nothing looks likely to be a serious problem
[15:07] <TheMue> So, switching branches
[15:28] <rog> niemeyer: https://codereview.appspot.com/6430044
[15:29] <rog> niemeyer: should be quite trivial
[15:35] <niemeyer> rog: Clean and sweet
[15:37] <rog> niemeyer: thanks!
[15:37] <niemeyer> rog: Btw, if you add -bug to lbox propose, it will link the bug to the proposal
[15:38] <rog> niemeyer: ah, thank you for reminding of that.
[15:38] <niemeyer> rog: and assign to you, and put it in progress, but hopefully those two should be done ahead of time (or with -wip)
[15:41] <rog> niemeyer: presumably then lbox submit should mark the bug as "fix committed" ?
[15:58] <niemeyer> That's lunch time!
[16:07] <rog> fwereade, niemeyer: i'm thinking of a signature something like this for juju.Conn.Deploy: http://paste.ubuntu.com/1098537/
[16:07] <rog> fwereade: what d'ya think?
[16:09] <fwereade> rog, looks reasonable, I think
[16:09] <fwereade> rog, the charm is the potentially-fuzzy charm name from the command line, yes?
[16:09] <rog> fwereade: yeah
[16:09] <fwereade> rog, yeah, that looks right, I think
[16:10] <rog> fwereade: i was wondering about using charm.URL, but i think better to keep the interface simpler, i think.
[16:10] <rog> fwereade: i don't think it loses any generality
[16:11] <fwereade> rog, I don't think you could even if you wanted, could you?
[16:11] <rog> fwereade: no?
[16:11] <fwereade> rog, we don't necessarily know the charm URL until we've messed around using potentially both state's default-series and the LocalRepoPath
[16:12] <rog> fwereade: i was going to assume that the caller would invoke charm.InferURL
[16:12] <rog> fwereade: (for which you don't need LocalRepoPath, incidentally)
[16:13] <fwereade> rog, good point; but they *do* need State
[16:13] <rog> fwereade: really?
[16:13] <rog> fwereade: i didn't think InferURL used State
[16:13] <fwereade> rog, where are you going to get default-series from?
[16:13] <rog> fwereade: ah!
[16:14] <rog> fwereade: an excellent point sir
[16:14]  * fwereade bows graciously
[16:16] <TheMue> rog: integrated your log testing, works like a charm
[16:16] <rog> TheMue: cool. i hoped you'd like it!
[16:16] <TheMue> rog: definitely
[16:17] <rog> TheMue: see, string-stream-data-fetching can be nice sometimes... :-)
[16:17] <fwereade> rog, I have a naming problem
[16:17] <rog> fwereade: god, me too
[16:17] <fwereade> rog, I'm writing a HookQueue
[16:17] <rog> :-)
[16:17] <TheMue> rog: well said
[16:18] <rog> fwereade: ok
[16:18] <fwereade> rog, and I basically want to pop hook executions in 2 stages
[16:18] <TheMue> rog: the test-by-string-stram-data-fetching-and-comparing-idiom
[16:19] <fwereade> rog, I'd like to call the operations Peek and Pop, except Peek is the thing that returns something and we should panic if we Pop without havng Peeked
[16:19] <fwereade> rog, that I want to do this at all *may* be evidence of total insanity on my part in the first place, but I think it'll all work out rather nicely
[16:19] <rog> fwereade: you could make the Pop operation work on the thing just peeked i suppose
[16:20] <fwereade> rog, it actually has to
[16:20] <rog> fwereade: why do you need a peek operation BTW?
[16:20] <rog> fwereade: perhaps you could paste a brief outline of your basic design intentions here
[16:20] <fwereade> rog, I'll give it a god
[16:21] <rog> fwereade: i've got a spare god if you need one
[16:21] <fwereade> rog, wait, balls, thought of a problem, need to think a sec
[16:22] <fwereade> rog, ah no it's ok
[16:22] <fwereade> rog, OK
[16:22] <fwereade> rog, the idea is that the HookQueue is a magical change-collapsing queue
[16:22] <fwereade> rog, we feed it RelationUnitsChange events
[16:22] <rog> fwereade: yeah, i figured something of the kind
[16:23] <fwereade> rog, and it decomposes them into individual hook executions
[16:23] <rog> fwereade: i still don't quite see how Peek fits in though
[16:23] <fwereade> rog, of which there should only be one of joined/changed/departed per unit in the queue at a time
[16:23] <fwereade> rog, the context is kinda necessary I think
[16:23] <rog> fwereade: also, Get is probably a better name for a queue than Pop, which implies a stack to me.
[16:24] <fwereade> rog, yeah, good point, bad connotations
[16:24] <fwereade> rog, ok, anyway
[16:24] <fwereade> rog, we always can collapse two potential hooks
[16:25] <fwereade> rog, if we get a changed on top of a changed, we can just ignore it
[16:25] <rog> fwereade: yup
[16:25] <fwereade> rog, if we get a departed on top of a changed, we drop the changed and append the departed, etc
[16:25] <rog> fwereade: uh huh
[16:25] <fwereade> rog, so, this is nice
[16:26] <rog> fwereade: ah, i think i see the need for Peek now
[16:26] <fwereade> rog, but I would like to be able to maintain all the hook queue state in one place
[16:26] <fwereade> rog, ie the HookQueue saves its state after every change, so it can reload itself if we restart
[16:27] <rog> fwereade: is that actually necessary?
[16:27] <fwereade> rog, I think so...
[16:27] <fwereade> rog, something needs to know what state we think we're in
[16:28] <TheMue> rog: propose is in again
[16:28] <rog> fwereade: so your HookQueue kinda corresponds to my "intentions" except that you save all the intentions at every step
[16:28] <fwereade> rog, and I think the HookQueue has basically all of it that we need
[16:28] <fwereade> rog, kind of; but an intention is not all we need to save, is it?
[16:29] <rog> fwereade: i'm not sure. what else do we need to save?
[16:29] <fwereade> rog, we need to keep track of relation membership and settings versions so far as the unit currently knows
[16:31] <rog> fwereade: ok, yes, in my original sketch, the intention was saved along with the version that corresponded to that intention
[16:31] <fwereade> rog, where do you keep the versions that don't correspond to a queued intention?
[16:31] <rog> fwereade: do they matter?
[16:32] <fwereade> rog, yes, we don't want to run a change hook for every unit whenever we bounce the process
[16:32] <fwereade> rog, how else do we reconcile latest state with saved state and know which changes are worth hooking?
[16:33] <rog> fwereade: we've already run hooks for those units, right?
[16:33] <rog> fwereade: so there will be an intention stored in the log with a version number
[16:33] <fwereade> rog, ah, ok, the intentions log grows without bound?
[16:33] <rog> fwereade: (it will be also be tagged as "done" in the log)
[16:34] <rog> fwereade: not without bound - we can collapse it occasionally
[16:34] <rog> fwereade: like a log-structured filesystem
[16:34] <rog> fwereade: we'd probably collapse it when we restart and if it grows bigger than some bound
[16:35] <fwereade> rog, hmm, how do we collapse the intentions we don't actually need to run?
[16:36] <rog> fwereade: each intention has some tag associated with it (originally i thought the zk path, but we don't have access to that) - we collapse intentions with the same tag.
[16:37] <rog> fwereade: it's quite possible that this scheme is entirely crackful, but i'd like to have a go to see if it is possible, because i *think* it could work nicely.
[16:37] <fwereade> rog, I don't see how we do all this collapsing without writing it all the time, though
[16:37] <rog> fwereade: it also potentially scales reasonably well too.
[16:38] <fwereade> rog, not to mention marking them as done
[16:38] <rog> fwereade: we *are* writing it all the time
[16:39] <fwereade> rog, ah, I thought you were claiming there was something different about what we were doing, and that my approach was a problem because I was writing all the time
[16:39] <rog> fwereade: the collapsing happens occasionally, by reading through the entire log
[16:39] <rog> fwereade: no
[16:39] <rog> fwereade: the problem i have is that your approach writes *everything* all the time
[16:40] <rog> fwereade: if the system grows quite big, there are going to be many many units and many many changes...
[16:40] <fwereade> rog, ...and a vast log file to scan through and write "done" in the middle of...?
[16:41] <rog> fwereade: we never write into the middle of the log file
[16:41] <rog> fwereade: it's append-only
[16:41] <fwereade> rog, ah, ok, that does make more sense
[16:43] <rog> fwereade: presumably in your approach you'd do something like alternate two files, writing the HookQueue alternately to each one?
[16:43] <fwereade> rog, basically yeah
[16:43] <fwereade> rog, I'm still trying to figure out how to do the constant event collapsing with the log approach
[16:44] <fwereade> rog, if we're lagging at all behind "now", which I imagine we will in a big system, how do we strip unwanted intentions from the log?
[16:45] <rog> fwereade: the hook queue event collapsing?
[16:45] <fwereade> rog, we add both "done"s and "whoops-don't-bother"s?
[16:45] <rog> fwereade: or the log collapsing?
[16:45] <fwereade> rog, the hook queue
[16:45] <fwereade> rog, the log collapsing doesn't sound too challenging
[16:45] <rog> fwereade: i'm thinking that the hook queue doesn't have any persistence at all
[16:45] <rog> fwereade: s/have/need/
[16:45] <fwereade> rog, so we just run loads more hooks?
[16:46] <rog> fwereade: no, the hook queue does event collapsing
[16:46] <rog> fwereade: but i don't think it needs to save its state
[16:46] <fwereade> rog, so it collapses an event, writes an intention, and then when that intention is no longer valid it...
[16:47] <fwereade> rog, or it doesn't write the intention until it's actually about to run the hook?
[16:47] <rog> fwereade: the way i'd thought about it is that the hook queue is running in a separate goroutine. it receives events, collapses them into a queue, and tries to send hooks down a channel to the hook executor
[16:48] <rog> fwereade: the hook executor is the thing that writes the intentions, when it's read the hooks from the hook queue
[16:48] <fwereade> rog, so the hook queue needs to read the intentions as well to know how to collapse?
[16:49] <fwereade> rog, how does it cancel a "joined" intention for example?
[16:49] <fwereade> rog, I think the second bit is the more interesting question, sorryt
[16:49] <rog> fwereade: the hook queue starts with our current knowledge of the state of the world
[16:50] <fwereade> rog, sorry, side question: how many not-done intentions will be in the log file at a time?
[16:50] <rog> fwereade: we've found that out by reading the log file at startup
[16:50] <rog> fwereade: one or two
[16:51] <rog> fwereade: any sequence of actions that corresponds to a single state change (i.e. version change)
[16:51] <rog> fwereade: so the first two intentions would be "install" and "start", presumably
[16:51] <fwereade> rog, ok, and this intention-storing is per-relation?
[16:51] <fwereade> who ok no
[16:51] <fwereade> whoa^^
[16:53] <fwereade> rog, ok, we have a constant stream of hooks coming from a bunch of places in separate streams, some of which need to be paused for arbitrary chunks of time
[16:53] <rog> fwereade: the intention storing is per hook execution - we're storing the "intention" to execute a hook, based on some database change
[16:53] <fwereade> rog, the relationship between database changes and hook executions is not simple
[16:53] <fwereade> rog, your hook executor will need ot know basicaly everything about the state of the system, won't it?
[16:54] <rog> fwereade: ah, well that may well be a killer for this approach :-)
[16:54] <rog> fwereade: i was kinda assuming that a single database change triggers a single hook execution
[16:54] <fwereade> rog, 0-2 in general, I think, and the changes come from lots of different places
[16:55] <rog> fwereade: ok... let's enumerate
[16:55] <rog> fwereade: 0?
[16:55] <fwereade> rog, and 0s may look like 1s without reference to past state, and 1s may look like 0s without reference to future state
[16:55] <fwereade> rog, change on change
[16:56] <fwereade> rog, the first change shouldn't actually happen
[16:56] <rog> fwereade: ah, i don't care about the number of hook executions a db change triggers
[16:56] <rog> fwereade: but...
[16:56] <fwereade> rog, what hooks needs to be executed depend on future state
[16:56] <rog> fwereade: is there ever a time when a hook execution depends on *more than one* db change?
[16:57] <fwereade> rog, in terms of deciding whether or not to run it, yes
[16:57] <rog> fwereade: how do you mean?
[16:57] <fwereade> rog, we can have a joined queued and then decide not to bother because we got a departed
[16:58] <rog> fwereade: ok, i see that. but i don't *think* that matters.
[16:58] <fwereade> rog, the fact that the system state changed does not automatically imply that a hook should be run, but we don't know the truth of that until later
[16:59] <rog> fwereade: the idea is that everything in the hook queue is "fuzzy might wanna do it stuff". but when you get something *out* of the queue, you are really going to try and execute it regardless
[16:59] <rog> fwereade: at some point, we have to commit to an intention
[16:59] <rog> fwereade: which is what the intentions log is supposed to mirror
[17:00] <fwereade> rog, hmm, ok, I might be able to see how it could work if we had one per relation
[17:01] <rog> fwereade: why one per relation?
[17:01] <fwereade> rog, what about the hooks we run and need to store the context of for an arbitrarily long time until we run them again?
[17:01] <fwereade> rog, while running other hooks in the interim?
[17:01] <rog> fwereade: how do you mean?
[17:01] <fwereade> rog, resolved
[17:02] <rog> fwereade: how does resolved work?
[17:02] <fwereade> rog, hook runs and errors; we pause the hook queue for that relation until the user resolves it, and may or may not want to retry the hook
[17:02] <rog> fwereade: do any extra hooks get run as a result of this?
[17:03] <fwereade> rog, you have a stored intention for the changed, after the joined that just failed
[17:03] <fwereade> rog, you cannot run that hook until the joined is resolved one way or another
[17:03] <rog> fwereade: how does the user decide whether or not to retry the hook?
[17:03] <fwereade> rog, --retry-hook or something
[17:04] <rog> fwereade: well, you don't mark an intention as done until it's completed without error.
[17:04] <fwereade> rog, ok, so intentions execute out of order?
[17:05] <fwereade> rog, or, alternatively, you have an intention log per relation, and one for the whole unit
[17:05] <rog> fwereade: hmm. i see, i thought a hook execution error paused execution of *all* hooks
[17:05] <fwereade> rog, just because one relation is screwed doesn't mean we should take *everything* down
[17:06] <rog> fwereade: hmm, seems slightly odd to me - the screwed-uppedness could be to do with the central working of the charm
[17:06] <rog> fwereade: but anyway, that's how it is
[17:06] <fwereade> rog, in that case, all the relations fall over, but they do so independently :)
 fwereade, niemeyer: i'm thinking of a signature something like this for juju.Conn.Deploy: http://paste.ubuntu.com/1098537/
[17:06] <niemeyer> rog: That's mixing concerns
[17:06] <niemeyer> rog: Deploy is about, well, deploying
[17:06] <rog> niemeyer: ok, that's why i was askin'!
[17:06] <fwereade> rog, I have to go I'm afrad
[17:07] <niemeyer> rog: And I'm answering!
[17:07] <rog> fwereade: me too. talk tomorrow. i'll keep thinking about it.
[17:07] <niemeyer> !? :-)
[17:07] <rog> niemeyer: thanks!
[17:07] <rog> niemeyer: what concerns is it mixing?
[17:07] <niemeyer> rog: Deploy is about deploying.. it shouldnt' be resolving URLs IMO
[17:07] <rog> niemeyer: fwereade made a good point earlier
[17:08] <rog> niemeyer: i don't think we can resolve the url without a state
[17:08] <rog> niemeyer: i guess we could add juju.Conn.InferURL
[17:09] <niemeyer> rog: Yep.. that's going to be an issue with other methods too
[17:09] <fwereade> rog, hmm, really gtgt, but: 3 operations really -- PublishCharm, AddService, AddUnits
[17:10] <rog> fwereade: yes, that sounds reasonable.
[17:11] <rog> fwereade: i'm kinda trying to avoid mirroring all the state types in juju. perhaps juju should be a more transparent layering.
[17:12] <niemeyer> rog: I'm not sure if InferURL is even the best interface in that specific case.. let me paste you the original point in the original CL
[17:12] <rog> fwereade: func Deploy(*state.State, *charm.URL) *state.Service
[17:12] <niemeyer> rog: http://pastebin.ubuntu.com/1098633/
[17:13] <rog> niemeyer: how do we get the charm URL without getting the default series from the state?
[17:14] <niemeyer> rog: I don't understand why you'd want to do that
[17:14] <niemeyer> rog: What's the problem with getting default-series from the state?
[17:14] <rog> niemeyer: it's an argument to InferURL
[17:14] <niemeyer> rog: That's where it lives..
[17:14] <niemeyer> rog: Yes.. let's use it.. ? :)
[17:15] <rog> niemeyer: right, so juju.Conn should expose its State field
[17:15] <rog> niemeyer: which i actually think is a good idea
[17:15] <niemeyer> rog: Why?
[17:15] <rog> niemeyer: otherwise we'll have to open another state just to get the default series, no?
[17:15] <niemeyer> rog: Not that I think it is a big problem.. it's already exposed, but I don't really get why exposing it or not is relevant in this case
[17:16] <niemeyer> rog: I'm pretty lost
[17:16] <niemeyer> rog: Conn has a State.. PutCharm is in Conn.. Deploy is in Conn
[17:16] <rog> niemeyer: PutCharm takes a charm.URL as an argument
[17:16] <niemeyer> rog: Ah, sure.. we can have InferURL in Conn too.. is that what you're concerned with?
[17:17] <rog> niemeyer: the canonical way of getting a charm.URL is by calling charm.InferURL
[17:17] <rog> niemeyer: yes!
[17:17] <niemeyer> rog: Phew.. ok..
[17:17] <rog> [18:08:26] <rog> niemeyer: i guess we could add juju.Conn.InferURL
[17:17] <niemeyer> rog: Either way sounds fine.. Conn.State exists today.. Conn.InferURL sounds fine too
[17:17] <rog> niemeyer: to be honest, i prefer the idea of the juju package as a relatively transparent layer atop Environ and State
[17:18] <niemeyer> rog: i'd prefer to not change what the juju package is today
[17:18] <rog> niemeyer: i don't intend to do that
[17:18] <niemeyer> rog: Conn looks great as it is
[17:18] <niemeyer> rog: It's working, implemented, and tested
[17:18] <niemeyer> rog: Let's solve the problem we have at hand and move on, IMO
[17:18] <rog> niemeyer: just that it means that we don't necessarily have to provide a totally complete layer on top of state
[17:19] <niemeyer> rog: We're not doing that
[17:19] <rog> niemeyer: so we can allow clients to, for example, get the default series from the state
[17:19] <niemeyer> rog: Look at the state interface, and look at the Conn interface
[17:19] <niemeyer> rog: They are pretty different, and seem to be going exactly in the direction we designed them to be
[17:19] <rog> niemeyer: yeah, but what i'm worried about is having types in juju mirroring the ones in state, e.g. juju.Service, juju.Relation etc
[17:20] <niemeyer> rog: We don't have that today, and I don't think we should have that.. why would we?
[17:20] <rog> niemeyer: i'm wondering how juju.Conn.SetRelation would work, for example
[17:20] <rog> niemeyer: how do we phrase the arguments to it?
[17:20] <niemeyer> rog: SetRelation?  What's that?
[17:21] <rog> niemeyer: sorry, RelationSet
[17:21] <niemeyer> rog: What's that?
[17:21] <rog> niemeyer: erk
[17:21] <niemeyer> rog: AddRelation?
[17:21] <rog> niemeyer: ok, Set
[17:22] <niemeyer> rog: Sorry, I'm lost.. those names don't mean anything to me
[17:22] <rog> sorry
[17:22] <rog> niemeyer: the juju set command
[17:22] <rog> niemeyer: equivalent in juju pkg would probably be juju.Conn.Set
[17:23] <niemeyer> rog: Okay, thanks
[17:23] <rog> hmm, i guess we can just use the svc name
[17:23] <niemeyer> rog: Yeah, or a *state.Service
[17:24] <rog> niemeyer: i was wondering if we'd return a *state.Service from juju.Conn.Deploy; then juju.Conn.Set could take a service
[17:24] <rog> niemeyer: yeah
[17:24] <rog> niemeyer: that's the kind of thing i mean by a "more transparent layer" - we're not entirely hiding the fact that state is under the surface
[17:25] <niemeyer> rog: I never thought of Conn like that
[17:25] <rog> niemeyer: cool
[17:25] <rog> niemeyer: then i'm happy
[17:25] <niemeyer> rog: We have State in there, after all
[17:25] <rog> niemeyer: yeah... i guess i'd thought of that mainly for testing purposes
[17:25] <niemeyer> rog: It's an API that allows us to poke into both the environment and the state together.. a facade if you will
[17:26] <rog> niemeyer: yeah, sounds good
[17:26] <rog> niemeyer: a collection of higher level operations
[17:26] <niemeyer> Yeah
[17:27] <rog> niemeyer: ok, so Deploy(*charm.URL) it is. that's lovely.
[17:27] <rog> niemeyer: and it's funny we both came up with *almost* exactly the same sig for Deploy. (i'd totally forgotten about your remark in that CL!)
[17:29] <niemeyer> rog: I think the suggested interface solves the issue better, but you'll probably figure as you implement it
[17:30] <rog> niemeyer: what do you see as the main differences, besides the URL resolving stuff?
[17:30] <niemeyer> rog: Deploying a charm may be done many times, with charms that are already in the state
[17:31] <niemeyer> rog: That does not involve any kind of URL resolution or inferring
[17:31] <niemeyer> rog: Putting a charm in the state can also be done by itself, independently
[17:31] <rog> niemeyer: yeah, i'm with you. i was wondering about other differences.
[17:32] <rog> niemeyer: (hence "besides" the URL resolving stuff)
[17:32] <rog> niemeyer: i've gotta go, but i'll see what you write, later.
[17:32] <rog> niemeyer: have a great rest-of-day and evening
[17:32] <niemeyer> rog: That's really it.. it seems to make more sense to deploy a *state.Charm than a *charm.URL, in Conn
[17:33] <niemeyer> rog: Cheers
[17:33] <rog> niemeyer: ok cool
[17:33] <rog> niemeyer: guess i'll see you in lisbon next!
[17:33] <niemeyer> rog: For you too
[17:33] <rog> niemeyer: hope your interminable travel time isn't too bad
[17:33] <niemeyer> rog: Generally not an issue.. there's so much to do with quiet time :)
[17:34] <niemeyer> rog: Thanks, though
[17:34] <niemeyer> rog: Have a good time yourself
[18:59] <niemeyer> "Are you uncertain, or hoping to convince me through Socratic dialogue?"
[18:59] <niemeyer> fwereade_: LOL
[19:43] <niemeyer> Giving a ride to Ale.. back in 15
[20:39] <niemeyer> fwereade_: ping
[20:51] <hazmat> yes
[21:05] <niemeyer> hazmat: I like your thinking
[21:08] <hazmat> econtext wrong channel
[21:09] <niemeyer> hazmat: The positiveness is welcome no matter what :-)
[21:09] <hazmat> awesome
[22:22] <davecheney> niemeyer: thanks for the reviews
[22:23] <davecheney> at some point today the workmen in my house will need to turn off the power
[22:23] <davecheney> so i'll go into the city and work from there
[22:56] <niemeyer> davecheney: np, and thanks for the note
[22:57] <fwereade_> niemeyer, pong
[22:57] <niemeyer> fwereade_: Heya
[22:57] <fwereade_> niemeyer, heyhey
[22:57] <niemeyer> I think you've got it all in your inbox already :)
[22:58] <fwereade_> niemeyer, cool :)
[22:59] <fwereade_> niemeyer, I see nothing on watch-presence-children?
[23:00] <fwereade_> niemeyer, thank you for the others though, and I think I like the RelationHandler suggestion
[23:01] <fwereade_> niemeyer, need a little time to absorb it though :)
[23:01] <fwereade_> niemeyer, I should probably do it while I sleep, though :)
[23:01] <fwereade_> niemeyer, nn
[23:03] <niemeyer> fwereade_: Sounds like a good plan! :-)
[23:03] <niemeyer> fwereade_: Have a good night
[23:03] <fwereade_> niemeyer, and you :)
[23:07] <fwereade_> niemeyer, hm, I have just had a thought about RelationHandler
[23:08] <fwereade_> niemeyer, no, it's not well-formed
[23:08] <fwereade_> niemeyer, really going now ;)
[23:08] <niemeyer> fwereade_: Cheers! ;)