[00:11] <davecheney> niemeyer: thanks for your review
[00:11] <niemeyer> davecheney: My pleasure
[00:12] <davecheney> the reason the firewaller watches the environ is I imagined that would be how it woudl talk to the (unknown) firewalling service
[00:12] <davecheney> as the Environ in this case represents ec2
[00:12] <davecheney> niemeyer: in the pythonic version, the provider managed openport/closeport
[00:13] <niemeyer> davecheney: Duh.. it is obvious indeed, sorry
[00:14] <davecheney> niemeyer: well, the facility doesn't exist on environ, yet
[00:14] <niemeyer> davecheney: Which facility?
[00:15] <davecheney> open port/close port
[00:15] <davecheney> it's there in goamz
[00:15] <davecheney> but there is no generic interface in environs
[00:16] <niemeyer> davecheney: Ah, I see, yep
[00:17] <niemeyer> davecheney: It kind of sucks that we'll have to redo this soon to be machine-based
[00:17] <niemeyer> davecheney: But it seems like the right thing to do, rather than get into a trip in unknown territory before we actually get things working at all
[00:18] <davecheney> niemeyer: right o
[00:18] <davecheney> niemeyer: i'm going to raise a bug on juju-core to define the open port / close port interface on environ
[00:18] <davecheney> if that is ok
[00:18] <niemeyer> davecheney: E.g. if we moved onto the machine agent, we actually have to watch relations too, at the machine level, so that we alter the firewall allowing them to go in and out
[00:19] <niemeyer> davecheney: Sounds great
[00:19] <niemeyer> davecheney: Thanks
[00:19] <davecheney> niemeyer: the reason it's a new type, not an addition to the Provisioner, was a hope that the service could be moved between agents
[00:19] <davecheney> but i don't know how realisitic that will be
[00:19] <niemeyer> davecheney: I love the fact you splitted it out
[00:20] <niemeyer> davecheney: It was a bad decision on the original implementation
[00:20] <niemeyer> davecheney: Even if we can't move it as-is, we can move something, or we can even erase it by itself if nothing else
[00:20] <davecheney> yup
[00:25] <niemeyer> davecheney: Hmm
[00:25] <niemeyer> davecheney: I don't think we should allow the environment name to change
[00:26] <niemeyer> davecheney: We that value internally in the environment itself
[00:26] <niemeyer> We use
[00:26] <davecheney> niemeyer: ok, i'll do it another way
[00:26] <niemeyer> davecheney: E.g. security groups
[00:26] <davecheney> it wasn't being set at all
[00:26] <niemeyer> davecheney: It feels slightly bad that we do this
[00:26] <niemeyer> davecheney: It would be better to have that as an external property of the environment
[00:27] <niemeyer> davecheney: But it's helpful to tell the user about what those machines he's running are about
[00:27] <davecheney> niemeyer: the current thing that is blocking bootstrap is there are no tools in my s3 bucket
[00:27] <davecheney> i can't see anything to generate those
[00:27] <niemeyer> davecheney: Oh
[00:27] <davecheney> 012/06/27 10:27:45 JUJU findTools searching for {{0 0 0} precise amd64} in []
[00:27] <davecheney> error: cannot start bootstrap instance: cannot find juju tools that would work on the specified instance: no compatible tools found
[00:28] <niemeyer> davecheney: bootstrap -upload-tools?
[00:28] <niemeyer> davecheney: bootstrap --upload-tools?
[00:28] <niemeyer> davecheney: I think you're right, though
[00:29] <niemeyer> davecheney: We do need to set the environment name, at least once
[00:29] <davecheney> 2012/06/27 10:28:50 JUJU environs: putting tools tools/juju-0.0.0-precise-amd64.tgz
[00:29] <davecheney> niemeyer: i'll find a nicer way to do it
[00:29] <niemeyer> davecheney: I feel like your CL is a good step forward, actually
[00:29] <niemeyer> davecheney: I'm probably mixing problems up
[00:29] <niemeyer> davecheney: We should prevent the user from replacing the name, but that's not the way to do it
[00:30] <davecheney> niemeyer: the name is an unfortunately property that comes out of the yaml file
[00:31] <niemeyer> davecheney: I'll file a bug, and LGTM your change
[00:31] <davecheney> niemeyer: ta, just a small cosmetic i found
[00:31] <niemeyer> davecheney: This may well be what's preventing it from running
[00:32] <davecheney> 2012/06/27 10:31:34 JUJU environs/ec2: started instance "i-acd0d3d5"
[00:35] <davecheney> niemeyer: dude, it bootstrapped !
[00:35] <niemeyer> davecheney: Woah!
[00:35] <davecheney> niemeyer: i'm going to pull this branch apart into smaller pieces, but i think it's working
[00:35] <davecheney> no idea if there is a PA at the other end yet
[00:35] <davecheney> also, juju status would be nice to have :)
[00:35] <niemeyer> davecheney: +1 :)
[00:36] <davecheney> niemeyer: time for a celebratory cup of tea
[00:36] <niemeyer> davecheney: Feel free to start looking at it actually, if you ever get blocked/bothered
[00:36] <niemeyer> s/bothered/bored/
[00:36] <davecheney> niemeyer: ok
[00:36] <davecheney> niemeyer: i'm not going to step on franks toes ?
[00:36] <davecheney> screw it, i'll just email juju dev and claim it
[00:38] <davecheney> niemeyer: 2012/06/27 10:34:27 JUJU environs: putting tools tools/juju-0.0.0-precise-amd64.tgz
[00:38] <davecheney> error: cannot upload tools: cannot write file "tools/juju-0.0.0-precise-amd64.tgz" to control bucket: Your socket connection to the server was not read from or written to within the timeout period. Idle connections will be closed.
[00:38] <davecheney> ^ kinda unreliable, but that is australia for ya
[00:38] <davecheney> maybe i'll try a different availability zone
[00:38] <niemeyer> davecheney: Definitely not stepping on Frank's.. he's not looking at status ATM
[00:38] <davecheney> sorry, william
[00:38]  * davecheney facepalm
[00:39] <niemeyer> davecheney: That error was kind of weird
[00:39] <niemeyer> davecheney: I don't think anyone is doing status yet
[00:39] <niemeyer> davecheney: I've not seen that before.. worth investigating
[00:40] <davecheney> niemeyer: anyway, time for a break
[00:46] <niemeyer> Woohay.. new battery arrived
[00:46] <niemeyer> It's time for me to leave too
[00:46] <niemeyer> I'm late, actually
[00:46] <niemeyer> Dinner with Go folks tonight.. back later
[05:04] <davecheney> niemeyer: how was dinner ?
[05:04]  * davecheney is a tad jealous
[05:05] <niemeyer> davecheney: It was awesome indeed
[05:05] <davecheney> niemeyer: i think i figured out why juju can't bootstrap in other avail zones
[05:05] <niemeyer> davecheney: Not the full team, unfortunately
[05:05] <davecheney> niemeyer: i bet rsc wasn't there
[05:06] <davecheney> he's elusive
[05:06] <niemeyer> davecheney: Yeah, we had Rob and Andrew
[05:06] <niemeyer> davecheney: He lives elsewhere.. I bet he'd join if he was around
[05:06] <davecheney> he lives in boston right ?
[05:11] <davecheney> niemeyer: can I commit rogers branch ? or should I merge it into one I own to lbox propose ?
[05:12] <niemeyer> davecheney: The right thing would be to branch from it and continue, or possibly even break it down in smaller chunks
[05:12] <niemeyer> davecheney: Yeah, he lives in Boston, or next to it I think
[05:12] <davecheney> niemeyer: i'll try to break it into two chunks, it's only 100 lines
[05:12] <davecheney> actually less than 90
[05:13] <niemeyer> davecheney: Yeah, but it's also incomplete, right?
[05:13] <davecheney> it's very close
[05:13] <davecheney> [  102.246904] init: juju-provision-agent main process (4356) terminated with status 127
[05:13] <davecheney> [  102.246937] init: juju-provision-agent main process ended, respawning
[05:13] <davecheney> ^ jujud isn't in the path, that is pretty much the only bit missing
[05:14] <niemeyer> davecheney: I had the impression it was a spike mostly.. rog mentioned it had no testing
[05:14] <niemeyer> davecheney: I confess to not have looked into it, though
[05:15] <davecheney> he's done some tests, > 50% of the change is in livetests
[05:22] <niemeyer> davecheney: Ah, nice
[05:22] <niemeyer> davecheney: I wonder what he was referring to then
[05:23] <davecheney> maybe it was the environ name problem
[05:23] <niemeyer> davecheney: Well, you're in a much better position than I am
[05:23] <davecheney> since I fixed that it's been working pretty well for me
[05:23] <niemeyer> davecheney: Whatever you feel should go to review, I'll be happy to look at
[05:23] <davecheney> ok
[05:24] <davecheney> niemeyer: did my diagnosis of the s3 problem make sense ?
[05:25] <davecheney> niemeyer: ubuntu@domU-12-31-39-0C-58-F1:~$ pgrep jujud -lf
[05:25] <davecheney> 4539 /var/lib/juju/tools/juju-0.0.0-precise-amd64/jujud provisioning --zookeeper-servers localhost:2181 --log-file /var/log/juju/provision-agent.log
[05:37] <niemeyer> davecheney: I think it does.. it's definitely an error we'll want to fix
[05:38] <davecheney> i think it should be pretty easy to add support for that
[05:38] <niemeyer> davecheney: My surprise was with the fact Amazon refuses to take a request on a region-specific endpoint because the payload doesn't mention the region-specific endpoint name *as well*
[05:39] <niemeyer> davecheney: Woah? Provisioner running?
[05:39] <davecheney> yes, almost
[05:39] <davecheney> i think i got initzk working as well (path issues)
[05:39] <davecheney> but it's blocked waiting on the right value for /environment
[05:43] <davecheney> niemeyer: [zk: localhost:2181(CONNECTED) 2] ls /
[05:43] <davecheney> [services, charms, relations, zookeeper, initialized, machines, units]
[05:43] <davecheney> initzk worked, just missing /environment
[05:44] <davecheney> niemeyer: i don't understand why location constraint has to be xml, why can't it just be a header
[05:44] <niemeyer> davecheney: That's expected
[05:45] <niemeyer> davecheney: /environment comes from juju deploy
[05:45] <niemeyer> davecheney: Well.. it doesn't even need to be a header
[05:45] <niemeyer> davecheney: The endpoint URL itself is already a great indicator
[05:45] <niemeyer> davecheney: There's no way to be talking to that endpoint URL and not wanting to.. well.. talk to it
[05:45] <davecheney> niemeyer: i thought there was
[05:46] <niemeyer> davecheney: Kind of strange.. either a very awkward decision, or I'm missing something more interesting
[05:46] <davecheney> there is a note in one page I found that said there is a generic s3 location you can talk to
[05:46] <davecheney> but they recommend not using it, because it will internally redirect your request
[05:46] <niemeyer> davecheney: Sure, but we're not using it, AFAIK
[05:46] <davecheney> either way, it's a bit of a wart
[05:47] <davecheney> niemeyer: so, juju deploy will be responsible for keeping /environment up to date ?
[06:08] <niemeyer> davecheney: Yeah
[06:08] <niemeyer> davecheney: Well, hopefully not up-to-date
[06:08] <niemeyer> davecheney: Only the first time
[06:08] <niemeyer> davecheney: THe Python version updates it every time, but that's a mistake
[06:09] <davecheney> niemeyer: ok, so i should not make the bootstrap process seed /environment
[06:11] <niemeyer> davecheney: It's tricky because to do that we'd have to wait until the environment bootstrapped
[06:11] <niemeyer> davecheney: Chicken and egg
[06:11] <niemeyer> davecheney: We don't want to send credentials over via user-data
[06:15] <davecheney> niemeyer: right, so to confirm, the cloud-init script is not secure ?
[06:29] <niemeyer> davecheney: Right, it puts thing up in an address that can be openly accessed from within the machine
[06:40] <fwereade> niemeyer, davecheney: heyhey
[06:40] <davecheney> fwereade: howdy
[06:40] <fwereade> niemeyer, perhaps I'm missing something re: https://codereview.appspot.com/6335057/diff/3001/state/presence/presence.go#newcode397
[06:42] <fwereade> niemeyer, but it seems to me that we can only guarantee absence notifications from the watches if we don't stop the childLoops at all
[06:42] <fwereade> bah
[06:43] <fwereade> ok, I'm off to take laura to nursery
[06:45] <davecheney> yeah, i'm off soon as well
[06:54] <niemeyer> davecheney: Yo
[06:54] <niemeyer> davecheney: Please feel free to tackle the location bug if you're up for it
[06:54] <niemeyer> davecheney: I'll certainly have a low attention level in the next few days with I/O going by
[06:56] <niemeyer> Time to head to bed now
[06:56] <niemeyer> davecheney: Have a good time there, and talk to you tomorrow
[06:56] <niemeyer> Night all
[07:31] <TheMue> Morning.
[07:33] <fwereade> TheMue, heyhey
[10:45] <fwereade> TheMue, any objection to me just straight-up deleting relationUnitWatcher? after the meeting yesterday we have a better plan for doing essentially the same stuff
[10:46] <TheMue> fwereade: No problem, so I'll just don't touch it when doing the Deleted/Removed change.
[10:47] <fwereade> TheMue, I consider this trivial enough to go straight in with a cursory review; sound sane?
[10:47] <fwereade> TheMue, I'll be proposing in a couple of mins...
[10:47] <TheMue> fwereade: Does the mimic also may relate the ServiceUnitsWatcher?
[10:47] <fwereade> TheMue, sorry, can't parse
[10:48] <TheMue> fwereade: The way you will do it in future, will this be interesting for the service units watcher too?
[10:50] <fwereade> TheMue, ah sorry; I don't *think* so
[10:50] <fwereade> TheMue, I don't think that should be considering agent presence
[10:50] <fwereade> TheMue, but I may not have thought it through properly
[10:50] <fwereade> TheMue, should it?
[10:50] <TheMue> fwereade: OK, I'll take a look when you proposed it. ;)
[10:51] <TheMue> fwereade: No, it's only that both are interested in units, but in different contexts.
[10:51] <fwereade> TheMue, yeah, this is specifically unit *relation* presence
[10:51] <TheMue> fwereade: ic
[10:51] <fwereade> TheMue, (the better plan is to use the forthcoming presence.ChildrenWatcher)
[10:59] <fwereade> TheMue, https://codereview.appspot.com/6351046
[11:00] <fwereade> TheMue, should be trivial :)
[11:01] <TheMue> fwereade: LGTM, those pure red ones are pretty simple. ;)
[11:01] <fwereade> TheMue, cheers; agree ok to submit directly? :)
[11:02] <TheMue> fwereade: If the removed watcher isn't yet used then it's ok.
[11:02] <fwereade> TheMue, don't worry, I ran the tests :)
[11:02] <fwereade> TheMue, submitting now
[11:04] <fwereade> TheMue, btw, if you have another couple of moments
[11:04] <fwereade> TheMue, it seems to me that the watcher use in cmd/jujud is a bit inconsistent/idiosyncratic
[11:05] <fwereade> TheMue, I would appreciate your opinion on whether it's justified, or if we should clean it up
[11:05] <fwereade> TheMue, by which I mainly mean moving stopWatcher and mustErr into state/watcher, and using them consistently
[11:06] <TheMue> fwereade: i take a look
[11:07] <fwereade> TheMue, cheers
[11:07] <TheMue> fwereade: hmm, do you have a pointer for me?
[11:09] <fwereade> TheMue, cmd/jujud/machine.go:93
[11:09] <fwereade> TheMue, I'm pretty sure that it's wrong to let a tomb.ErrStillRunning slip through
[11:09] <fwereade> TheMue, and in line 80 we discard errors
[11:10] <fwereade> TheMue, I haven't looked at provisioner properly but I'm pretty sure it suffers similar problems
[11:12] <fwereade> TheMue, it may or may not be justifiable to rearrange NewMachiner such that it follows the usual model -- ie start the watcher before we start the loop
[11:13] <fwereade> TheMue, IMO it's a cleaner way of doing things, and is anyway better in the context of juju because it's more like the model we use throughout state
[11:13] <TheMue> fwereade: yes, this uncommon way may lead to misunderstandings
[11:13] <fwereade> TheMue, cool, I'll have a go at that; we can see what niemeyer thinks when he;s around
[11:14] <TheMue> fwereade: yes
[11:14] <TheMue> fwereade: lunchtime
[11:14] <fwereade> TheMue, cool, enjoy
[11:41] <Aram> hi.
[11:47] <fwereade> Aram, heyhey
[11:49] <fwereade> TheMue, it crosses my mind that *actually* the PA has it right wrt the watcher... there doesn't ever seem to be a good reason to create a subwatcher outside the loop
[11:50] <fwereade> TheMue, or even to keep it in a field on a, er, superwatcher
[11:51] <TheMue> re
[11:51] <TheMue> Aram: Hi
[11:52] <TheMue> fwereade: Maybe, it only puzzles me if something is different than in most/all other places.
[11:52] <fwereade> TheMue, I can't see anywhere a watcher field is used outside a loop method
[12:21] <TheMue> fwereade: Maybe it's my old OO as well as Erlang thinking that I do most initialization in a constructor and use the loop method only as loop.
[12:22] <fwereade> TheMue, IMO a field that's used in only one method should not really be a field ;)
[12:22] <fwereade> TheMue, anyway, I'm experimenting with what might be a noticeable simplification of the stuff in state/watcher.go, I'll let you know if it works out :)
[12:23] <TheMue> fwereade: But then I would not call the method loop(). I often name them backend() in my projects. ;)
[12:23] <fwereade> TheMue, I would personally gravitate towards run() :)
[12:24] <TheMue> fwereade: That's the job of the whole program: come on, run, Forrest ...
[12:24] <fwereade> TheMue, haha
[12:46] <fwereade> TheMue, making this change has made some of the state watchers start to look a bit inconsistent to me: an awful lot of them seem to me to be at risk of sending "changes" that don't actually represent changes
[12:47] <fwereade> TheMue, I rather consider that to be poor behaviour; opinion?
[12:49] <fwereade> TheMue, basically all of them seem to do what FlagWatcher did before I suggested changing it yesterday
[12:51] <fwereade> TheMue, excluding the Machine ones, that is
[12:51] <TheMue> fwereade: Hmm, maybe they should be fixed  then one-by-one in small changes.
[12:51] <fwereade> TheMue, yeah, that sounds sensible
[12:52] <TheMue> fwereade: We've done it this way for the error messages in state.
[12:52] <fwereade> TheMue, sorry, expand please
[12:52] <fwereade> TheMue, ah, how we make the changes
[12:53] <TheMue> fwereade: +1
[12:53] <TheMue> fwereade: yep
[12:53] <TheMue> fwereade: I think the should be fixed to work exactly as expeted.
[12:53] <fwereade> TheMue, cool
[12:54] <fwereade> TheMue, I need to think about what I've done a little more but it feels like a big win ATM
[12:54] <TheMue> fwereade: great
[12:54] <fwereade> TheMue, I'll see what niemeyer thinks before I start hanging more changes off it though ;)
[12:55] <TheMue> fwereade: I'm just doing the last changes for the service unit watcher. niemeyer had a very good hint how to test it more elegant.
[12:55] <fwereade> TheMue, yeah, I saw that
[12:55] <fwereade> TheMue, sounds good
[12:55] <TheMue> fwereade: it's so much clearer now.
[12:55] <fwereade> TheMue, sweet -- that style does make my head hurt a bit sometimes :)
[12:56] <TheMue> fwereade: it still uses test tables, but more simple ones
[12:57] <fwereade> TheMue, perfect
[12:57] <TheMue> fwereade: will now propose it and then start a new branch for the deleted to removed change
[12:57]  * fwereade worries that that change will be a real hassle to merge with his current work
[12:58] <fwereade> TheMue, can you wait a few mins? I'll propose what I have -wip so you can (1) tell me if it's sane and (2) decide whether the conflicts will be worth the hassle
[13:02] <TheMue> fwereade: Which change you mean, the first one or the deleted/removed one?
[13:02] <fwereade> TheMue, added/removed
[13:02] <fwereade> TheMue, (sorry, remind me what the first one is?)
[13:04] <TheMue> fwereade: The fixes after the review of the service units watcher.
[13:04] <TheMue> fwereade: They are proposed now.
[13:04] <fwereade> TheMue, I'm happy to suck up the conflicts on that one myself
[13:04] <TheMue> fwereade: The work on the deleted-to-be-named-removed will start now.
[13:05] <fwereade> TheMue, but Added/Removed might hurt
[13:05] <TheMue> fwereade: This is why I will do it in an extra branch. There are influences in more packages than just state.
[13:06] <fwereade> TheMue, true
[13:06] <TheMue> fwereade: But I'll wait for your proposal.
[13:08] <fwereade> TheMue, https://codereview.appspot.com/6347045
[13:09] <fwereade> TheMue, the reason I worry slightly is that it totally guts state/watcher.go
[13:10] <fwereade> TheMue, actually, looking at it, I doubt Added/Removed will hurt that much
[13:10] <fwereade> TheMue, but I'd still appreciate your preliminary opinion
[13:10] <fwereade> TheMue, the important bit is the changes to state/watcher.go, most of the rest can probably be a separate CL
[13:11] <TheMue> fwereade: In the moment the field is renamed in watcher.go any user of this field has to be renamed too.
[13:12] <TheMue> fwereade: So it may be better type by type instead file by file.
[13:12] <fwereade> TheMue, you mean the Added/Removed change?
[13:12] <fwereade> TheMue, my CL does not alter observable behaviour AFAICT
[13:12] <TheMue> fwereade: Yes
[13:12] <fwereade> TheMue, cool
[13:14] <TheMue> fwereade: The other one - don't raise events for unwanted content changes - is a different topic and should be discussed with niemeyer
[13:14] <fwereade> TheMue, yeah, I haven't even started on that
[13:14] <TheMue> fwereade: Maybe you should file it first.
[13:15] <fwereade> TheMue, hopefully I'll get a chance to see him and discuss it tonight
[13:15] <TheMue> fwereade: So which change of mine do you think could conflict yours?
[13:15] <fwereade> TheMue, I decided it probably wouldn't hurt too much in the end
[13:15] <fwereade> TheMue, there will be conflicts but they'll be trivial
[13:16] <fwereade> TheMue, feared they might be ugly before I looked at the actual diff
[13:17] <TheMue> fwereade: ah, ok, understand
[13:17] <fwereade> TheMue, anyway, does that change look worth it to you?
[13:18] <TheMue> fwereade: i'm not yet through, one moment
[13:20] <fwereade> TheMue, MW and MUW are not finished there
[13:20] <fwereade> TheMue, you should be able to ignore them and still have a clear idea of what's involved
[13:27] <TheMue> fwereade: Mostly LGTM, I only have troubles with the panic() in MustErr().
[13:27] <fwereade> TheMue, that was already there :)
[13:28] <TheMue> fwereade: Maybe, but not by me. ;)
[13:28] <fwereade> TheMue, the idea is that if a subwatcher's channel is closed while the watcher is reading from it, it indicates a fundamental logic error
[13:28] <TheMue> fwereade: The question is: Is this situation simply an error itself and recoverable or do we really need to tear down the program?
[13:29] <fwereade> TheMue, it means that we, as coders, are demonstrably on crack
[13:29] <fwereade> TheMue, ie not recoverable IMO
[13:29] <TheMue> fwereade: If it can't happen by any runtime situation then I'm fine with hit.
[13:29] <TheMue> it
[13:29] <fwereade> TheMue, that's the idea
[13:33] <TheMue> fwereade: This is what I really like in Erlang. Here a supervisor would recognize this crash and start the needed tasks to recover the system (which sometimes is only the restart and registration of a coroutine (process) and sometimes may be the stopping and restarting of large process trees).
[13:34] <fwereade> TheMue, but if the error is "the programmers are demonstrably failing basic logic" then I think it's better to fail hard AAP
[13:34] <TheMue> fwereade: supervisors can also supervise other ones, so it cascades if one fails during restart
[13:34] <fwereade> ASAP
[13:35] <TheMue> fwereade: yes, indeed, if it is definitely not recoverable and a logical error
[13:36] <fwereade> TheMue, if a watcher has closed its change channel while its owner is still making use of it, that is IMO evidence of fundamental crackfulness
[14:37] <mthaddon> hey folks, does anyone know who's responsible for maintaining http://jujucharms.com/ ?
[14:42] <james_w> mthaddon, that's hazmat's
[14:43] <mthaddon> james_w: ah, thx