| davecheney | ubuntu@ip-10-4-114-16:~$ pgrep jujud -lf | 00:55 |
|---|---|---|
| davecheney | 5074 /var/lib/juju/tools/juju-0.0.0-precise-amd64/jujud provisioning --zookeeper-servers localhost:2181 --log-file /var/log/juju/provision-agent.log --debug | 00:55 |
| davecheney | 5077 /var/lib/juju/tools/juju-0.0.0-precise-amd64/jujud machine --zookeeper-servers localhost:2181 --machine-id 0 --log-file /var/log/juju/machine-agent.log --debug | 00:55 |
| rogpeppe | davecheney: mornin' | 05:55 |
| davecheney | rogpeppe: hows' tricks ? | 05:55 |
| rogpeppe | davecheney: good thanks. finally got around to painting the kitchen last weekend... two years after we did the rest of it! | 05:56 |
| rogpeppe | davecheney: still needs another coat, but getting there | 05:56 |
| davecheney | rogpeppe: yup, you'd think as an adult painting would be simple | 05:56 |
| davecheney | turns out it's really bloody hard | 05:56 |
| rogpeppe | davecheney: i've had an idea about the watchers which i'd like to run past you | 05:57 |
| davecheney | btw, I think ec2/config_test.go might be broken | 05:57 |
| davecheney | as in, doesn't test anything | 05:57 |
| rogpeppe | davecheney: hmm, how so? | 05:57 |
| davecheney | trying to write a _failing_ test case for https://bugs.launchpad.net/juju-core/+bug/1025128 | 05:57 |
| davecheney | { | 05:58 |
| davecheney | "admin-secret: 81a1e7429e6847c4941fda7591246594\n" + baseConfig, | 05:58 |
| davecheney | func(cfg *providerConfig) {}, | 05:58 |
| davecheney | this should fail right ? | 05:58 |
| davecheney | "", | 05:58 |
| davecheney | }, | 05:58 |
| davecheney | rogpeppe: did you read my mid afternoon status, we have a machine agent running on machine/0 now | 05:59 |
| * rogpeppe has a look | 06:00 | |
| rogpeppe | davecheney: oh, no i didn't - i've got a branch almost done which runs the MA. that's cool though, it's not much code. | 06:00 |
| davecheney | rogpeppe: dagnabbit, we're all over each others toes | 06:01 |
| rogpeppe | davecheney: i know, it's as things start to close in | 06:01 |
| davecheney | i'm kinda hanging in the wind until deploy secrets is comitted | 06:01 |
| rogpeppe | davecheney: that'll be soon, i hope, if one or other of the config branches gets in | 06:02 |
| * davecheney nods | 06:02 | |
| rogpeppe | davecheney: did you have a look at this BTW? https://codereview.appspot.com/6343107/ | 06:02 |
| davecheney | rogpeppe: i had a brief look, SGTM | 06:03 |
| davecheney | but i'm really only intersted in getting secrets deployed | 06:03 |
| rogpeppe | davecheney: it's part and parcel | 06:03 |
| davecheney | config has had _so_ many polishes, it must ground down to a nub now | 06:03 |
| rogpeppe | davecheney: it's surprisingly subtle | 06:04 |
| davecheney | rogpeppe: that suggests that its serving many masters | 06:04 |
| rogpeppe | davecheney: i found that when trying to hack on william's code | 06:04 |
| davecheney | subtle the way state.Info is subtle | 06:04 |
| rogpeppe | davecheney: it was trying to be two things at once | 06:04 |
| rogpeppe | davecheney: i don't thing state.Info is subtle | 06:05 |
| rogpeppe | think | 06:05 |
| rogpeppe | davecheney: there was lots of code doing not very much | 06:05 |
| rogpeppe | davecheney: you're right, config tests are fucked | 06:06 |
| davecheney | rogpeppe: should I raise a bug | 06:07 |
| davecheney | i fixed 1025128, but it's hard to test when the test harness is rooted | 06:07 |
| rogpeppe | davecheney: i just changed one of the existing error messages and the test still passed... | 06:07 |
| davecheney | rogpeppe: indeed o_O | 06:08 |
| rogpeppe | davecheney: go test -gocheck.f TestConfig | 06:08 |
| rogpeppe | davecheney: runs no tests | 06:08 |
| rogpeppe | oops | 06:08 |
| davecheney | bwahaha | 06:08 |
| davecheney | hwo did that happen ? | 06:08 |
| rogpeppe | davecheney: ha! | 06:08 |
| rogpeppe | davecheney: i see | 06:08 |
| rogpeppe | davecheney: someone at some point (maybe me!) changed (configSuite) to (s *configSuite) | 06:09 |
| davecheney | bzr blame ? | 06:09 |
| rogpeppe | davecheney: i dare not | 06:09 |
| davecheney | care to unfuk ? | 06:10 |
| rogpeppe | davecheney: gimme 30s | 06:10 |
| rogpeppe | davecheney: ha! not so fast - loads of tests fail... | 06:11 |
| rogpeppe | shit | 06:12 |
| davecheney | rogpeppe: maybe log a bug | 06:13 |
| davecheney | i'll do it | 06:13 |
| rogpeppe | davecheney: looks like william might've done it... | 06:13 |
| rogpeppe | fwereade_: ahem | 06:13 |
| rogpeppe | so easy to do | 06:13 |
| davecheney | rogpeppe: sohuld thre be two bugs then ? on against gocheck to avoid this kind of thing ? | 06:13 |
| rogpeppe | davecheney: arguably. it should probably give an error if it finds tests on the value type when it's been given a pointer | 06:14 |
| davecheney | rogpeppe: i think this should be raised as a bug | 06:15 |
| rogpeppe | davecheney: yeah | 06:15 |
| davecheney | rogpeppe: i'll do it | 06:15 |
| rogpeppe | davecheney: can i run my watcher idea past you? | 06:16 |
| davecheney | rogpeppe: https://bugs.launchpad.net/juju-core/+bug/1025138 | 06:16 |
| davecheney | ^ can you put some debug in this issue | 06:16 |
| davecheney | then i'll log one against gocheck to make sure we can't screw ourselves again | 06:16 |
| davecheney | rogpeppe: sure, tell me about your watcher | 06:16 |
| rogpeppe | davecheney: i'm concerned by how hard it is to write multiplexers. this was some code i suggested to frank for the firewall code: http://paste.ubuntu.com/1089462/ | 06:18 |
| rogpeppe | davecheney: the code inside loop() there should really only be three lines of code. | 06:18 |
| rogpeppe | davecheney: and i think the problem is to do with the way we stop watchers | 06:19 |
| davecheney | rogpeppe: one idea i had was to pass into the watcher a channel to receive changes | 06:19 |
| rogpeppe | davecheney: each watcher has its own Stop function. | 06:19 |
| davecheney | this is unusual, but might make a mux style watcher simpler | 06:19 |
| rogpeppe | davecheney: i think a nicer alternative is to pass a stop channel into the watcher | 06:19 |
| rogpeppe | davecheney: then the watcher closes its channel when done | 06:20 |
| davecheney | rogpeppe: but you can't select on a closed channel, right ? | 06:20 |
| rogpeppe | davecheney: that way you've got a *single* stop channel for all watchers you're muxing | 06:20 |
| rogpeppe | davecheney: 'course you can | 06:20 |
| rogpeppe | davecheney: (select on read) | 06:20 |
| * davecheney remains unsure | 06:20 | |
| rogpeppe | davecheney: and only a single channel to read from each watcher | 06:20 |
| rogpeppe | davecheney: i *think* it could simplify things considerably | 06:21 |
| * davecheney goes to play with play.g.o for a second | 06:21 | |
| davecheney | rogpeppe: indeed, this will work, http://play.golang.org/p/G15bEHpZkP | 06:23 |
| rogpeppe | davecheney: absolutely. it's the way closed chans are meant to be used | 06:23 |
| davecheney | nice | 06:24 |
| rogpeppe | davecheney: having a single stop channel means that a muxer can "delegate" its stop request to those things it's watching | 06:24 |
| davecheney | rogpeppe: if you pass it in as <- chan, then you know you are the only one who can close it | 06:24 |
| rogpeppe | davecheney: then every watcher would provider a Wait (sp?) method which would read messages from the watcher chan until EOF, then return the watcher's error | 06:24 |
| rogpeppe | davecheney: exactly | 06:24 |
| rogpeppe | davecheney: i *think* this means most watchers can dispense with tomb | 06:25 |
| davecheney | rogpeppe: certainly for those watchers where we don't need any sort of error value from, yes | 06:25 |
| rogpeppe | davecheney: no, errors are easy too | 06:25 |
| rogpeppe | davecheney: most watchers are single-goroutine | 06:26 |
| rogpeppe | davecheney: it's easy for them to get an error from the watcher they're reading from and put it in an instance variable | 06:26 |
| rogpeppe | davecheney: the only way for a client to get the error is to call Wait. | 06:26 |
| rogpeppe | davecheney: and when that returns, we're guaranteed that the error is in a known state | 06:27 |
| rogpeppe | davecheney: errors are crucial, because all watchers need to be able to return an error | 06:30 |
| * davecheney nods | 06:30 | |
| rogpeppe | shit, i've just buggered cobzr | 06:32 |
| davecheney | rm -rf ../../.bzr/cobzr ? | 06:33 |
| rogpeppe | davecheney: nope, edit .bzr/branch/location | 06:34 |
| rogpeppe | davecheney: that's a really bad suggestion BTW! | 06:34 |
| rogpeppe | davecheney: it would erase all my branches | 06:34 |
| davecheney | rogpeppe: I did the above by mistake once, once | 06:34 |
| rogpeppe | davecheney: oops | 06:35 |
| rogpeppe | davecheney: in this case it wasn't so bad. i did branch -m ec2-fix-configsuite' | 06:35 |
| rogpeppe | ' | 06:35 |
| rogpeppe | (note the linefeed) | 06:35 |
| rogpeppe | davecheney: it doesn't like it! | 06:35 |
| rogpeppe | davecheney: at least, it "works" | 06:35 |
| rogpeppe | davecheney: then you can't do anything | 06:36 |
| rogpeppe | davecheney: i'll raise a bug! that's what i'll do... | 06:36 |
| rogpeppe | davecheney: ha. can't do that. | 06:37 |
| rogpeppe | davecheney: i did one watcher: https://codereview.appspot.com/6373048 | 06:57 |
| rogpeppe | davecheney: the important bit is the change on lines 162-167 | 06:59 |
| rogpeppe | davecheney: we don't have to select on output any more | 06:59 |
| rogpeppe | davecheney: which makes muxers much easier (and we will have lots of them in the firewall and unit agent) | 06:59 |
| davecheney | rogpeppe: holy shit, a closed channel always fires !?? | 07:22 |
| rogpeppe | davecheney: absolutely! | 07:22 |
| rogpeppe | davecheney: have you not read the spec? | 07:22 |
| rogpeppe | davecheney: they're *really* useful | 07:22 |
| davecheney | rogpeppe: not for a while | 07:22 |
| rogpeppe | davecheney: it's the only way to use a chan to broadcast | 07:23 |
| rogpeppe | davecheney: it's always been like that. except in the early days you'd get a panic if you read more than N times N =~ 20. | 07:24 |
| davecheney | rogpeppe: i can't find a reference to it in the spec | 07:24 |
| davecheney | but a closed channel looks like it always behaves like it has a value of the zero value and false | 07:25 |
| rogpeppe | davecheney: http://golang.org/ref/spec#Close | 07:25 |
| davecheney | I guess you're talking about this bit "zero value for the channel's type without blocking" | 07:25 |
| davecheney | it doesn't specifically mention it's use in select {} but whatever | 07:26 |
| davecheney | as you say, a very useful property | 07:26 |
| rogpeppe | davecheney: yeah | 07:26 |
| rogpeppe | to both | 07:26 |
| davecheney | the fact it's level triggered is very useful | 07:27 |
| davecheney | although, it's not really acting as a default case | 07:27 |
| rogpeppe | davecheney: thanks to fwereade_'s refactoring, the state/wacher.go changes are pretty minimal, it seems | 07:27 |
| rogpeppe | davecheney: what do you mean by that? | 07:27 |
| davecheney | that is, receiving from a closed channel will be chosen pseudorandomly from the set of ready channels | 07:28 |
| rogpeppe | davecheney: yeah | 07:28 |
| rogpeppe | davecheney: which is fine, i think. | 07:28 |
| davecheney | yup | 07:28 |
| rogpeppe | davecheney: you might have two closed channels, of course | 07:28 |
| davecheney | combined with nil'ing the channel, it's very powerful | 07:28 |
| rogpeppe | davecheney: definitely | 07:28 |
| davecheney | anyway, time for a break, and some dinner | 07:28 |
| rogpeppe | davecheney: both of those things weren't in Limbo, and i really appreciate them | 07:29 |
| rogpeppe | davecheney: enjoy! | 07:29 |
| rogpeppe | davecheney: 't'was good to catch you | 07:29 |
| davecheney | later lads, if I don't see you again tonight, we'll talk at le standup! | 07:29 |
| rogpeppe | davecheney: aye | 07:29 |
| TheMue | morning | 08:04 |
| fwereade_ | TheMue, heyhey | 08:09 |
| fwereade_ | rogpeppe, also heyhey :) | 08:09 |
| rogpeppe | fwereade_: yo! | 08:09 |
| rogpeppe | fwereade_: i've had a minor revelation about the watchers | 08:10 |
| rogpeppe | TheMue: yay! | 08:10 |
| fwereade_ | rogpeppe, oh yes? | 08:10 |
| rogpeppe | fwereade_: i was thinking about this code, which i suggested to TheMue as part of the firewall code: http://paste.ubuntu.com/1089462/ | 08:10 |
| fwereade_ | rogpeppe, is this to do with the which-ready-channel-you-receive-from-is-random thing? | 08:10 |
| fwereade_ | rogpeppe, that has had me a little suspicious lately, easy to miss | 08:11 |
| rogpeppe | fwereade_: that was a side shoot | 08:11 |
| rogpeppe | fwereade_: or... maybe i don't know what you're referring to there | 08:11 |
| rogpeppe | fwereade_: i thought that the code above was way more complex than it should be | 08:11 |
| fwereade_ | rogpeppe, I shouldn't worry about it for now, I'm just developing a sl. twitchy feeling that we may not have fully analysed some of the watchers, I'll figure it out properly myself :) | 08:12 |
| rogpeppe | fwereade_: this potential change makes them easier to analyse, i think | 08:12 |
| rogpeppe | fwereade_: the idea is that actually most watchers layer on top of other watchers | 08:12 |
| fwereade_ | rogpeppe, anyway, sorry derail | 08:12 |
| fwereade_ | rogpeppe, yes, this is very true | 08:13 |
| * TheMue listens | 08:13 | |
| rogpeppe | fwereade_: and so the fact that each watcher has its own Stop method makes things hard | 08:13 |
| fwereade_ | rogpeppe, ok... | 08:13 |
| rogpeppe | fwereade_: we can do better, i think, if we *pass in* a stop channel | 08:13 |
| rogpeppe | fwereade_: and use the EOF status of the watch channel as an indication that the watcher has completed | 08:14 |
| TheMue | rogpeppe: Hmm, an idea: why don't we build the watchers so that, when creating an instance, we don't pass behaviors based on interfaces. They are called on changes and errors. | 08:14 |
| fwereade_ | rogpeppe, hmm, this has crossed my mind in individual cases | 08:14 |
| TheMue | rogpeppe: Everything else is handled internally. | 08:14 |
| rogpeppe | TheMue: i'm not sure i understand that | 08:14 |
| TheMue | rogpeppe: Wait a moment, I'll write a paste as outline. | 08:15 |
| rogpeppe | fwereade_: here's an example of what i mean: https://codereview.appspot.com/6373048/diff/1/state/watcher/watcher.go | 08:15 |
| fwereade_ | rogpeppe, the trouble is that pass-a-stopper is a nice thing internally, but as a consumer of our APIs I would much rather get an object I can Stop() myself | 08:15 |
| rogpeppe | fwereade_: yes, that's the down side | 08:15 |
| rogpeppe | fwereade_: *but* | 08:15 |
| rogpeppe | fwereade_: it makes everything else simpler | 08:15 |
| rogpeppe | fwereade_: particularly in the muxing case, which we're going to be doing a *lot* in the firewall and unit agent code | 08:17 |
| rogpeppe | fwereade_: that code i pasted earlier turns into something like this: http://paste.ubuntu.com/1094518/ | 08:18 |
| rogpeppe | fwereade_: i.e. exactly what it *should* look like | 08:18 |
| fwereade_ | rogpeppe, hmm, yes, I agree it's useful in those cases | 08:19 |
| * fwereade_ is fretting that it's not necessarily everything we need | 08:19 | |
| rogpeppe | fwereade_: how do you mean? | 08:19 |
| fwereade_ | rogpeppe, eg in one of the things I have up for review | 08:19 |
| fwereade_ | rogpeppe, relationUnitsWatcher I think | 08:19 |
| fwereade_ | rogpeppe, I found that stop chans weren't quite enough for my single-thing-watcher goroutines | 08:20 |
| fwereade_ | rogpeppe, and I convinced myself that I needed a whole separate tomb for every goroutine | 08:20 |
| rogpeppe | fwereade_: i don't *think* you do | 08:21 |
| fwereade_ | rogpeppe, maybe I should have been keeping references to the subwatchers around directly, that would probably be enough | 08:21 |
| fwereade_ | rogpeppe, the specific issue was "make sure this goroutine is not going to send anything else on the updates channel" | 08:22 |
| rogpeppe | fwereade_: ah! | 08:22 |
| fwereade_ | rogpeppe, ie, make sure I can't get a changed event after a deleted event | 08:22 |
| rogpeppe | fwereade_: but that's the important thing about this change! | 08:22 |
| rogpeppe | fwereade_: we don't care any more! | 08:22 |
| TheMue | rogpeppe: http://paste.ubuntu.com/1094521/ is very, very quick. The important idea should be that you don't bother with different channels and tombs. The watcher does this and calls the methods of the passed behavior. | 08:22 |
| rogpeppe | fwereade_: because the way we wait for a watcher to complete is by reading all events from the channel. | 08:22 |
| TheMue | rogpeppe: It's used in Erlang/OTP very much. Services provide the backend, behaviors concentrate on business logic. | 08:23 |
| rogpeppe | fwereade_: have a look at that watcher code i posted above | 08:23 |
| fwereade_ | rogpeppe, I do like that, but I'm still not sure it helps my case | 08:23 |
| fwereade_ | rogpeppe, this is me waiting until I'm sure that one specific goroutine is no longer going to be sending on a shared channel | 08:24 |
| rogpeppe | TheMue: i'm not sure i understand the motivation, or the context. | 08:24 |
| rogpeppe | fwereade_: why do you need to do that? | 08:24 |
| rogpeppe | TheMue: is this a suggestion for state/watcher.go ? | 08:24 |
| fwereade_ | rogpeppe, to ensure that I will not get any embarrassing changed events after I've sent a departed event for a particular relation units | 08:25 |
| rogpeppe | TheMue: it looks quite similar to what fwereade_'s done in that file already | 08:25 |
| rogpeppe | fwereade_: because you've got two watchers watching different aspects of the same thing? | 08:26 |
| TheMue | rogpeppe: Inside one watcher, yes. But take a look at your paste above. We still build new watchers upon, with own tombs, with multiply channels and all that stuff. | 08:26 |
| fwereade_ | rogpeppe, assume that I got a change immediately followed by a delete, and my main goroutine is processing the delete while the child goroutine is processing the change | 08:27 |
| TheMue | rogpeppe: My intention is to simplify that. | 08:27 |
| rogpeppe | TheMue: i'd have to see a more complete example to understand what you're suggesting. | 08:27 |
| fwereade_ | rogpeppe, unless I wait for the child to actually die, I can't be sure which of the channels it's selecting on will be selected | 08:28 |
| rogpeppe | fwereade_: isn't the problem there that you've got two things operating on the same state? | 08:28 |
| fwereade_ | rogpeppe, ie when the scheduler gets around to it next, Dying is closed but updates is also unblocked | 08:28 |
| rogpeppe | fwereade_: which channels is it selecting on? | 08:28 |
| fwereade_ | rogpeppe, I have two different things operating on two different pieces of state that are in the same system | 08:29 |
| fwereade_ | rogpeppe, waiting to receive from a stop chan (which might be a tomb.Dying()), or send on an updates chan for the attention of the main goroutine | 08:29 |
| rogpeppe | fwereade_: with this change, you would not do that | 08:30 |
| rogpeppe | fwereade_: you'd just send on the updates chan unconditionally | 08:30 |
| fwereade_ | rogpeppe, right, so how do I avoid getting change events on the updates chan as soon as I want to stop receiving them? | 08:31 |
| rogpeppe | fwereade_: you don't. | 08:31 |
| rogpeppe | fwereade_: but i don't see why that's a problem. | 08:31 |
| fwereade_ | rogpeppe, at the time I stop, the child goroutine may already be waiting to send | 08:31 |
| rogpeppe | fwereade_: that's fine. | 08:31 |
| fwereade_ | rogpeppe, I want to be damn sure it will not do so | 08:31 |
| rogpeppe | fwereade_: at the receiving side, you have to ignore the subsequent event(s) | 08:32 |
| fwereade_ | rogpeppe, because a relationUnitsWatcher that sends crap like unit-0-0 deleted followed by unit-0-0 changed is just crackful | 08:32 |
| rogpeppe | fwereade_: but that shouldn't be hard | 08:32 |
| fwereade_ | rogpeppe, when N goroutines are sending on the shared update channel? | 08:32 |
| fwereade_ | rogpeppe, I keep more state lying around to know which events I should filter out f the updates stream? | 08:33 |
| rogpeppe | fwereade_: yeah, it's not a difficult problem, i think. you maintain some state per goroutine on the receiver side. | 08:33 |
| rogpeppe | fwereade_: each even coming from the shared update channel will identify its goroutine | 08:33 |
| fwereade_ | rogpeppe, hmm, doesn't really feel like a win to me, but I could perhaps be convinced | 08:33 |
| rogpeppe | fwereade_: so the state can be there trivially | 08:33 |
| rogpeppe | s/each even/each event/ | 08:34 |
| rogpeppe | fwereade_: i *think* it's a much easier structure to reason about | 08:34 |
| fwereade_ | rogpeppe, it's just a feeling that the watchers which keep state around are harder to reason about | 08:35 |
| fwereade_ | rogpeppe, it could well still be a net win; makes most things easier and some harder | 08:35 |
| rogpeppe | fwereade_: at least this state is simple local state, no concurrent interactions with it | 08:36 |
| rogpeppe | fwereade_: in this case, i think it's as simple as "x.ignore = true"; [..] if x.ignore { continue} | 08:36 |
| rogpeppe | fwereade_: but i may well misunderstand your problem | 08:37 |
| fwereade_ | rogpeppe, well, it's multiple goroutines, so I'd need to invert it and store the ones I *do* care about in a map | 08:37 |
| fwereade_ | rogpeppe, I agree not very complex | 08:37 |
| fwereade_ | rogpeppe, I'm not arguing it's an insupportable burden | 08:38 |
| rogpeppe | fwereade_: i don't think so... each goroutine stores info about itself | 08:38 |
| rogpeppe | fwereade_: and sends that info on the watch channel. | 08:38 |
| rogpeppe | fwereade_: so when an event arrives, you get the info too, which you can manipulate as local state. | 08:38 |
| fwereade_ | rogpeppe, but the critical info always comes in on the main goroutine, not the child, I don't think the child should be storing that state | 08:39 |
| fwereade_ | rogpeppe, I am not opposed to exploring this idea further | 08:39 |
| rogpeppe | fwereade_: cool | 08:39 |
| Aram | moin everyone. | 08:39 |
| TheMue | Hi Aram | 08:39 |
| rogpeppe | Aram: yo | 08:39 |
| rogpeppe | ! | 08:39 |
| fwereade_ | rogpeppe, just pointing out a use case which I think is legitimate and not the best fit | 08:39 |
| fwereade_ | Aram, heyhey | 08:39 |
| rogpeppe | fwereade_: i'll be interested to have a look at the CL | 08:39 |
| fwereade_ | rogpeppe, I have a stack of them, bit insomniac this w/e | 08:40 |
| rogpeppe | fwereade_: i had this idea sitting in my head all weekend, but couldn't get anywhere near a computer... | 08:40 |
| fwereade_ | rogpeppe, TheMue: in fact I think https://codereview.appspot.com/6408045/ is basically a trivial | 08:40 |
| fwereade_ | rogpeppe, TheMue: but one with a surprisingly significant effect | 08:40 |
| fwereade_ | rogpeppe, TheMue: so I would appreciate both your opinions | 08:41 |
| fwereade_ | rogpeppe, TheMue: it's independent of https://codereview.appspot.com/6405044/ which is a prereq for https://codereview.appspot.com/6402048/ | 08:41 |
| fwereade_ | rogpeppe, the first one of those 2 is the use case I was talking about | 08:42 |
| fwereade_ | rogpeppe, the second one ties together a bunch of stuff in a way that I find pleasing | 08:42 |
| fwereade_ | rogpeppe, and gets us a RelationUnit type that is aware of the existence/settings of other RelationUnits in other agents | 08:43 |
| Aram | fwereade_: rogpeppe: TheMue: I have to take at least half a day off... I've stayed too much in the sun and I have a fever and an excruciating headache. | 08:43 |
| fwereade_ | Aram, np, look after yourself :) | 08:43 |
| rogpeppe | fwereade_: 6408045 is because yaml doesn't marshal deterministically? | 08:43 |
| fwereade_ | rogpeppe, yeah :/ | 08:43 |
| Aram | woke up and drank 2 liters of water. | 08:43 |
| rogpeppe | Aram: ok, sorry about that | 08:43 |
| rogpeppe | fwereade_: although i'm jealous of your sun | 08:43 |
| rogpeppe | fwereade_: it's been 13 degrees here for weeks | 08:43 |
| fwereade_ | rogpeppe, it was swapping dict field order about 1 time in 10 in normal use | 08:44 |
| rogpeppe | fwereade_: i've got the central heating on | 08:44 |
| rogpeppe | fwereade_: ah, makes sense | 08:44 |
| fwereade_ | rogpeppe, the test case hits it 100 times and it sometimes switches rendering order >50 times | 08:44 |
| rogpeppe | fwereade_: LGTM | 08:44 |
| fwereade_ | rogpeppe, that was a fun midnight bug-hunt though | 08:44 |
| fwereade_ | rogpeppe, cool | 08:44 |
| rogpeppe | fwereade_: good catch! | 08:44 |
| fwereade_ | TheMue, if I could get one from you too I'll merge it straight in | 08:45 |
| fwereade_ | rogpeppe, cheers :) | 08:45 |
| rogpeppe | fwereade_: although... | 08:45 |
| fwereade_ | rogpeppe, go on | 08:45 |
| rogpeppe | fwereade_: why did it cause anything to fail? | 08:45 |
| rogpeppe | fwereade_: i'd've thought the tests should be resilient to config nodes changing-but-not-changing | 08:46 |
| fwereade_ | rogpeppe, AIUI ConfigNode is meant to *not* change-without-changing | 08:46 |
| fwereade_ | rogpeppe, a while ago niemeyer said he would consider that behaviour to be a bug | 08:46 |
| fwereade_ | rogpeppe, and it is an assumption I was depending on in the tests | 08:47 |
| rogpeppe | fwereade_: nonetheless, the watchers *should* be resilient to it, no? | 08:47 |
| rogpeppe | fwereade_: ah | 08:47 |
| fwereade_ | rogpeppe, no, IMO the system should assume this useful property of ConfigNode | 08:47 |
| fwereade_ | rogpeppe, this case specifically | 08:47 |
| rogpeppe | fwereade_: ok | 08:47 |
| fwereade_ | rogpeppe, relation unit writes private-address to its settings node on creation | 08:48 |
| fwereade_ | rogpeppe, the private address *might* have changed if the UA was restarted and is rejoining an existing one | 08:48 |
| rogpeppe | fwereade_: i suppose contentWatcher copes with updates-without-changes at too low a level | 08:48 |
| fwereade_ | rogpeppe, or it might not, or the node might not exist at all | 08:48 |
| fwereade_ | rogpeppe, IMO the nice thing to do is just to always set private-address on join | 08:48 |
| fwereade_ | rogpeppe, and let the lower level filter out the changes-that-don't-chnage | 08:49 |
| fwereade_ | rogpeppe, with the bug, a particular node's version can change under the hod and send me an unepected event | 08:49 |
| rogpeppe | "relation unit writes private-address to its settings node on creation" | 08:50 |
| fwereade_ | rogpeppe, not even an unexpected event, actually | 08:50 |
| rogpeppe | i don't understand that | 08:50 |
| fwereade_ | rogpeppe, just an unexpected version | 08:50 |
| fwereade_ | rogpeppe, ok, every unit participating in a relation has its own settings and presence nodes | 08:50 |
| TheMue | fwereade_: Looks good | 08:50 |
| rogpeppe | fwereade_: what's a "relation unit" and what's the private address we're talking about here? | 08:50 |
| fwereade_ | rogpeppe, existence of the presence node implies validity of the settings | 08:50 |
| rogpeppe | fwereade_: ok | 08:51 |
| fwereade_ | rogpeppe, one thing we guarantee is that if you're in a relation with some other units, you will have access to their private-address setting | 08:51 |
| fwereade_ | rogpeppe, so that you can do whatever magic youpersonally require to get your charm talking to the other side of the relation | 08:51 |
| rogpeppe | fwereade_: ah, i didn't know that | 08:51 |
| fwereade_ | rogpeppe, make sense roughly? | 08:51 |
| rogpeppe | fwereade_: yup | 08:52 |
| fwereade_ | rogpeppe, I'm not saying there isn't a better way to do it, but the python seems to work pretty well like that :) | 08:52 |
| rogpeppe | fwereade_: i guess i'm surprised that the config node watcher doesn't filter out occasions when the content has changed but the attrs haven't | 08:53 |
| rogpeppe | fwereade_: but i'm happy with your fix | 08:53 |
| rogpeppe | fwereade_: it seems very plausible | 08:53 |
| fwereade_ | rogpeppe, feels like a lot more hassle to do it at output time when we already have all the info available at input time, as it were | 08:53 |
| rogpeppe | fwereade_: yeah | 08:53 |
| fwereade_ | rogpeppe, and then we get more predictable behaviour at a lower level and can build higher-level stuff with more confidence | 08:54 |
| rogpeppe | fwereade_: definitely. | 08:54 |
| rogpeppe | fwereade_: thanks for the explanation | 08:54 |
| fwereade_ | rogpeppe, need to take a longish break, cath was away for the weekend and I need to JIT some housekeeping which got sacrificed to coding while laura was asleep ;) | 08:55 |
| rogpeppe | fwereade_: :-) | 08:55 |
| rogpeppe | fwereade_: when you're back i should have a watcher CL for your perusal | 08:55 |
| fwereade_ | rogpeppe, cool | 08:55 |
| fwereade_ | rogpeppe, TheMue: I'll just merge the ConfigNode change before I break | 08:56 |
| rogpeppe | fwereade_: and i'll add some tests to that config proposal. i *hope* that gustavo likes it. | 08:56 |
| fwereade_ | rogpeppe, that's always the worry | 08:56 |
| fwereade_ | rogpeppe, I wish I had a better niemeyer sim | 08:56 |
| rogpeppe | fwereade_: mine's still training the neural net. | 08:57 |
| rogpeppe | fwereade_: it's pretty erratic | 08:57 |
| fwereade_ | rogpeppe, indeed, I find it actually gets worse with time because it functions accurately for weeks and then, suddenly, *total* failure | 08:57 |
| rogpeppe | fwereade_: wildly non-linear solution space | 08:58 |
| fwereade_ | rogpeppe, indeed :) | 08:58 |
| fwereade_ | rogpeppe, TheMue: thanks, submitted | 09:00 |
| TheMue | fwereade_: Cheers | 09:08 |
| TheMue | Oh, revision 300, nice number. | 09:21 |
| TheMue | davecheney: Heya | 09:51 |
| davecheney | TheMue: howdy | 09:51 |
| davecheney | i'll leave the firewaller to you | 09:51 |
| TheMue | davecheney: Thx *bow* | 09:53 |
| fwereade_ | davecheney, heyhey | 10:01 |
| davecheney | 'sup ! | 10:03 |
| * TheMue thankfully bought the Ubuntu fleece jacket in Oakland. It's pretty cold today here. | 10:14 | |
| * fwereade_ is exhausted by the mere prospect of the 10-minute walk in blazing sunshine to collect laura from nursery | 10:49 | |
| fwereade_ | bbs | 10:49 |
| davecheney | niemeiyer: any comment ? https://www.youtube.com/watch?v=32DD4DF7Qpo | 11:00 |
| rogpeppe | davecheney, TheMue, fwereade_: request for comment: https://codereview.appspot.com/6373048/ | 11:11 |
| rogpeppe | davecheney: lol | 11:14 |
| * davecheney reads | 11:15 | |
| rogpeppe | fwereade_: i'm not sure that passing a tomb around would be a good idea | 11:56 |
| fwereade_ | rogpeppe, yeah, it feels too bulky | 11:56 |
| fwereade_ | rogpeppe, potentially | 11:57 |
| rogpeppe | fwereade_: a tomb has a single idea of an error | 11:57 |
| fwereade_ | rogpeppe, ah, sorry, what's the problem there? | 11:57 |
| rogpeppe | fwereade_: my main inspiration for the CL was my realisation that there's a fundamenal asymmetry using channels | 11:57 |
| rogpeppe | fwereade_: channels fan in but they don't fan out | 11:57 |
| fwereade_ | rogpeppe, yeah | 11:57 |
| rogpeppe | fwereade_: which leads me to think that stopping your sources is more appropriate as a broadcast. | 11:58 |
| rogpeppe | fwereade_: then each source tells you when it has responded and completed. | 11:58 |
| TheMue | rogpeppe, fwereade_: What's still somehow too complex for me is the chaining of goroutines. We do goroutine(baseWatcher) -> goroutine(somehowSpezializedWatcher) -> goroutine(neededWatcher) | 11:58 |
| rogpeppe | fwereade_: and when we've got a channel coming from that source, EOF on the channel seems the Right Way. | 11:59 |
| fwereade_ | rogpeppe, sure, that's what we do anyway, right? | 11:59 |
| rogpeppe | TheMue: chaining of goroutines is the Go Way. | 11:59 |
| fwereade_ | TheMue, yeah, the chaining of goroutines makes me happy | 11:59 |
| rogpeppe | fwereade_: yes, but the stopping is still one-to-one. | 11:59 |
| fwereade_ | TheMue, so long s they're sanely written they make very nice building blocks IMO | 11:59 |
| TheMue | rogpeppe, fwereade_: I would like to already tell the first one what I want to do and only think about the business logic. | 11:59 |
| rogpeppe | TheMue: i'm not sure what you mean by "the business logic" | 12:00 |
| fwereade_ | TheMue, IMO the good thing is that you *can* as long as you enforce communication by channels | 12:00 |
| TheMue | fwereade_: But we have so much overhead around it, each time with new tombs. | 12:00 |
| fwereade_ | rogpeppe, the domain logic if you prefer | 12:00 |
| rogpeppe | TheMue: my proposal does away with most of the tombs | 12:00 |
| fwereade_ | TheMue, is this overhead serious? | 12:00 |
| TheMue | rogpeppe: The stuff that I want to be done, sorry for the wording. | 12:00 |
| fwereade_ | rogpeppe, yeah, that's what makes me uncomfortable, I find the tombs *really* helpful | 12:01 |
| TheMue | fwereade_: IMHO it can be done more simple using interfaces. | 12:01 |
| rogpeppe | fwereade_: a tomb is overkill much of the time | 12:01 |
| rogpeppe | fwereade_: did you look at the watcher implementations in the CL? | 12:01 |
| fwereade_ | rogpeppe, if "knowing when you've stopped" is overkill, then yes | 12:01 |
| rogpeppe | fwereade_: they don't use a tomb, and the code is no more complex IMHO | 12:01 |
| rogpeppe | fwereade_: when you've only got one goroutine, there's no difficulty knowing when you've stopped :-) | 12:02 |
| rogpeppe | fwereade_: you return... | 12:02 |
| rogpeppe | TheMue: the fundamental problem we're trying to solve here is that there are things changing all over the system, and we need to respond to them in a coherent way | 12:03 |
| rogpeppe | s/to them/to the changes/ | 12:03 |
| rogpeppe | TheMue: channel multiplexing can work really nicely here, but it's a right pain if you have to select every time you want to send or receive on a channel. | 12:04 |
| TheMue | rogpeppe: How does your CL helps regarding this point. | 12:04 |
| TheMue | rogpeppe: OK, get's more clear. | 12:05 |
| rogpeppe | TheMue: you see the two pieces of code at the start of the CL description? | 12:05 |
| rogpeppe | TheMue: that's the simplification that this CL buys you | 12:05 |
| TheMue | rogpeppe: That's why I started with using range. | 12:05 |
| rogpeppe | TheMue: *exactly*! | 12:05 |
| TheMue | rogpeppe: Indeed. | 12:05 |
| rogpeppe | TheMue: but with the way things are, you *can't use range*. | 12:05 |
| rogpeppe | TheMue: which seems wrong. | 12:06 |
| rogpeppe | IMHO this CL lets us write more natural Go code. | 12:06 |
| TheMue | rogpeppe: ack | 12:06 |
| rogpeppe | fwereade_: one possibility i did consider was to allow passing in a nil stop channel to the watcher, which would mean "make your own stop channel"; and a method, Stop, which would close it. | 12:07 |
| rogpeppe | fwereade_: i even started doing it, but it doesn't work very well. | 12:08 |
| fwereade_ | rogpeppe, I think the thing I don;t like is the action-at-a-distance nature of it all | 12:09 |
| rogpeppe | fwereade_: you mean that stopping isn't synchronous? | 12:09 |
| fwereade_ | rogpeppe, you close the channel you originally passed in and a whole tree of goroutines shut themselves down, sending an arbitrary number of additional events, before everything is finally stopped | 12:10 |
| fwereade_ | rogpeppe, internally to the watchers the receive-until-close works nicely | 12:10 |
| fwereade_ | rogpeppe, but outside of them it feels like we're exposing our internals in a slightly inappropriate way | 12:11 |
| fwereade_ | rogpeppe, does that make sense? | 12:11 |
| rogpeppe | fwereade_: i know what you mean, but i think that it's not an internal thing - it's that telling anything to stop is a two-way process - we tell them to stop and then they get around to doing it. | 12:12 |
| rogpeppe | fwereade_: and that an *inevitable* consequence of a synchronous stop is that you have to select on every channel send. | 12:12 |
| rogpeppe | fwereade_: and i think that leads to uglier code overall | 12:12 |
| rogpeppe | fwereade_: particularly when we come to the unit agent | 12:13 |
| rogpeppe | fwereade_: and the firewall code | 12:13 |
| rogpeppe | fwereade_: and, i think that it really won't be hard at all to avoid relying on synchronous-stop semantics | 12:14 |
| fwereade_ | rogpeppe, won't the firewall code also have to deal with a lot more state to figure out what events should just be dropped because we don't care about them any more? | 12:14 |
| rogpeppe | fwereade_: i don't think so. | 12:14 |
| rogpeppe | fwereade_: we'll only stop things at the end, AFAICS | 12:14 |
| fwereade_ | rogpeppe, you surely have a clearer idea of the details there than me | 12:14 |
| fwereade_ | rogpeppe, surely when a machine is terminated you'll want to stop paying attention to changes? | 12:15 |
| fwereade_ | rogpeppe, some changes anyway | 12:15 |
| fwereade_ | rogpeppe, which could still be queued up from the past but will potentially want to write to state that you've deleted? | 12:15 |
| rogpeppe | fwereade_: if that's the case, it's trivial to mark the machine as dead | 12:15 |
| rogpeppe | fwereade_: if we see an update from a dead machine, we'll just ignore it. one if. | 12:16 |
| fwereade_ | rogpeppe, ok, and when do we tidy up the dead machines? | 12:16 |
| rogpeppe | fwereade_: GC | 12:16 |
| fwereade_ | rogpeppe, how? when do we know that we we're guaranteed no more changes for a machine? | 12:17 |
| rogpeppe | fwereade_: we don't. the goroutine that's responsible for sending on the channel holds a reference to the machine. | 12:17 |
| rogpeppe | fwereade_: when it exits, the last reference goes | 12:17 |
| fwereade_ | rogpeppe, ok, and how do we tell that goroutine to make the machine dead? | 12:17 |
| rogpeppe | fwereade_: we don't | 12:18 |
| fwereade_ | rogpeppe, we'll be findin out it's dead on a separate goroutine, right? | 12:18 |
| rogpeppe | fwereade_: no | 12:18 |
| rogpeppe | fwereade_: hold on | 12:18 |
| rogpeppe | fwereade_: let me paste some code | 12:18 |
| fwereade_ | rogpeppe, thnks | 12:18 |
| rog | fwereade_: bugger, my machine just randomly rebooted for no apparent reason | 12:23 |
| rog | fwereade_: i was about 10 lines into my example | 12:23 |
| rog | fwereade_: will be another 5 mins | 12:23 |
| fwereade_ | rog, np, I'll stick some food on | 12:25 |
| rog | fwereade_: http://paste.ubuntu.com/1094802/ | 12:36 |
| fwereade_ | rog, sorry, will deal converse about this in a bit | 12:44 |
| rog | fwereade_: np | 12:45 |
| * Aram is feeling just slightly slightly better. | 12:50 | |
| rog | fwereade_: why don't you like Environ.Config BTW? | 12:50 |
| Aram | just enough to get out of bed :). | 12:50 |
| niemeyer | Good morning! | 12:58 |
| rog | niemeyer: yo! | 13:03 |
| rog | niemeyer: did you have a good conference? | 13:03 |
| rog | Aram: if you feel bad, just take the day off sick... | 13:03 |
| niemeyer | rog: Yeah, it was pretty nice | 13:03 |
| rog | niemeyer: your talk went down well, i trust | 13:04 |
| niemeyer | rog: Nothing so different, though, given I had been in other MongoDB conferences recently | 13:04 |
| niemeyer | rog: Seeing old friends and making new ones is great, though | 13:04 |
| niemeyer | rog: Yeah, talk was alright | 13:04 |
| rog | niemeyer: yeah | 13:04 |
| fwereade_ | niemeyer, heyhey | 13:04 |
| niemeyer | rog: Haven't had much time to really prepare something great, so it was below what I'd like to do | 13:05 |
| niemeyer | fwereade_: Heya! | 13:05 |
| rog | niemeyer: you can't do everything! | 13:05 |
| niemeyer | The 5-Gram Experiment turned out nicely, though | 13:05 |
| fwereade_ | rog, I do like your environ config... did I comment on the wrong CL or something? :) | 13:05 |
| niemeyer | rog: True that is :) | 13:05 |
| rog | niemeyer: i've been thinking about watchers recently, and i wonder what you think of this: https://codereview.appspot.com/6373048/ | 13:05 |
| rog | fwereade_: i meant, specifically the Config method on Environ. | 13:06 |
| fwereade_ | rog, ah sorry | 13:07 |
| fwereade_ | rog, I don't see a use case for it | 13:08 |
| fwereade_ | rog, when would you need it? | 13:08 |
| rog | fwereade_: apart from anything else, it makes for a great test | 13:08 |
| fwereade_ | rog, do we really need anything more than name? | 13:08 |
| rog | fwereade_: that caught quite a few errors when i was doing the branch | 13:08 |
| fwereade_ | rog, hm, like what? | 13:08 |
| rog | fwereade_: times when i'd forgotten to return all the attributes from Attrs, for example. | 13:09 |
| rog | fwereade_: i can also see it being useful for debugging | 13:09 |
| rog | fwereade_: (it makes it trivial to print all the attributes of the current environ) | 13:09 |
| fwereade_ | rog, I dunno, I think we can manipulate and verify and test configs just fine on their own | 13:09 |
| rog | fwereade_: by putting the test in jujutest.LiveTests, we automatically get that test for every environ that passes through that, which is more or less every environ tests | 13:10 |
| rog | s/tests/tested | 13:10 |
| fwereade_ | rog, but that environ is usually wrong | 13:11 |
| fwereade_ | rog, so it will make debugging harder :p | 13:11 |
| rog | fwereade_: it doesn't matter if it's wrong or right - it should still roundtrip | 13:11 |
| rog | fwereade_: sorry, wrong why? | 13:11 |
| niemeyer | rog: This code has just been refactored by fwereade_.. I think it's time for us to let it alone a bit so we can finish the rest | 13:12 |
| rog | niemeyer: fwereade_'s refactoring made it nice and easy to make this change :-) | 13:12 |
| rog | niemeyer: and i'm pretty sure it'll make writing the firewall and unit agent code much easier | 13:12 |
| niemeyer | rog: I don't feel like the code has improved significantly either, to be honest | 13:13 |
| rog | niemeyer: what about the two code fragments in the CL description? | 13:13 |
| rog | niemeyer: that's a common idiom | 13:13 |
| niemeyer | https://codereview.appspot.com/6373048/diff/5001/state/watcher.go | 13:13 |
| niemeyer | Line 44 on the new watcher | 13:13 |
| niemeyer | 32 on the old file | 13:13 |
| rog | niemeyer: i changed the name because it does the go itself now | 13:14 |
| niemeyer | It seems just as tricky, and requires as much understanding of the surroundings, or perhaps more, than the previous version | 13:14 |
| rog | niemeyer: perhaps runLoop might be better | 13:14 |
| fwereade_ | rog, I see how you do the machine changes, I guess it's ok, but I'd still rather have the watchers behave prdictably than to have simple idioms for handling their unpredictability | 13:14 |
| niemeyer | rog: Not worried about naming | 13:15 |
| niemeyer | fwereade_++ | 13:15 |
| fwereade_ | rog, even at the cost of a few selects :) | 13:15 |
| rog | i think we should be able to write nice idiomatic Go code. and selecting on *everything* isn't good for that. | 13:16 |
| fwereade_ | rog, and I don't think the UA stuff is too much of a car crash with the existing style ;) | 13:16 |
| fwereade_ | rog, mileages may ofc vary | 13:17 |
| rog | the "watcher cleans itself up and tells you when it has done" idiom works well, and is commonly used | 13:17 |
| * rog is convinced this, or something like it, is the Right Way to do it. | 13:18 | |
| rog | niemeyer: that code seems pretty equivalent to me. | 13:19 |
| rog | niemeyer: it's nice we can use a range, of course | 13:20 |
| rog | niemeyer: and 5 lines shorter is nice too | 13:21 |
| niemeyer | rog: LOL | 13:21 |
| rog | niemeyer: i agree it doesn't make much difference at the edges, but it really helps the multiplexer case, and both the firewaller and the unit agent do a lot of multiplexing. | 13:23 |
| niemeyer | rog: I haven't seen any kind of improvement in that direction in the CL | 13:24 |
| rog | niemeyer: that's because we don't have any multiplexers yet | 13:24 |
| niemeyer | rog: In fact, both the provisioner and the machiner have gained complexity | 13:24 |
| niemeyer | rog: Rather than being simplified | 13:24 |
| rog | niemeyer: i don't think one line extra is much gain. | 13:25 |
| niemeyer | rog: and now we have those awkward stop channels spread through the whole code base | 13:25 |
| niemeyer | (of state, specifically) | 13:25 |
| niemeyer | rog: I'd prefer to not do that now, and as suggested focus on pushing things forward | 13:25 |
| rog | niemeyer: yes, that is indeed the worst bit of it | 13:25 |
| niemeyer | rog: mstate is coming along as well | 13:26 |
| rog | niemeyer: whether we're using mstate or state doesn't make much difference to the code i'm thinking about | 13:27 |
| niemeyer | rog: Yes, it doesn't, but this is changing the interface while Aram is working on it | 13:27 |
| rog | niemeyer: ok, that's a reasonable point. | 13:27 |
| niemeyer | rog: It'd be great if we could stop fuzzing with this interface for a few weeks while we push the implementation of agents forward | 13:27 |
| rog | niemeyer: it was precisely because i was thinking about the implementation of agents that i wanted to make this change | 13:28 |
| rog | niemeyer: but i hope you'll bear this in mind when we find that the firewall code looks unnecessarily big and bulky | 13:29 |
| niemeyer | rog: That's only part of the suggestion ;-) | 13:29 |
| niemeyer | rog: I haven't seen it, but given the current agents we have in place, I really hope that this isn't the case | 13:29 |
| rog | niemeyer: ok, time for another slapdown. i was sympathetic to fwereade_'s cries of distress over the config stuff, tried to make it better, but couldn't. i then wondered whether an alternative approach might work. https://codereview.appspot.com/6353092/ | 13:33 |
| fwereade_ | niemeyer, rog: IMO the ugliest bit of the UA is likely to be the relationUnitWatcher, and I think that worked out tolerably | 13:33 |
| rog | niemeyer: it's lacking tests, and william made some comments that i will address if you like it. | 13:34 |
| niemeyer | rog: This is a branch from William? | 13:35 |
| rog | niemeyer: no, this was something i did very quickly while trying to work out what might work. | 13:36 |
| rog | niemeyer: i wanted to avoid saying yet again "try this" without knowing the implications | 13:36 |
| niemeyer | rog: This is a branch from William | 13:37 |
| rog | niemeyer: oops, so it is | 13:37 |
| rog | niemeyer: https://codereview.appspot.com/6343107/ | 13:37 |
| fwereade_ | niemeyer, sorry about that branch -- I did my best to follow what you wanted but wasn't able to make it nice | 13:38 |
| fwereade_ | niemeyer, davecheney thought it was kinda OK so it might be an ok fallback, but I thought rog's approach was much cleaner | 13:39 |
| niemeyer | Oh, this again.. | 13:40 |
| fwereade_ | niemeyer, sorry | 13:41 |
| fwereade_ | niemeyer, believe me the thorn has been in my side too :) | 13:41 |
| rog | fwereade_: i'd like to see your code for relationUnitWatcher | 13:41 |
| niemeyer | fwereade_, rog: So, rather than evaluating yet another proposal from the ground up, what is the problem with the one brought up in the mailing list? | 13:42 |
| rog | fwereade_: it's your code, you say :-) | 13:43 |
| fwereade_ | niemeyer, the problem is that I tried my best and was not satisfied with the result | 13:43 |
| niemeyer | fwereade_: That's not a problem.. that seems like a consequence of the problem | 13:43 |
| rog | from my point of view there was lots of code for little result | 13:44 |
| rog | having the config type hold attributes for two different things made things awkward. | 13:45 |
| fwereade_ | niemeyer, well, my version is still up on codereview if you want to take a look at that; I was not happy with it, though, and I didn't think you would be either | 13:45 |
| rog | conversely, embedding it seems to make things fall out more naturally. | 13:45 |
| niemeyer | fwereade_: Heh.. | 13:46 |
| fwereade_ | niemeyer, if I misjudged that then sorry; as it was I felt I'd screwed up the 3rd or 4th attempt at implementing it, and cutting my losses did not seem like a bad idea, especially since it let me get back to the unit agent | 13:46 |
| niemeyer | fwereade_: Is there a reason you can identify why that is the case? :-) | 13:46 |
| rog | niemeyer: the ComposeConfig function in this file seems to me to epitomise the difficulties: https://codereview.appspot.com/6353092/diff/2001/environs/ec2/provider.go | 13:47 |
| fwereade_ | niemeyer, I feel like the actualy code is ugly and hard to follow and I'm still suspicious of corner cases despite heavy testing | 13:47 |
| niemeyer | rog: The proposal didn't even have this method | 13:47 |
| rog | niemeyer: it's fwereade_'s name for Validate | 13:48 |
| rog | niemeyer: same semantics | 13:48 |
| fwereade_ | niemeyer, by best guess is that I Just Do Not Get It, and have failed to apprehend some aspect of what you're looking for | 13:48 |
| niemeyer | rog: There was nothing in config with that semantics, whatever its name | 13:48 |
| fwereade_ | niemeyer, indeed; I tried to do what you were asking but I presumably misunderstood something, or was too obsessive about validation, or *something* that I cannot precisely identify | 13:49 |
| niemeyer | fwereade_: Okay.. I think I'll give it a try then | 13:49 |
| fwereade_ | niemeyer, I look forward to the inevitable schooling ;p | 13:50 |
| fwereade_ | niemeyer, but I'm sorry to add more to your plate :( | 13:50 |
| niemeyer | fwereade_: Well, one of us will definitely learn something.. not sure which yet :-) | 13:51 |
| fwereade_ | niemeyer, we'll see :) | 13:52 |
| rog | [Monday 09 July 2012] [14:54:01] <rog>niemeyer: something like this, perhaps? http://paste.ubuntu.com/1082778/ | 13:55 |
| rog | [Monday 09 July 2012] [14:54:26] <niemeyer>rog: Yeah, except it should be called Validate as we've been agreeing on | 13:55 |
| niemeyer | ?! | 13:57 |
| rog | niemeyer: it takes an old config, a new config and returns a validated config by composing the two. seems fairly similar. | 13:57 |
| niemeyer | rog: Heh | 13:58 |
| rog | niemeyer: but like william, i probably misunderstood | 13:58 |
| niemeyer | rog: No, you didn't.. you just changed the proposal | 13:58 |
| niemeyer | rog: That paste is very different from what the CL has | 13:58 |
| niemeyer | rog: Anyway, would you mind if I tried to implement the proposal made, in smaller chunks? | 13:59 |
| rog | niemeyer: that's true, but i tried to move in that direction and failed. | 13:59 |
| rog | niemeyer: sigh | 13:59 |
| rog | niemeyer: i think we have spent quite a long time on this | 13:59 |
| niemeyer | rog: Yes, we have indeed! | 13:59 |
| rog | niemeyer: and i don't think my proposal is too shit | 13:59 |
| niemeyer | rog: Yet all the time I spent on it seems a bit wasted | 13:59 |
| niemeyer | rog: Is my proposal shit then? | 14:00 |
| niemeyer | :-/ | 14:00 |
| rog | niemeyer: i didn't see yours implemented | 14:00 |
| rog | niemeyer: but i'm sure fwereade_ did the best he could | 14:00 |
| niemeyer | rog: fwereade_ was working on it, supposedly | 14:00 |
| niemeyer | rog: and you came up with something else.. | 14:00 |
| rog | niemeyer: yes, because i tried to make something like yours work, and couldn't. | 14:01 |
| fwereade_ | niemeyer, I was working on it, and I proposed a CL that pretty clearly betrayed my state of despair | 14:01 |
| rog | niemeyer: and i think that it ended up really quite nice a different way | 14:01 |
| fwereade_ | niemeyer, so from my perspective it was a helpful thing to do that brought a great sense of relief | 14:02 |
| niemeyer | That's all fine, but please don't judge me as I try to implement what I actually presented and was ignored | 14:02 |
| fwereade_ | niemeyer, I would love to see you make it work | 14:02 |
| niemeyer | "We've spent quite a long time on this", as you say | 14:02 |
| fwereade_ | niemeyer, please do not think I did not try | 14:02 |
| rog | niemeyer: ok, have a go | 14:02 |
| niemeyer | Thanks | 14:02 |
| fwereade_ | niemeyer, and as you say one of us will learn from it; I suspect it will be me, because it usually is :) | 14:03 |
| rog | niemeyer: seems like we have already got something workable, but go for it | 14:03 |
| niemeyer | rog: We've got lots of things workable.. fwereade_ had a different proposal a while ago that was workable too | 14:03 |
| rog | niemeyer: fair enough | 14:04 |
| niemeyer | rog: I don't appreciate the fact New returns "unknown" for example | 14:04 |
| rog | niemeyer: ? | 14:04 |
| niemeyer | rog: Nor the fact that there's no way for an environment to validate settings | 14:04 |
| rog | niemeyer: what? | 14:04 |
| niemeyer | rog: All of those things in your proposal were already debated | 14:04 |
| rog | niemeyer: which New returns "unknown"? | 14:04 |
| rog | niemeyer: and all environment settings are validated in my proposal | 14:05 |
| rog | niemeyer: at least, that was the intention | 14:05 |
| niemeyer | rog: There's no hook point as far as I can see to compare an old and a new configuration | 14:05 |
| niemeyer | rog: and prevent changes at the client side | 14:05 |
| rog | niemeyer: environs.Config.Change | 14:05 |
| niemeyer | rog: In case the modification is rendered invalid | 14:05 |
| niemeyer | rog: This must be per environment | 14:06 |
| rog | niemeyer: it *is* per environment | 14:06 |
| niemeyer | rog: I don't think I understand | 14:06 |
| rog | niemeyer: environs.EnvironConfig is implemented by each environment | 14:06 |
| niemeyer | rog: Ah, I see | 14:06 |
| rog | niemeyer: you can embed config.Config to get the common stuff | 14:07 |
| niemeyer | This is the unknowns I was talking about: 38 // New creates a new Config from the given attributes, and also returns any attr | 14:07 |
| niemeyer | ibutes | 14:07 |
| niemeyer | 39 // not known. | 14:07 |
| niemeyer | 40 func New(attrs map[string]interface{}) (*Config, map[string]interface{}, error) | 14:07 |
| niemeyer | rog: How do we return this from state? | 14:07 |
| rog | niemeyer: that's because it's designed to be embedded. | 14:07 |
| niemeyer | rog: How do we return this from state? | 14:08 |
| rog | niemeyer: have a look at how it's used in https://codereview.appspot.com/6343107/diff/11001/environs/ec2/config.go | 14:08 |
| niemeyer | rog: How do we return this from state? | 14:08 |
| niemeyer | :-( | 14:08 |
| rog | niemeyer: what state? | 14:08 |
| niemeyer | rog: *State.EnvironConfig() | 14:08 |
| rog | niemeyer: i don't think we need to do that. | 14:09 |
| niemeyer | rog: Heh | 14:09 |
| niemeyer | rog: That whole conversation started there | 14:09 |
| rog | niemeyer: i known. but i think it's a false premise | 14:09 |
| rog | niemeyer: but... | 14:09 |
| rog | niemeyer: if we want it to, then EnvironConfig could live inside the config package | 14:10 |
| niemeyer | rog: and then all the registry has to live there as well | 14:10 |
| rog | niemeyer: well... either state uses an environ to validate what comes out of the db or not | 14:11 |
| rog | niemeyer: if it *does* then there has to be some registry not in environs | 14:11 |
| rog | niemeyer: if it does not, then it cannot return a validated environ config | 14:11 |
| niemeyer | rog: We can have a first class type being returned from state even if it's not validated | 14:11 |
| niemeyer | rog: Because it was already validated on entrance | 14:12 |
| rog | niemeyer: why not just return the attributes? | 14:12 |
| rog | niemeyer: same difference | 14:12 |
| niemeyer | Maybe I should just give up on this.. it's like the fourth time I'm going over this | 14:13 |
| niemeyer | Rather than converging there's a brand new implementation every time | 14:14 |
| rog | niemeyer: so your objection to my branch is that we can't return a config.Config from state? | 14:14 |
| niemeyer | rog: I'm just going bit by bit about how the proposal sent to the mailing list, for which there's no reply other than "looks great", was put in place | 14:15 |
| rog | niemeyer: are you talking about my proposal now, or william's, or yours? | 14:16 |
| niemeyer | rog: Heh | 14:16 |
| TheMue | So, next iteration of firewaller is in for review: https://codereview.appspot.com/6404051 . And this time the adding of the former CL as a requisite worked. Yeah! | 15:07 |
| rog | TheMue: i'm taking a look | 15:21 |
| niemeyer | rog, TheMue, fwereade_, Aram: Are any of you planning on buying a Raspberry Pi? | 15:29 |
| rog | niemeyer: not currently | 15:30 |
| rog | niemeyer: i don't have any spare geek time :-) | 15:30 |
| niemeyer | rog: Hehe :) | 15:30 |
| fwereade_ | niemeyer, not really, I like the idea but doubt I'd actually do anything with it if I had one ;) | 15:30 |
| fwereade_ | niemeyer, what rog said ;) | 15:30 |
| niemeyer | rog: That's wise.. I have a bunch of hardware I've never played at home :-) | 15:30 |
| niemeyer | Some of them did make up for some good time, though | 15:30 |
| rog | niemeyer: yeah, and what fwereade_ said too - too much decaying h/w in the attic | 15:31 |
| niemeyer | mramm: What about you, any plans? | 15:31 |
| TheMue | niemeyer: I'm not planing it, I never had been good in HW things. Even if it looks interesting. | 15:32 |
| mramm | niemeyer: no plans yet | 15:32 |
| niemeyer | Crap :) | 15:32 |
| mramm | niemeyer: but I keep thinking about it | 15:32 |
| TheMue | niemeyer: But a box with a Tilera 64 would be nice. ;) | 15:32 |
| Aram | niemeyer: I am planning to buy one (or more), why? | 15:32 |
| niemeyer | Aram: Because they are now available, and I was going to ask someone to take one to the sprint in case they arrived on time | 15:33 |
| niemeyer | Aram: http://www.raspberrypi.org/archives/1588 | 15:33 |
| niemeyer | Aram: But now that I read it, it won't fly | 15:33 |
| Aram | I'll order some, but it won't get in time | 15:33 |
| niemeyer | It's taking months to deliver | 15:33 |
| niemeyer | Yeah | 15:34 |
| mramm | There is a guy at the co-working space I sometimes use who has a couple | 15:34 |
| mramm | but he's been on the list forever | 15:34 |
| mramm | and has a startup that's doing a bunch of open source hardware/computer vision stuff | 15:35 |
| mramm | so he's unlikely to let me take one away from him for 3 weeks | 15:36 |
| mramm | (which is how long I'll be away counting this sprint through the one on the isle of man) | 15:37 |
| niemeyer | mramm: Aw :) | 15:53 |
| mramm | haha | 15:53 |
| niemeyer | Okay, I'll grab some quick lunch and bbiab | 15:54 |
| rog | TheMue: you have a review | 16:40 |
| rog | TheMue: i'm concerned about the unsafe access to firewaller private variables from the tests. | 16:41 |
| rog | TheMue: they could be flaky in a very hard-to-diagnose way in heavily loaded environments | 16:42 |
| rog | TheMue: how about something like this, so allow instrumenting the internal state: http://paste.ubuntu.com/1095153/ | 16:42 |
| rog | TheMue: again, with a comment on CheckProgress this time: http://paste.ubuntu.com/1095160/ | 16:44 |
| TheMue | rog: Thx, have to get deeper behind your idea. | 16:59 |
| rog | TheMue: how do you mean? | 16:59 |
| TheMue | rog: To understand it. Only had two quick views on it. | 16:59 |
| rog | TheMue: which idea? | 17:00 |
| TheMue | rog: Huh, the one you posted above. | 17:02 |
| rog | TheMue: ah, the test instrumenting idea? | 17:02 |
| TheMue | rog: Yes. | 17:02 |
| niemeyer | "The conference will begin when the LEADER arrives.. there are.. currently.. *9* other participants." | 17:02 |
| rog | niemeyer: ha | 17:03 |
| TheMue | rog: Where do I access private variables? | 17:03 |
| rog | TheMue: i don't actually like it that much, because it clutters the runtime code for the tests, but i don't see a better alternative | 17:03 |
| mramm | niemeyer: yea, the hold music is particularly awesome today too | 17:03 |
| rog | TheMue: in CheckProgress you can safely call functions defined in export_test.go that access private variables | 17:04 |
| niemeyer | mramm: Seems the same for me | 17:04 |
| * niemeyer tries to code without being disturbed by the bad music | 17:04 | |
| rog | TheMue: i thought about another alternative that adds a global variable, but i didn't like it | 17:05 |
| TheMue | rog: In other places we use that kind of export_test too | 17:05 |
| TheMue | rog: No, indeed, no global. | 17:05 |
| rog | TheMue: there's nowhere as far as i know that accesses private variables unsafely. | 17:05 |
| fwereade_ | so, it turns out somehow it's past 7 | 17:05 |
| mramm | niemeyer: perhaps I just haven't listened to it long enough | 17:05 |
| fwereade_ | gn all, see you tomorrow | 17:05 |
| mramm | fwereade_: good night | 17:05 |
| rog | fwereade_: gn, enjoy the evening! | 17:05 |
| TheMue | fwereade_: enjoy | 17:06 |
| rog | i've gotta go in a moment too | 17:06 |
| * TheMue too, but later I'll take a deeper look into rogs ideas | 17:06 | |
| rog | TheMue: another (and worse) alternative is to put a mutex around the private variables. | 17:07 |
| TheMue | rog: No, the tests shouldn't bother the productive code | 17:08 |
| rog | TheMue: i agree, but i don't see a good alternative | 17:08 |
| rog | TheMue: we're trying to introspect the internal state of the firewaller | 17:08 |
| rog | TheMue: i was trying to make the instrumentation code as minimal as possible, but it's still there. | 17:09 |
| rog | TheMue: to be honest, i wouldn't mind no tests at all for this code and lots of tests for the final behaviour which is what we're after | 17:10 |
| TheMue | rog: So maybe I could keep it while we're approaching the final state and then remove it. | 17:11 |
| rog | TheMue: but until we get there, something like this is useful i think. we can delete these tests later. | 17:11 |
| rog | TheMue: exactly | 17:12 |
| rog | TheMue: if there was a "test" build flag, we could do it with no overhead... | 17:12 |
| TheMue | rog: ??? How? | 17:13 |
| rog | TheMue: i'd have two type definitions, one in testing mode and the other in non-testing mode. the non-testing mode type would have stubs for the functions, which would be empty. the testing mode would have members like testStub in my idea. | 17:14 |
| rog | TheMue: calls to empty functions are removed by the compiler, so... no overhead. | 17:14 |
| rog | right, i'm off for the day. see y'all tomorrow. | 17:19 |
| TheMue | rog: Interesting, thx for sharing. We should discuss it tomorrow. | 17:19 |
| rog | TheMue: definitely | 17:19 |
| TheMue | rog: Enjoy, here is dinner time too | 17:20 |
| * TheMue waves | 17:20 | |
| niemeyer | davecheney: yo! | 23:05 |
| niemeyer | davecheney: how're things going there? | 23:06 |
| davecheney | niemeyer: hey, how was your conference ? | 23:16 |
| niemeyer | davecheney: It was pretty good | 23:16 |
| niemeyer | davecheney: Always a chance to meet old and new friends | 23:17 |
| niemeyer | davecheney: and exchange ideas on how things have been moving | 23:17 |
| davecheney | indeed | 23:17 |
| niemeyer | davecheney: How're things going for you? | 23:20 |
| davecheney | good, just talking a spin through roger and williams comments | 23:20 |
| niemeyer | davecheney: Do you have a moment for a quick call? | 23:21 |
| davecheney | siure | 23:22 |
| davecheney | skype or g+ ? | 23:22 |
| niemeyer | davecheney: G+, if that works | 23:23 |
| davecheney | two secs | 23:23 |
| davecheney | niemeyer: just rearranging the myriad of usb devices | 23:25 |
| niemeyer | davecheney: Cool, no worries | 23:25 |
| davecheney | imma online, but you do not appear to be | 23:27 |
| niemeyer | davecheney: I've just invited you | 23:28 |
| niemeyer | davecheney: Maybe the wrong you? :) | 23:28 |
| davecheney | niemeyer: maybe, nothign here | 23:29 |
| niemeyer | https://plus.google.com/hangouts/_/4ac7f87247068630837d29e94eee4ad282d7c8f7?authuser=0&hl=en | 23:29 |
| niemeyer | OK: 12 passed | 23:52 |
| niemeyer | PASS | 23:52 |
| niemeyer | ok launchpad.net/juju-core/worker/provisioner 8.444s | 23:52 |
| niemeyer | davecheney: !! | 23:52 |
| niemeyer | :-) | 23:52 |
| davecheney | niemeyer: whoop whoop | 23:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!