[00:10] davecheney: Will do [00:15] niemeyer: ty [00:16] davecheney: The NewConfig branch seems polluted by an undeclared pre-req [00:16] davecheney: https://codereview.appspot.com/6209092/diff/10001/environs/ec2/ec2.go?column_width=80 [00:16] yeah, only open.go and open_test.go [00:16] I will resubmit it once SetConfig is landed [00:16] davecheney: Ok, it'd be nice to get the diff fixed [00:17] if you coudl take a cursory glance at environs/open{,_test}.go [00:17] that is the only part changed [00:17] davecheney: It looks good [00:18] its not a big change, just exposing the EnvironConfig before it gets passed to Open [00:18] davecheney: Yeah, it looks fine if that's all, but it'd be nice to clean up the diff before getting it in [00:18] i will certainly do that [00:21] davecheney: Cool, should be a trivial LGTM after that [07:55] aujuju: Creating volume group in nova-volume Juju charm [09:32] morning [09:44] heya TheMue [09:49] TheMue, fwereade: yo! [09:51] rogpeppe, fwereade: Heya [09:51] rogpeppe, heyhey [09:51] fwereade: i bet this weather is just what you're used to... were you really pining for a bit of old-fashioned english drizzle? :-) [09:52] rogpeppe, well, it *is* really nice for about 10 minutes [09:52] * rogpeppe likes a bit of rain too [09:52] but not for months on end :-) [09:52] rogpeppe, after that I start to congratulate myself for moving ;) [09:55] * TheMue sits again on the veranda, right now 27°C and wonderful sun. === Guest__ is now known as smaddock [10:20] So, short break, have to bring daughter to work. [10:20] morning. [10:24] hey ra [10:24] hey Aram [11:25] * davecheney waves to rogpeppe [11:25] * rogpeppe waves to davecheney [11:25] * rogpeppe waves to davechen1y [11:25] so, how would you like to manage this merge ? [11:26] davecheney: 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] how is imagespec looking ? [11:26] davecheney: i've spent... too much time pissing around with splitting up branches and merging and remerging [11:26] f'yeah, i'm over reqs [11:27] davecheney: 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] that is cool [11:27] davecheney: it should be less important now that i've unexported the types. [11:27] i'm happy to wait for those two to go in [11:27] then I'll merge and adjust as needed [11:27] gostavo has LGTM in principal the various bits [11:27] davecheney: thanks. [11:27] i'm sad that the proxy idea didn't get up [11:28] but at the end of it I couldn't offer a strong enough argument over Environ.SetConfig [11:28] davecheney: i think it made sense. but i suppose it only makes sense when the provider has no state [11:28] possibly if there was going to be more than one PA [11:28] once we have somethig working [11:29] i'll hvae another crack at introducing the proxy idea [11:29] as a way of seperating concerns [11:29] davecheney: i quite liked the "return a proxy within ec2" idea [11:29] I was thinking proxy.(*proxyEnviron).SetConfig(config) [11:29] which would reload it on command [11:30] which might resolve gustavo's concerns about control [11:30] from my vantage point, the Environs themselves contain almost no state [11:30] davecheney: i was thinking type Proxy struct {env environs.Environ} [11:30] yup [11:30] or even [11:31] type Proxy struct { environs.Environ } [11:31] davecheney: and then environs.config.Open returns Proxy{ec2environ} [11:31] but that has visilibity problems [11:31] anyway, i'll wait til we have something that works [11:31] davecheney: yeah, i think you want to be sure that all methods are reimplemented [11:31] rather than spinning on this [11:32] davecheney: yeah, the current thing will work. [11:33] from my POV I need some commands to do some functional testing on the PA [11:33] but I was wondering how jujuc is coming along ? [11:33] davecheney: it needs more stuff. [11:34] from a quick look at the supercommand stuff [11:34] davecheney: i'm sure fwereade will be cranking out more commands soon though [11:34] i can add extra commands to the framework pretty simply [11:34] davecheney: by "functional testing" you mean ad-hoc testing? [11:34] I basically need to invoke, addmachine and remove machine [11:34] in another window [11:34] davecheney: can't you write tests that use the State methods to do that? [11:35] i also need to figure out how to hook the provisioning command up to StateTest so I can do unit tests against dummy with it [11:35] davecheney: 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:36] i've found there is value in doing both [11:36] for example I wrote a quick SOCKS over ssh proxy [11:36] then left that running on the open internet for a day to load test the ssh client code [11:36] davecheney: these days i tend to put the effort i would spend on thinking of ad hoc tests into thinking of reusable tests [11:37] davecheney: there's definitely room for functional testing, but even then i think there's value in scripting it. [11:37] davecheney: 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:38] of course, hence the need for simple cli tools to push the state around so I can watch the PA recact to it [11:39] davecheney: 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] davecheney: perhaps using the dummy provider so you can read actions as they take place. [11:39] yup, i'm not sure how much of that harness exists atm [11:40] i know environs/jujutest does some of it [11:40] davecheney: cmd/juju does a certain amount of it. [11:40] davecheney: really, i think it would be nice if the dummy provider could be configured to start zk [11:41] davecheney: then we get an almost-complete environment for testing. [11:41] i'll look into that [11:41] i don't think it does that at the moment [11:41] davecheney: it doesn't [11:41] davecheney: it shouldn't be hard though, particular with the recent refactoring of it i've done. [11:41] in fact I think it works the opposite, it waits for the juju/testing to make a zk [11:41] indeed [11:41] i'll look at it tomorrow [11:43] rogpeppe: can you chain req branches ? [11:43] davecheney: yeah [11:44] ie propose a, propose b -req a, propose c -req b [11:44] davecheney: although it's a hassle, because once you've changed the base, you have to remerge all the subsequent branches in the chain [11:44] yes, that doth blow [11:45] davecheney: 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] i'm looking forward to phase of th eproject when we've got something we can itterate on [11:45] davecheney: definitely. [11:45] when I was building the platform at atlassian we got a prototype working very quickly [11:45] davecheney: i'm just about to propose a branch that gets the go tools and runs them on the server side, BTW [11:46] but then there was a big gap of nearly 2 months before we got the next version working [11:46] davecheney: we've already got a prototype - the python version... [11:46] rogpeppe: nice [11:46] rogpeppe: the first version of the atlassian platform was three desktops that we built by hand [11:46] but the next version took a long time because we decided to build all the automation first [11:47] but, once it was running itteration was very fast [11:47] davecheney: i think that the Go version will be nice to iterate on when we've got it up. [11:47] yup [11:47] davecheney: i'm quite happy with how the overall structure is turning out [11:48] davecheney: slight misgivings about the whole necessity of the State abstraction, as you pointed out, but i'm ok with it. [11:49] rogpeppe: tell me more [11:50] davecheney: i think it was you that suggested that all of state could be represented more compactly by a simple parent/child/attribute abstraction. [11:51] I 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 themselves [11:51] s/as the/ask the/ ? [11:52] ya [11:52] for the particular example of wtching machines [11:52] I want the ability to subscribe to a part of the toploogy [11:52] what we have now works fine [11:52] davecheney: +1 [11:52] watch /topology, then parse the full thing [11:53] but given that the State already has the whole topology as a data structure [11:53] it would make sense that it could push data to any watcher directly [11:53] but I wonder if gustavo has plans for the topology [11:53] as it is a crutch for the limitations of zk [11:53] davecheney: i think the eventual idea is that we push all of state behind a server API [11:54] at which point all the watchers are going to be pub/sub kind of things [11:54] davecheney: then we don't have to worry about atomic changes to the topology because the server can mediate that [11:54] davecheney: exactly [11:54] speaking of Environs [11:55] davecheney: it's interesting to think about what level we might want the server API [11:55] I was going to try a branch that removes the requirement to pass state.Info to Environ.StartInstance [11:55] davecheney: how would the new instance know how to connect to the state then? [11:55] rogpeppe: i hope it looks like a URL tree [11:55] pretty much the same as the zk topology [11:55] but with REST calls that you can subscribe to changes to a path and it's children, etc [11:55] davecheney: yeah, but what level of abstraction are the URLs? [11:56] without giving it any thought [11:56] davecheney: i dunno. i was wondering about something higher level. [11:56] i think the URL's should look like the current ZK path [11:56] but that is without giving it any serious consideration [11:56] davecheney: i don't think so, actually. i think that constrains the possibilities to db optimisation. [11:56] s/to db/for db/ [11:57] davecheney: higher level API gives more scope for changing the impl behind the scenes [11:57] davecheney: anyway... back to ground level [11:57] yeah [11:57] davecheney: how does StartInstance work without having a StateInfo? [11:57] s/does/would/ [11:58] there is a StateInfo method on the Environ itself [11:58] ergo, it already knows it's state [11:58] err, state.Info [11:58] davecheney: not necessarily. i wanted to keep open the possibility that one environ might connect to a state from elsewhere. [11:58] davecheney: there's no essential connection between a state and a given environ. [11:59] davecheney: for instance, at some point in the future, we want to have the possibility of several environments using the same "bootstrap" node. [12:00] rogpeppe, 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] rogpeppe, it feels to me like a state is pretty strongly associated with an environ [12:01] fwereade: i agree [12:01] so either Environ gets closely bound to a state.Info, or is completely abstracted from it [12:01] either way, the requirement to pass state.Info in StartInstance will go away [12:02] rogpeppe, davecheney: yeah, that param definitely feels wrong to me [12:02] fwereade: 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:03] davecheney: yes, at least that. i'm trying to remember the other scenarios. [12:03] or to say it another way, how it can build and Environ that matches that of the Provisioning Agent that created it [12:03] davecheney, rogpeppe: sorry, I'm confused... doesn't the environ already know its own state info? [12:03] fwereade: yes, it must, because there is this method [12:04] Environ.StateInfo [12:04] fwereade: only if you've bootstrapped it. [12:04] rogpeppe: i'm happy with that restriction, the PA doesnt' run til bootstrapping is complete, unless i'm mistaken [12:04] fwereade: 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] rogpeppe, 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 complete [12:05] rogpeppe: ah, i see what i'm missing [12:05] bootstrapping uses Environ.StartInstance to start machine(0) [12:06] davecheney: actually, it uses startInstance; slightly different. [12:06] still, possibly a strategic nil could be passed, suggesting, use whatever state you have internally [12:07] part of the problem is we don't want to pass the ec2 secret key in the cloudinit script [12:07] davecheney, rogpeppe: it feels to me like bootstrap is the special case, and it's misleading to expose those details to the outside world [12:07] i concur [12:08] hmm, i think it feels quite clean actually. we explicitly tell the new machine how to connect to the juju state. [12:08] it's always going to need to do that, i think. [12:08] rogpeppe, but the environ is so closely tied to the state that it feels wrong to have to tell it again [12:09] fwereade: i don't see the coupling as that close actually. [12:10] rogpeppe, Environ.StateInfo implies otherwise, to me [12:10] fwereade: i like the idea of being able to have the machines in a single environ controlled by multiple states, for example. [12:11] rogpeppe, sure, that has potential: but I'm pretty sure that's not what we're implementing right now :p [12:11] rogpeppe, would you say that Environ.StateInfo was a misstep that we should remove? [12:11] fwereade: 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 stateinfo [12:12] davecheney, exactly so [12:12] on the other hand, if the envion shuldn't need to know about the stae, then the StateInfo method is misleading [12:12] davecheney: no, i don't think an environ does know its own state [12:12] as it stands, it's not a big deal to do si, _ := environ.StateInfo(); environ.StartInstance(99, si) [12:12] davecheney: the private keys are not accessible to an environ for example [12:13] rogpeppe: then what is returned from environ.StateInfo() is not actually useful then ? [12:13] oh, hang on [12:13] i'm talking about stateInfo, which is just a pretty connection string to find zookeepers [12:15] davecheney, I must have missed something; how does that change things? [12:16] fwereade: 22:12 < rogpeppe> davecheney: the private keys are not accessible to an environ for example [12:16] * rogpeppe is thinking [12:16] ^ i interprited that to mean more than it did [12:16] davecheney, ah, cool, thanks [12:16] davecheney: yeah, and i'm not sure i meant what i said. [12:17] fwereade: rogpeppe i'm sure this is over simplifying this, but for the privisioning, machine and unit agents [12:17] to get up and running [12:17] all then need to know is the argument they are passed to jujud, and a stateinfo, which is really just a string passed to --zookeeper-servers [12:18] davecheney, yes, essentially [12:18] and so, Environ.StartInstance() is shorthand for 'start a new machine, and run this daemon passing these arguments' [12:18] davecheney, the PA also needs to know the environ, and the UA/MA need to know what U/M they's acting for [12:18] yeah, there is clearly more too it than I just glossed over [12:19] but from the point of view of 'how do I find zookeeper so I can watch stuff' [12:19] davecheney: 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:20] davecheney, sure, understood [12:20] * fwereade thinks [12:20] rogpeppe: 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 network [12:20] davecheney: what about the juju client? [12:21] davecheney, 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 node [12:21] rogpeppe: good point, i don't have an answer to that, other than saying we probably need two state.Info's [12:21] external and internal [12:21] or something that clearly identifies external (as in not inside the environment) and internal (that are) [12:21] otherwise we'll go mad with the permutations [12:22] davecheney: how does a machine know if it's internal or external? [12:22] jujuc == external, jujud == internal [12:22] as a start [12:22] davecheney, so it feels wrong to push that one specific difference up to that level while leaving the others implicit [12:22] davecheney, wait, what? [12:23] but trying to make one connectino string serve two masters sounds worse [12:23] rogpeppe: as I understand it, all machines are internal; the are inside the environment [12:23] currently StateInfo returns the external address. [12:23] if ec2 lets that work, then it's a solved problem for now [12:23] davecheney: the difficulty is that Environ is used by the client as well as the cloud machines [12:24] davecheney: i think the connection methods are different too. i'm not sure we use ssh between cloud machines. [12:25] ahh, so the reason StartInstance takes a state.Info is to allow us to construct an 'internal' connection string [12:25] davecheney: that's certainly one reason, yes. [12:26] davecheney, rogpeppe: hmm... except when bootstrapping, the state info comes from an Instance, right? [12:27] davecheney, rogpeppe: which already knows both internal and external addresses [12:27] fwereade: no, it comes from the private storage [12:27] rogpeppe, how did it get there? [12:27] fwereade: it was put there at bootstrap time. [12:27] rogpeppe, if not from the Instance constructed on bootstrap [12:27] fwereade: at that time we don't know the DNS addresses of the instance. [12:28] rogpeppe: 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] rogpeppe, wait, you just said the information was put in provider storage at bootstrap time [12:28] fwereade: the instance id only [12:29] davecheney, no, the slice is all ZKs; internal/external is just about what address we use to connect to a ZK or group thereof [12:29] so... if i understand the general idea, we could have ... [12:29] rogpeppe, 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:30] one mo, tel call [12:30] rogpeppe, ...and we're now discussing how we go about reacquiring the private address? [12:30] fwereade: yeah [12:30] rogpeppe, it seems to me that the trouble is in throwing away half the information in the first place [12:31] so... if i understand the general idea, we could have a "local" argument to StateInfo which determines which info to get. [12:31] 'cos we can't get rid of StateInfo entirely, right? [12:32] so we're suggesting that StartInstance call StateInfo(local) inside itself, rather than taking an explicit state.Info arg? [12:33] rogpeppe, perhaps something like that: I haven't thought it through entirely [12:33] rogpeppe: i'd like to try that [12:33] rogpeppe, but it feels like a potentially fruitful avenue [12:33] i don't really see the point. i don't think it'll make anything significantly simpler [12:33] or at least have StartInstance(314, nil) as a sentinal for 'pass whatever state.Info you know/makes sense/sounds best' [12:34] rogpeppe: as it sounds like the reason StartInstance takes a state.Info is to facilitate bootstrapping [12:34] davecheney, doesn't StartInstance imply non-bootstrap in the first place? [12:34] * davecheney shrugs [12:35] fwereade: yes [12:35] davecheney, 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] davecheney, rogpeppe: ... or .StateInfo [12:35] davecheney, rogpeppe: depending on how we do the details [12:36] fwereade: it means that StartInstance will have to read the provider local storage each time, i think. [12:36] fwereade: 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 argument [12:36] at this moment the only thing it can use is the value from a.Conf.StateInfo [12:37] which comes from the jujud super command framework [12:37] davecheney, rogpeppe: how does the same argument not apply to Environ.StateInfo? [12:37] fwereade: that is an intersting one [12:37] fwereade: if the caller already has a StateInfo, it can pass that to StartInstance each time. [12:37] the PA is connected to a state.State [12:38] it find a *ConfigNode by watching /enviornment, and passes that to environs.NewEnviron [12:38] and gets an Environ [12:38] if 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] what that environ.StateInfo() returns is a damn good question [12:39] davecheney: the bucket will be configured in the attributes it passes to SetConfig [12:39] davecheney: and that's what StateInfo will use to return the state.Info [12:39] rogpeppe, we surely will at some stage... in which case surely the right answer *is* to read it every time [12:40] rogpeppe, but this seems essentially orthogonal to the question of whether it needs to be passed to StartInstance [12:40] fwereade: 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:41] fwereade: bootstrap and client connections, of course. [12:41] fwereade: there's no way of *watching* provider storage, i think [12:41] rogpeppe, I thought you said bootstrap didn't use StartInstance [12:42] rogpeppe, davecheney: oh hell, EOD, got to go to the bank :( [12:42] BTW the provider state is written *after* the bootstrap node is successfully started. [12:42] rogpeppe, davecheney: hope I wasn't just muddying the waters for you [12:42] fwereade: no, it's all good stuff [12:42] rogpeppe, I know; don't get current significance though [12:43] +1 [12:43] fwereade: it means that the bootstrap node doesn't necessarily have access to the provider state when it starts. [12:43] of course in practice with ec2, it's so slow that it will in fact. [12:44] rogpeppe, but we bake the stateinfo into cloud-init regardless [12:44] fwereade: we can't do that if we can't pass it in to StartInstance [12:45] fwereade: because StartInstance will, of necessity, retrieve the state info from the provider storage bucket [12:45] rogpeppe, 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 localhost [12:45] rogpeppe, and did I misunderstand about StartInstance not actually being used to bootstrap anyway? [12:46] fwereade: i'm not sure that the Environ knows that it's on a bootstrap node. [12:46] if I can interrupt for a moment [12:46] davecheney: please do [12:47] * fwereade listens [12:47] is 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] davecheney: that's not the only reason. [12:47] davecheney: the reason we were just talking about was that there's more than one source for a StateInfo [12:48] davecheney: it might come from the provider storage; or it might come from being passed into the cloudinit script. [12:48] rogpeppe: but from the POV of the PA, there is only one stateinfo [12:48] their are many, but this one belongs to the PA [12:48] davecheney: yeah, the one that's passed on the command line [12:48] davecheney, rogpeppe: I do actually have to go now; ttyl :) [12:48] fwereade: k [12:48] kk [12:48] fwereade: c u tomorrow [12:49] * rogpeppe also has to go to lunch soon [12:49] fair enough, lets wrap up [12:49] from what i've read today, the best course of action is to not change naything [12:49] the PA calls environ.StartInstance(999, a.Conf.StateInfo) [12:50] we'll revisit it later, if necessary [12:50] davecheney: 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:51] davecheney: passing in StateInfo explicitly means the environ doesn't need to care how we got it. [12:52] davecheney: which i *think* is a reasonable separation of concerns. but please keep on considering alternatives! [12:52] right, lunch! [12:53] davecheney: see you tomorrow, i hope, if i get up early enough. [13:09] rogpeppe: kk, i'll catch you on the next rotation [13:54] rogpeppe: hi [13:55] rogpeppe: I sent another review [13:58] andrewsmedina: thanks. i'll take a look. [14:03] * niemeyer waves [14:03] niemeyer: yo! [14:07] andrewsmedina: LGTM [14:07] rogpeppe: thanks \o/ [14:08] niemeyer: i updated https://codereview.appspot.com/6188068/ to unexport the types and change the docs. hopefully should be better now. [14:09] rogpeppe: Cool, thanks [14:09] rogpeppe: Agreed regarding it going out of the ec2 package eventually [14:09] rogpeppe: 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:10] niemeyer: will do. [14:14] niemeyer: Moin and thx for the reviews. [14:14] rogpeppe: It actually feels like going backwards a bit [14:15] niemeyer: oh? [14:15] rogpeppe: All those field name changes will have to go back again in the next branch that does the actual switch over [14:15] rogpeppe: The way we have colseries and colServer also feels quite awkward [14:15] rogpeppe: These names should be internally consistent [14:16] niemeyer: i'm not sure. it's only the instance contraint that will have to change [14:16] niemeyer: colseries is my mistake - a global replace too enthusiastic! [14:16] rogpeppe: Ah, phew [14:16] TheMue: Heya! [14:16] TheMue: np [14:16] niemeyer: i think that instanceSpec will remain private [14:16] niemeyer: and instanceConstraint will be significantly different [14:17] rogpeppe: I'm tempted to suggest moving the InstanceConstraints to environs right now [14:17] niemeyer: so i don't mind it being private now [14:17] rogpeppe: If it will be significantly different, we're doing it wrong [14:17] niemeyer: can we leave it as it is - i've got three upstream branches that i'm juggling on top of this one [14:17] niemeyer: then i can do the constraints stuff later [14:18] rogpeppe: Yep, sure [14:20] rogpeppe: Done [14:20] niemeyer: thanks a lot [14:20] rogpeppe: np [14:46] niemeyer: remind me again how i can get the full revision id for the current branch out of bzr, please. [14:46] niemeyer: my google-fu has deserted me [14:46] rogpeppe: revision-info should do [14:46] ah! [14:47] niemeyer: i was trying revno and info [14:47] rogpeppe: revision-info --tree is relevant in some cases [14:47] revno -v should do it, i reckon [14:51] bcsaller: ping [14:52] aujuju: Juju error: Server refused to accept client [15:12] Lunch, biab [16:08] TheMue: ping [16:08] niemeyer: pong [16:08] TheMue: yo [16:08] TheMue: Would you mind if we at least delayed the addition of that method? [16:09] TheMue: 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 have [16:09] niemeyer: So far I've got no problem. The original is used two or three times (beside the tests). [16:09] TheMue: So it'd be awesome if we could keep that out while we figure the answer for that [16:09] niemeyer: So I'll see when I reach this point. [16:10] TheMue: I'm sure it's used, but I think we have a better interface that may enable its original use case withou tit [16:10] niemeyer: https://codereview.appspot.com/6099051/ [16:10] andrewsmedina: Neat, will give it a final pass [16:10] niemeyer: OK, so I'll kick it with the next proposal. [16:11] TheMue: Cheers, please ping me once you're done.. I suspect it's ready for going in, and will check as soon as you push [16:11] andrewsmedina: Why network.Subnet has no xml tag? [16:14] niemeyer: its only used to create a new net [16:16] niemeyer: this attribute is not returned in the xml generated by virsh [16:19] andrewsmedina: Cool [16:22] andrewsmedina: The tests will fail without root, right? [16:29] niemeyer: wrong [16:29] andrewsmedina: Ah, you're faking the whole thing.. hmmm [16:29] andrewsmedina: That's both nice, and not''' [16:30] niemeyer: yes... [16:30] andrewsmedina: Either way, we can move forward like that and address it later [16:30] andrewsmedina: It's better then the opposite I guess [16:31] niemeyer: but it was the only way that I found not to change the network configuration of the machine on which the tests have been run [16:32] andrewsmedina: Yeah, that's what I mean.. your approach is better than being *just* on the other side of the fence [16:32] andrewsmedina: 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 real [16:32] andrewsmedina: Or maybe not.. external functional tests would deal with that too [16:32] andrewsmedina: Either way, you have a review with a few trivialities, and LGTM [16:33] andrewsmedina: Please ping me once these are done and I'll submit [16:33] andrewsmedina: (rog had a trivial too) [16:33] niemeyer: ok [16:33] niemeyer: what trivial rog had? [16:33] andrewsmedina: It's in the CL [16:33]