niemeyerdavecheney: Will do00:10
davecheneyniemeyer: ty00:15
niemeyerdavecheney: The NewConfig branch seems polluted by an undeclared pre-req00:16
niemeyerdavecheney: https://codereview.appspot.com/6209092/diff/10001/environs/ec2/ec2.go?column_width=8000:16
davecheneyyeah, only open.go and open_test.go00:16
davecheneyI will resubmit it once SetConfig is landed00:16
niemeyerdavecheney: Ok, it'd be nice to get the diff fixed00:16
davecheneyif you coudl take a cursory glance at environs/open{,_test}.go00:17
davecheneythat is the only part changed00:17
niemeyerdavecheney: It looks good00:17
davecheneyits not a big change, just exposing the EnvironConfig before it gets passed to Open00:18
niemeyerdavecheney: Yeah, it looks fine if that's all, but it'd be nice to clean up the diff before getting it in00:18
davecheneyi will certainly do that00:18
niemeyerdavecheney: Cool, should be a trivial LGTM after that00:21
twobottuxaujuju: Creating volume group in nova-volume Juju charm <http://askubuntu.com/questions/141552/creating-volume-group-in-nova-volume-juju-charm>07:55
fwereadeheya TheMue09:44
rogpeppeTheMue, fwereade: yo!09:49
TheMuerogpeppe, fwereade: Heya09:51
fwereaderogpeppe, heyhey09:51
rogpeppefwereade: i bet this weather is just what you're used to... were you really pining for a bit of old-fashioned english drizzle? :-)09:51
fwereaderogpeppe, well, it *is* really nice for about 10 minutes09:52
* rogpeppe likes a bit of rain too09:52
rogpeppebut not for months on end :-)09:52
fwereaderogpeppe, after that I start to congratulate myself for moving ;)09:52
* TheMue sits again on the veranda, right now 27°C and wonderful sun.09:55
=== Guest__ is now known as smaddock
TheMueSo, short break, have to bring daughter to work.10:20
fwereadehey ra10:24
fwereadehey Aram10:24
* davecheney waves to rogpeppe 11:25
* rogpeppe waves to davecheney11:25
* rogpeppe waves to davechen1y11:25
davecheneyso, how would you like to manage this merge ?11:25
rogpeppedavecheney: good question. if the ImageSpec->instanceSpec stuff can go in today, it would be quite nice if i could do them both at once.11:26
davecheneyhow is imagespec looking ?11:26
rogpeppedavecheney: i've spent... too much time pissing around with splitting up branches and merging and remerging11:26
davecheneyf'yeah, i'm over reqs11:26
rogpeppedavecheney: it should be good to go, but it needs gustavo's go ahead because i'm not quite sure what he wants in the doc comments.11:27
davecheneythat is cool11:27
rogpeppedavecheney: it should be less important now that i've unexported the types.11:27
davecheneyi'm happy to wait for those two to go in11:27
davecheneythen I'll merge and adjust as needed11:27
davecheneygostavo has LGTM in principal the various bits11:27
rogpeppedavecheney: thanks.11:27
davecheneyi'm sad that the proxy idea didn't get up11:27
davecheneybut at the end of it I couldn't offer a strong enough argument over Environ.SetConfig11:28
rogpeppedavecheney: i think it made sense. but i suppose it only makes sense when the provider has no state11:28
davecheneypossibly if there was going to be more than one PA11:28
davecheneyonce we have somethig working11:28
davecheneyi'll hvae another crack at introducing the proxy idea11:29
davecheneyas a way of seperating concerns11:29
rogpeppedavecheney: i quite liked the "return a proxy within ec2" idea11:29
davecheneyI was thinking proxy.(*proxyEnviron).SetConfig(config)11:29
davecheneywhich would reload it on command11:29
davecheneywhich might resolve gustavo's concerns about control11:30
davecheneyfrom my vantage point, the Environs themselves contain almost no state11:30
rogpeppedavecheney: i was thinking type Proxy struct {env environs.Environ}11:30
davecheneyor even11:30
davecheneytype Proxy struct { environs.Environ }11:31
rogpeppedavecheney: and then environs.config.Open returns Proxy{ec2environ}11:31
davecheneybut that has visilibity problems11:31
davecheneyanyway, i'll wait til we have something that works11:31
rogpeppedavecheney: yeah, i think you want to be sure that all methods are reimplemented11:31
davecheneyrather than spinning on this11:31
rogpeppedavecheney: yeah, the current thing will work.11:32
davecheneyfrom my POV I need some commands to do some functional testing on the PA11:33
davecheneybut I was wondering how jujuc is coming along ?11:33
rogpeppedavecheney: it needs more stuff.11:33
davecheneyfrom a quick look at the supercommand stuff11:34
rogpeppedavecheney: i'm sure fwereade will be cranking out more commands soon though11:34
davecheneyi can add extra commands to the framework pretty simply11:34
rogpeppedavecheney: by "functional testing" you mean ad-hoc testing?11:34
davecheneyI basically need to invoke, addmachine and remove machine11:34
davecheneyin another window11:34
rogpeppedavecheney: can't you write tests that use the State methods to do that?11:34
davecheneyi also need to figure out how to hook the provisioning command up to StateTest so I can do unit tests against dummy with it11:35
rogpeppedavecheney: i tend not to do any ad-hoc testing, but maybe that's wrong. that was always the only method i ever used, in previous lives...11:35
davecheneyi've found there is value in doing both11:36
davecheneyfor example I wrote a quick SOCKS over ssh proxy11:36
davecheneythen left that running on the open internet for a day to load test the ssh client code11:36
rogpeppedavecheney: these days i tend to put the effort i would spend on thinking of ad hoc tests into thinking of reusable tests11:36
rogpeppedavecheney: there's definitely room for functional testing, but even then i think there's value in scripting it.11:37
rogpeppedavecheney: and until we have a framework for that, maybe we should just spend time writing unit tests as good as we can think of.11:37
davecheneyof course, hence the need for simple cli tools to push the state around so I can watch the PA recact to it11:38
rogpeppedavecheney: i think i'd just hook them both up to the same zk and use the state methods and check that the pa reacts in the correct way.11:39
rogpeppedavecheney: perhaps using the dummy provider so you can read actions as they take place.11:39
davecheneyyup, i'm not sure how much of that harness exists atm11:39
davecheneyi know environs/jujutest does some of it11:40
rogpeppedavecheney: cmd/juju does a certain amount of it.11:40
rogpeppedavecheney: really, i think it would be nice if the dummy provider could be configured to start zk11:40
rogpeppedavecheney: then we get an almost-complete environment for testing.11:41
davecheneyi'll look into that11:41
davecheneyi don't think it does that at the moment11:41
rogpeppedavecheney: it doesn't11:41
rogpeppedavecheney: it shouldn't be hard though, particular with the recent refactoring of it i've done.11:41
davecheneyin fact I think it works the opposite, it waits for the juju/testing to make a zk11:41
davecheneyi'll look at it tomorrow11:41
davecheneyrogpeppe: can you chain req branches ?11:43
rogpeppedavecheney: yeah11:43
davecheneyie propose a, propose b -req a, propose c -req b11:44
rogpeppedavecheney: although it's a hassle, because once you've changed the base, you have to remerge all the subsequent branches in the chain11:44
davecheneyyes, that doth blow11:44
rogpeppedavecheney: but tbh, it's not as bad as it was. you really only need to remerge when you come to actually submit the next branch.11:45
davecheneyi'm looking forward to phase of th eproject when we've got something we can itterate on11:45
rogpeppedavecheney: definitely.11:45
davecheneywhen I was building the platform at atlassian we got a prototype working very quickly11:45
rogpeppedavecheney: i'm just about to propose a branch that gets the go tools and runs them on the server side, BTW11:45
davecheneybut then there was a big gap of nearly 2 months before we got the next version working11:46
rogpeppedavecheney: we've already got a prototype - the python version...11:46
davecheneyrogpeppe: nice11:46
davecheneyrogpeppe: the first version of the atlassian platform was three desktops that we built by hand11:46
davecheneybut the next version took a long time because we decided to build all the automation first11:46
davecheneybut, once it was running itteration was very fast11:47
rogpeppedavecheney: i think that the Go version will be nice to iterate on when we've got it up.11:47
rogpeppedavecheney: i'm quite happy with how the overall structure is turning out11:47
rogpeppedavecheney: slight misgivings about the whole necessity of the State abstraction, as you pointed out, but i'm ok with it.11:48
davecheneyrogpeppe: tell me more11:49
rogpeppedavecheney: i think it was you that suggested that all of state could be represented more compactly by a simple parent/child/attribute abstraction.11:50
davecheneyI was thinking that I would like it if I could as the topology for a watcher, rather than the way watchers are quite chummy with the zk nodes themselves11:51
rogpeppes/as the/ask the/ ?11:51
davecheneyfor the particular example of wtching machines11:52
davecheneyI want the ability to subscribe to a part of the toploogy11:52
davecheneywhat we have now works fine11:52
rogpeppedavecheney: +111:52
davecheneywatch /topology, then parse the full thing11:52
davecheneybut given that the State already has the whole topology as a data structure11:53
davecheneyit would make sense that it could push data to any watcher directly11:53
davecheneybut I wonder if gustavo has plans for the topology11:53
davecheneyas it is a crutch for the limitations of zk11:53
rogpeppedavecheney: i think the eventual idea is that we push all of state behind a server API11:53
davecheneyat which point all the watchers are going to be pub/sub kind of things11:54
rogpeppedavecheney: then we don't have to worry about atomic changes to the topology because the server can mediate that11:54
rogpeppedavecheney: exactly11:54
davecheneyspeaking of Environs11:54
rogpeppedavecheney: it's interesting to think about what level we might want the server API11:55
davecheneyI was going to try a branch that removes the requirement to pass state.Info to Environ.StartInstance11:55
rogpeppedavecheney: how would the new instance know how to connect to the state then?11:55
davecheneyrogpeppe: i hope it looks like a URL tree11:55
davecheneypretty much the same as the zk topology11:55
davecheneybut with REST calls that you can subscribe to changes to a path and it's children, etc11:55
rogpeppedavecheney: yeah, but what level of abstraction are the URLs?11:55
davecheneywithout giving it any thought11:56
rogpeppedavecheney: i dunno. i was wondering about something higher level.11:56
davecheneyi think the URL's should look like the current ZK path11:56
davecheneybut that is without giving it any serious consideration11:56
rogpeppedavecheney: i don't think so, actually. i think that constrains the possibilities to db optimisation.11:56
rogpeppes/to db/for db/11:56
rogpeppedavecheney: higher level API gives more scope for changing the impl behind the scenes11:57
rogpeppedavecheney: anyway... back to ground level11:57
rogpeppedavecheney: how does StartInstance work without having a StateInfo?11:57
davecheneythere is a StateInfo method on the Environ itself11:58
davecheneyergo, it already knows it's state11:58
davecheneyerr, state.Info11:58
rogpeppedavecheney: not necessarily. i wanted to keep open the possibility that one environ might connect to a state from elsewhere.11:58
rogpeppedavecheney: there's no essential connection between a state and a given environ.11:58
rogpeppedavecheney: for instance, at some point in the future, we want to have the possibility of several environments using the same "bootstrap" node.11:59
fwereaderogpeppe, hmm, my reading of that had been that they'd still be separate states despite all happening to coexist within the same zookeeper (there's a chrooty thing for ZK, right?)12:00
fwereaderogpeppe, it feels to me like a state is pretty strongly associated with an environ12:00
davecheneyfwereade: i agree12:01
davecheneyso either Environ gets closely bound to a state.Info, or is completely abstracted from it12:01
davecheneyeither way, the requirement to pass state.Info in StartInstance will go away12:01
fwereaderogpeppe, davecheney: yeah, that param definitely feels wrong to me12:02
davecheneyfwereade: rogpeppe i'm guessing it's there to pass details to the machine agent on how it can find the ZK and bootstrap itself so to speak ?12:02
rogpeppedavecheney: yes, at least that. i'm trying to remember the other scenarios.12:03
davecheneyor to say it another way, how it can build and Environ that matches that of the Provisioning Agent that created it12:03
fwereadedavecheney, rogpeppe: sorry, I'm confused... doesn't the environ already know its own state info?12:03
davecheneyfwereade: yes, it must, because there is this method12:03
rogpeppefwereade: only if you've bootstrapped it.12:04
davecheneyrogpeppe: i'm happy with that restriction, the PA doesnt' run til bootstrapping is complete, unless i'm mistaken12:04
rogpeppefwereade: but you could reasonably have an environ that you hadn't bootstrapped, but were using with a provisioning agent and a state from somewhere else, i think.12:04
fwereaderogpeppe, ok, but if you haven't bootstrapped it you don't have a state: you just know that there *will* be one on localhost once bootstrap is complete12:04
davecheneyrogpeppe: ah, i see what i'm missing12:05
davecheneybootstrapping uses Environ.StartInstance to start machine(0)12:05
rogpeppedavecheney: actually, it uses startInstance; slightly different.12:06
davecheneystill, possibly a strategic nil could be passed, suggesting, use whatever state you have internally12:06
rogpeppepart of the problem is we don't want to pass the ec2 secret key in the cloudinit script12:07
fwereadedavecheney, rogpeppe: it feels to me like bootstrap is the special case, and it's misleading to expose those details to the outside world12:07
davecheneyi concur12:07
rogpeppehmm, i think it feels quite clean actually. we explicitly tell the new machine how to connect to the juju state.12:08
rogpeppeit's always going to need to do that, i think.12:08
fwereaderogpeppe, but the environ is so closely tied to the state that it feels wrong to have to tell it again12:08
rogpeppefwereade: i don't see the coupling as that close actually.12:09
fwereaderogpeppe, Environ.StateInfo implies otherwise, to me12:10
rogpeppefwereade: i like the idea of being able to have the machines in a single environ controlled by multiple states, for example.12:10
fwereaderogpeppe, sure, that has potential: but I'm pretty sure that's not what we're implementing right now :p12:11
fwereaderogpeppe, would you say that Environ.StateInfo was a misstep that we should remove?12:11
davecheneyfwereade: rogpeppe it appears to me that if the environ has a StateInfo method, then it knows it's own state, so StartInstance shuldn't need to be told a stateinfo12:11
fwereadedavecheney, exactly so12:12
davecheneyon the other hand, if the envion shuldn't need to know about the stae, then the StateInfo method is misleading12:12
rogpeppedavecheney: no, i don't think an environ does know its own state12:12
davecheneyas it stands, it's not a big deal to do si, _ := environ.StateInfo(); environ.StartInstance(99, si)12:12
rogpeppedavecheney: the private keys are not accessible to an environ for example12:12
davecheneyrogpeppe: then what is returned from environ.StateInfo() is not actually useful then ?12:13
davecheneyoh, hang on12:13
davecheneyi'm talking about stateInfo, which is just a pretty connection string to find zookeepers12:13
fwereadedavecheney, I must have missed something; how does that change things?12:15
davecheneyfwereade: 22:12 < rogpeppe> davecheney: the private keys are not accessible to an environ for example12:16
* rogpeppe is thinking12:16
davecheney^ i interprited that to mean more than it did12:16
fwereadedavecheney, ah, cool, thanks12:16
rogpeppedavecheney: yeah, and i'm not sure i meant what i said.12:16
davecheneyfwereade: rogpeppe i'm sure this is over simplifying this, but for the privisioning, machine and unit agents12:17
davecheneyto get up and running12:17
davecheneyall then need to know is the argument they are passed to jujud, and a stateinfo, which is really just a string passed to --zookeeper-servers12:17
fwereadedavecheney, yes, essentially12:18
davecheneyand so, Environ.StartInstance() is shorthand for 'start a new machine, and run this daemon passing these arguments'12:18
fwereadedavecheney, the PA also needs to know the environ, and the UA/MA need to know what U/M they's acting for12:18
davecheneyyeah, there is clearly more too it than I just glossed over12:18
davecheneybut from the point of view of 'how do I find zookeeper so I can watch stuff'12:19
rogpeppedavecheney: one issue is the possibility that we need a different internal vs external IP address. i'm not *sure* that the environ is in a good place to know which to use.12:19
fwereadedavecheney, sure, understood12:20
* fwereade thinks12:20
davecheneyrogpeppe: for the short term (read ec2 and workalikes) I think we can use the assumption that all machiens in the same environment areusing the same network12:20
rogpeppedavecheney: what about the juju client?12:20
fwereadedavecheney, it still seems to me that the fact that there's a different ZK address is an incredibly minor aspect of the things that are different when starting the first node12:21
davecheneyrogpeppe: good point, i don't have an answer to that, other than saying we probably need two state.Info's12:21
davecheneyexternal and internal12:21
davecheneyor something that clearly identifies external (as in not inside the environment) and internal (that are)12:21
davecheneyotherwise we'll go mad with the permutations12:21
rogpeppedavecheney: how does a machine know if it's internal or external?12:22
davecheneyjujuc == external, jujud == internal12:22
davecheneyas a start12:22
fwereadedavecheney, so it feels wrong to push that one specific difference up to that level while leaving the others implicit12:22
fwereadedavecheney, wait, what?12:22
davecheneybut trying to make one connectino string serve two masters sounds worse12:23
davecheneyrogpeppe: as I understand it, all machines are internal; the are inside the environment12:23
rogpeppecurrently StateInfo returns the external address.12:23
davecheneyif ec2 lets that work, then it's a solved problem for now12:23
rogpeppedavecheney: the difficulty is that Environ is used by the client as well as the cloud machines12:23
rogpeppedavecheney: i think the connection methods are different too. i'm not sure we use ssh between cloud machines.12:24
davecheneyahh, so the reason StartInstance takes a state.Info is to allow us to construct an 'internal' connection string12:25
rogpeppedavecheney: that's certainly one reason, yes.12:25
fwereadedavecheney, rogpeppe: hmm... except when bootstrapping, the state info comes from an Instance, right?12:26
fwereadedavecheney, rogpeppe: which already knows both internal and external addresses12:27
rogpeppefwereade: no, it comes from the private storage12:27
fwereaderogpeppe, how did it get there?12:27
rogpeppefwereade: it was put there at bootstrap time.12:27
fwereaderogpeppe, if not from the Instance constructed on bootstrap12:27
rogpeppefwereade: at that time we don't know the DNS addresses of the instance.12:27
davecheneyrogpeppe: fwereade reviewing state/open.go, State info has a slice of addresses to connect to, is that what you mean by internal and external ?12:28
fwereaderogpeppe, wait, you just said the information was put in provider storage at bootstrap time12:28
rogpeppefwereade: the instance id only12:28
fwereadedavecheney, no, the slice is all ZKs; internal/external is just about what address we use to connect to a ZK or group thereof12:29
rogpeppeso... if i understand the general idea, we could have ...12:29
fwereaderogpeppe, ok, and the instance id is used at some point to get an Instance, from which we get the public address and discard the private?12:29
rogpeppeone mo, tel call12:30
fwereaderogpeppe, ...and we're now discussing how we go about reacquiring the private address?12:30
rogpeppefwereade: yeah12:30
fwereaderogpeppe, it seems to me that the trouble is in throwing away half the information in the first place12:30
rogpeppeso... if i understand the general idea, we could have a "local" argument to StateInfo which determines which info to get.12:31
rogpeppe'cos we can't get rid of StateInfo entirely, right?12:31
rogpeppeso we're suggesting that StartInstance call StateInfo(local) inside itself, rather than taking an explicit state.Info arg?12:32
fwereaderogpeppe, perhaps something like that: I haven't thought it through entirely12:33
davecheneyrogpeppe: i'd like to try that12:33
fwereaderogpeppe, but it feels like a potentially fruitful avenue12:33
rogpeppei don't really see the point. i don't think it'll make anything significantly simpler12:33
davecheneyor at least have StartInstance(314, nil) as a sentinal for 'pass whatever state.Info you know/makes sense/sounds best'12:33
davecheneyrogpeppe: as it sounds like the reason StartInstance takes a state.Info is to facilitate bootstrapping12:34
fwereadedavecheney, doesn't StartInstance imply non-bootstrap in the first place?12:34
* davecheney shrugs12:34
rogpeppefwereade: yes12:35
fwereadedavecheney, rogpeppe: I'm not bothered what we pass to startInstance, but I'm concerned about dirtying up StartInstance when in practice it will *always* be called with nil... won;t it?12:35
fwereadedavecheney, rogpeppe: ... or  .StateInfo12:35
fwereadedavecheney, rogpeppe: depending on how we do the details12:35
rogpeppefwereade: it means that StartInstance will have to read the provider local storage each time, i think.12:36
davecheneyfwereade: or to put it another way, if the PA has to call StartInstance when it sees a machine appear in the topology, what is it supposed to use for that argument12:36
davecheneyat this moment the only thing it can use is the value from a.Conf.StateInfo12:36
davecheneywhich comes from the jujud super command framework12:37
fwereadedavecheney, rogpeppe: how does the same argument not apply to Environ.StateInfo?12:37
davecheneyfwereade: that is an intersting one12:37
rogpeppefwereade: if the caller already has a StateInfo, it can pass that to StartInstance each time.12:37
davecheneythe PA is connected to a state.State12:37
davecheneyit find a *ConfigNode by watching /enviornment, and passes that to environs.NewEnviron12:38
davecheneyand gets an Environ12:38
rogpeppeif at some point we want to be able to adjust the set of database machines at runtime, it will be good to be able to do that through the existing state.12:38
davecheneywhat that environ.StateInfo() returns is a damn good question12:38
rogpeppedavecheney: the bucket will be configured in the attributes it passes to SetConfig12:39
rogpeppedavecheney: and that's what StateInfo will use to return the state.Info12:39
fwereaderogpeppe, we surely will at some stage... in which case surely the right answer *is* to read it every time12:39
fwereaderogpeppe, but this seems essentially orthogonal to the question of whether it needs to be passed to StartInstance12:40
rogpeppefwereade: i'm not sure. i think the provider storage is good for bootstrap, but we can use other means when we're up and running.12:40
rogpeppefwereade: bootstrap and client connections, of course.12:41
rogpeppefwereade: there's no way of *watching* provider storage, i think12:41
fwereaderogpeppe, I thought you said bootstrap didn't use StartInstance12:41
fwereaderogpeppe, davecheney: oh hell, EOD, got to go to the bank :(12:42
rogpeppeBTW the provider state is written *after* the bootstrap node is successfully started.12:42
fwereaderogpeppe, davecheney: hope I wasn't just muddying the waters for you12:42
rogpeppefwereade: no, it's all good stuff12:42
fwereaderogpeppe, I know; don't get current significance though12:42
rogpeppefwereade: it means that the bootstrap node doesn't necessarily have access to the provider state when it starts.12:43
rogpeppeof course in practice with ec2, it's so slow that it will in fact.12:43
fwereaderogpeppe, but we bake the stateinfo into cloud-init regardless12:44
rogpeppefwereade: we can't do that if we can't pass it in to StartInstance12:44
rogpeppefwereade: because StartInstance will, of necessity, retrieve the state info from the provider storage bucket12:45
fwereaderogpeppe, but the environ *knows* it's bootstrapping, and it fully controls the cloud-init, and it *knows* it's starting a ZK and relevant agents on that instance, and hence it *knows* that it can use localhost12:45
fwereaderogpeppe, and did I misunderstand about StartInstance not actually being used to bootstrap anyway?12:45
rogpeppefwereade: i'm not sure that the Environ knows that it's on a bootstrap node.12:46
davecheneyif I can interrupt for a moment12:46
rogpeppedavecheney: please do12:46
* fwereade listens12:47
davecheneyis the reason we're saying 'we can't just let the Environ pass what it' thinks is it's state info to any new machine via StartInstance' is because it might have 'localhost' as its zk host ?12:47
rogpeppedavecheney: that's not the only reason.12:47
rogpeppedavecheney: the reason we were just talking about was that there's more than one source for a StateInfo12:47
rogpeppedavecheney: it might come from the provider storage; or it might come from being passed into the cloudinit script.12:48
davecheneyrogpeppe: but from the POV of the PA, there is only one stateinfo12:48
davecheneytheir are many, but this one belongs to the PA12:48
rogpeppedavecheney: yeah, the one that's passed on the command line12:48
fwereadedavecheney, rogpeppe: I do actually have to go now; ttyl :)12:48
rogpeppefwereade: k12:48
rogpeppefwereade: c u tomorrow12:48
* rogpeppe also has to go to lunch soon12:49
davecheneyfair enough, lets wrap up12:49
davecheneyfrom what i've read today, the best course of action is to not change naything12:49
davecheneythe PA calls environ.StartInstance(999, a.Conf.StateInfo)12:49
davecheneywe'll revisit it later, if necessary12:50
rogpeppedavecheney: i *think* that's good. i'm open to the idea of a change, but in this case i'm not sure it helps much.12:50
rogpeppedavecheney: passing in StateInfo explicitly means the environ doesn't need to care how we got it.12:51
rogpeppedavecheney: which i *think* is a reasonable separation of concerns. but please keep on considering alternatives!12:52
rogpepperight, lunch!12:52
rogpeppedavecheney: see you tomorrow, i hope, if i get up early enough.12:53
davecheneyrogpeppe: kk, i'll catch you on the next rotation13:09
andrewsmedinarogpeppe: hi13:54
andrewsmedinarogpeppe: I sent another review13:55
rogpeppeandrewsmedina: thanks. i'll take a look.13:58
* niemeyer waves14:03
rogpeppeniemeyer: yo!14:03
rogpeppeandrewsmedina: LGTM14:07
andrewsmedinarogpeppe: thanks \o/14:07
rogpeppeniemeyer: i updated https://codereview.appspot.com/6188068/ to unexport the types and change the docs. hopefully should be better now.14:08
niemeyerrogpeppe: Cool, thanks14:09
niemeyerrogpeppe: Agreed regarding it going out of the ec2 package eventually14:09
niemeyerrogpeppe: We don't have to do that now, but let's just watch out to lead the code towards making them sensible there (it's already going in that direction it seems)14:09
rogpeppeniemeyer: will do.14:10
TheMueniemeyer: Moin and thx for the reviews.14:14
niemeyerrogpeppe: It actually feels like going backwards a bit14:14
rogpeppeniemeyer: oh?14:15
niemeyerrogpeppe: All those field name changes will have to go back again in the next branch that does the actual switch over14:15
niemeyerrogpeppe: The way we have colseries and colServer also feels quite awkward14:15
niemeyerrogpeppe: These names should be internally consistent14:15
rogpeppeniemeyer: i'm not sure. it's only the instance contraint that will have to change14:16
rogpeppeniemeyer: colseries is my mistake - a global replace too enthusiastic!14:16
niemeyerrogpeppe: Ah, phew14:16
niemeyerTheMue: Heya!14:16
niemeyerTheMue: np14:16
rogpeppeniemeyer: i think that instanceSpec will remain private14:16
rogpeppeniemeyer: and instanceConstraint will be significantly different14:16
niemeyerrogpeppe: I'm tempted to suggest moving the InstanceConstraints to environs right now14:17
rogpeppeniemeyer: so i don't mind it being private now14:17
niemeyerrogpeppe: If it will be significantly different, we're doing it wrong14:17
rogpeppeniemeyer: can we leave it as it is - i've got three upstream branches that i'm juggling on top of this one14:17
rogpeppeniemeyer: then i can do the constraints stuff later14:17
niemeyerrogpeppe: Yep, sure14:18
niemeyerrogpeppe: Done14:20
rogpeppeniemeyer: thanks a lot14:20
niemeyerrogpeppe: np14:20
rogpeppeniemeyer: remind me again how i can get the full revision id for the current branch out of bzr, please.14:46
rogpeppeniemeyer: my google-fu has deserted me14:46
niemeyerrogpeppe: revision-info should do14:46
rogpeppeniemeyer: i was trying revno and info14:47
niemeyerrogpeppe: revision-info --tree is relevant in some cases14:47
rogpepperevno -v should do it, i reckon14:47
smaddockbcsaller: ping14:51
twobottuxaujuju: Juju error: Server refused to accept client <http://askubuntu.com/questions/141687/juju-error-server-refused-to-accept-client>14:52
niemeyerLunch, biab15:12
niemeyerTheMue: ping16:08
TheMueniemeyer: pong16:08
niemeyerTheMue: yo16:08
niemeyerTheMue: Would you mind if we at least delayed the addition of that method?16:08
niemeyerTheMue: It feels like an awkward interface to me.. I can't see what this method is offering that we can't do with the other methods we have16:09
TheMueniemeyer: So far I've got no problem. The original is used two or three times (beside the tests).16:09
niemeyerTheMue: So it'd be awesome if we could keep that out while we figure the answer for that16:09
TheMueniemeyer: So I'll see when I reach this point.16:09
niemeyerTheMue: I'm sure it's used, but I think we have a better interface that may enable its original use case withou tit16:10
andrewsmedinaniemeyer: https://codereview.appspot.com/6099051/16:10
niemeyerandrewsmedina: Neat, will give it a final pass16:10
TheMueniemeyer: OK, so I'll kick it with the next proposal.16:10
niemeyerTheMue: Cheers, please ping me once you're done.. I suspect it's ready for going in, and will check as soon as you push16:11
niemeyerandrewsmedina: Why network.Subnet has no xml tag?16:11
andrewsmedinaniemeyer: its only used to create a new net16:14
andrewsmedinaniemeyer: this attribute is not returned in the xml generated by virsh16:16
niemeyerandrewsmedina: Cool16:19
niemeyerandrewsmedina: The tests will fail without root, right?16:22
andrewsmedinaniemeyer: wrong16:29
niemeyerandrewsmedina: Ah, you're faking the whole thing.. hmmm16:29
niemeyerandrewsmedina: That's both nice, and not'''16:29
andrewsmedinaniemeyer: yes...16:30
niemeyerandrewsmedina: Either way, we can move forward like that and address it later16:30
niemeyerandrewsmedina: It's better then the opposite I guess16:30
andrewsmedinaniemeyer: but it was the only way that I found not to change the network configuration of the machine on which the tests have been run16:31
niemeyerandrewsmedina: Yeah, that's what I mean.. your approach is better than being *just* on the other side of the fence16:32
niemeyerandrewsmedina: At some point in the future, once that's all working well, we can add some extra tests to *optionally* guarantee that it works for real16:32
niemeyerandrewsmedina: Or maybe not.. external functional tests would deal with that too16:32
niemeyerandrewsmedina: Either way, you have a review with a few trivialities, and LGTM16:32
niemeyerandrewsmedina: Please ping me once these are done and I'll submit16:33
niemeyerandrewsmedina: (rog had a trivial too)16:33
andrewsmedinaniemeyer: ok16:33
andrewsmedinaniemeyer: what trivial rog had?16:33
niemeyerandrewsmedina: It's in the CL16:33
niemeyerandrewsmedina: https://codereview.appspot.com/6099051/diff/15001/environs/local/network.go#newcode9216:34
niemeyerandrewsmedina: Also part of the LGTM mail he sent16:34
andrewsmedinaniemeyer: I will work on these issues16:36
TheMueniemeyer: ping, propose is in.16:50
* hazmat lunches16:59
rogpeppeniemeyer: this should be trivial: https://codereview.appspot.com/6257043/17:00
niemeyerTheMue: Cheers17:05
niemeyerrogpeppe: Cool17:05
rogpeppeniemeyer: as should this: https://codereview.appspot.com/6249049/17:06
rogpeppeniemeyer: and the one after that is the important one... just doing final amazon test now before proposing17:07
niemeyerrogpeppe: LGTM on first17:08
rogpeppeniemeyer: thanks a lot17:08
niemeyerrogpeppe: LGTM on second too, thanks17:09
niemeyerTheMue: Going through it now17:09
rogpeppeniemeyer: lovely jubbly, ta17:09
TheMueniemeyer: great, thx17:09
rogpeppeniemeyer: here's the boy: https://codereview.appspot.com/624504817:23
rogpeppeniemeyer: i tried to split it up more, but it wasn't having any of it (too much interdependency)17:23
rogpeppeniemeyer: if it's any consolation, the code ends up 200 lines *shorter* after this CL...17:23
rogpeppeniemeyer: gotta go now.17:24
niemeyerrogpeppe: Super, thanks a lot17:24
rogpeppeniemeyer: i'll be quite excited if this goes in17:24
rogpeppeniemeyer: see ya tomorrow17:25
niemeyerrogpeppe: Well it will certainly go in17:25
niemeyerrogpeppe: Cheers17:25
rogpeppeniemeyer: well, you might say it's too big or just wrong or something :-)17:26
niemeyerrogpeppe: I can't anticipate that, but the concept of the feature we want for sure17:26
niemeyerTheMue: Sorry, ended up sidelined by other conversations.. back to the review17:42
TheMueniemeyer: np17:42
niemeyerTheMue: "It's allowed today in endpoint.py. Maybe it's an error there too?"17:43
niemeyerTheMue: I believe it is.. can you please drop it so that we can be sure about what's going in?  If we find issues, it'll be a nice insight17:43
TheMueniemeyer: Ok, will be dropped.17:44
niemeyerTheMue: "// /topology node in ZooKeeper.s"17:46
niemeyerTheMue: Extra "s" at the end, topology.go line 4617:46
niemeyerTheMue: LGTM with these sorted. Thank you!17:47
niemeyerTheMue: Review delivered for the record too17:47
TheMueniemeyer: Thanks. The s may be from saving without ctrl. ;)17:49
niemeyerTheMue: Hehe, I know how these go :)17:55
TheMueniemeyer: Sh.., had to leave computer, "accident" here. My wife lost her earring into the plughole. *sigh*18:02
niemeyerTheMue: Wow, what/18:06
niemeyerTheMue: Oh, ok18:06
niemeyerTheMue: Did you pick it up?18:07
niemeyerrobbiew: Is it meeting time?18:36
robbiewyesss!...one sec18:37
robbiewg+ invite sent18:38
TheMueniemeyer: So, final propose before LGTM. And the earring has to wait, I had to bring my daughter to before. Tasks when the kids grow. ;)18:58
niemeyerTheMue: Cool :)18:59
niemeyerTheMue: With robbiew, will check it right afterwards18:59
TheMueniemeyer: Fine, so I can submit it later or as my first task tomorrow morning.19:05
niemeyerTheMue: LGTM19:08
TheMueniemeyer: Great, thx. A good step for the end of the day.19:09
niemeyerTheMue: +1, very happy to see this going n19:09
TheMueniemeyer: Me too.19:10
* hazmat likes the new golang book20:57
hazmatthe mark summerfield one20:58
niemeyerhazmat: Good to hear21:30
niemeyerhazmat: Haven't checked it out yet21:30
hazmatniemeyer, i've read some of his others pyqt.. etc.. he's a good author21:34
niemeyerdavecheney: Good morning21:41
niemeyerhazmat: Yeah, haven't read those, but I did see his name around books for a while21:41
davecheneyniemeyer: morning21:42
niemeyerdavecheney: Had to put https://codereview.appspot.com/6229046/ back to WIP unfortunately.. a race was introduced in the latest change21:42
davecheneyi'll address that now21:42
niemeyerI'm heading off to dinner.. will be back later to review rog's latest goodness21:55
robbiewdavecheney: ping23:05
andrewsmedinaniemeyer: I sent the new proposal23:07
davecheneyrobbiew: ack23:08
davecheneyi should be on skype23:08
robbiewdavecheney: cool...one sec23:10
davecheneyniemeyer: out for review: https://codereview.appspot.com/624405323:27

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