[04:15] good morning. [07:46] yay, I has power [08:15] going out to get some coffee and a pastry, still recovering from broken fridge :/ [09:02] heya TheMue [09:02] Hi fwereade [09:03] Hmm, I hope my line gets more stable when the new provider has set it up. [09:15] Just seen, need a new watcher. [09:44] TheMue, sorry, what watcher is that now? :) [09:45] fwereade: The ServiceWatcher for added or removed services. It's needed by the firewall. [09:45] TheMue, ah, cool [09:46] fwereade: Thx to your watcher changes it's pretty simple now. [09:46] TheMue, I kinda suspect that about 50% of juju will actually just be watchers of one sort or another by the time we're done :) [09:46] TheMue, awesome [09:46] fwereade: Yes, we're a kind of event-driven [09:47] TheMue, yeah, totally [09:49] fwereade: So one architecture alternative could have been that all those watchers publish their changes to a kind of bus where interested listeners subscribe to those events. [09:50] TheMue, not sure, there are a number of subtle ordering interactions in a couple of places [09:51] TheMue, we only want to watch X in between a specific pair of events on Y, for example [09:51] TheMue, and we don't want to just watch ALL THE CHANGES all the time [09:51] TheMue, I guess it could be done [09:52] fwereade: That's depending on the subscription mechanism, would be possible [09:52] TheMue, but I think unwatching X at the right time, in the example above, would be a bit fiddly [09:54] TheMue, offhand, do you know whether we currently have any mechanism for putting *valid* environment configs into /environment? [09:54] TheMue, I know we can poo an arbitrary map[string]interface{} in but I don't think there's yet any way to push a real env config implemented in real code [09:57] fwereade: No, that doesn't sound real got. [09:58] fwereade: The environment fields depend on the chosen environment? [09:58] TheMue, yeah [09:58] TheMue, I'm implementing bits and pieces in that area right now [09:59] TheMue, I'm just trying to figure out whether I will break things *any worse than they currently are* if I follow my preferred path [09:59] fwereade: So I would setup a type hierarchy per environment and implement a Validate() on each type working top-down. [09:59] TheMue, whoa, hierarchy?> [09:59] fwereade: I'm doing so for my private RSS and Atom implementation. [09:59] fwereade: And that could be marshaled to e.g. JSON inside the node. [09:59] TheMue, we already have code for validating them [09:59] TheMue, NewConfig :) [10:01] fwereade: Just see it, very generic. [10:14] Morning! [10:19] niemeyer, heyhey [10:19] fwereade: Heya! [10:20] niemeyer, how's it going? [10:20] fwereade: Great [10:20] fwereade: A little bit on the short side in terms of sleep, but hopefully will fix that tomorrow :) [10:20] niemeyer, awesome; thanks again for the conversation yesterday, everything seems to be lining up really neatly now [10:20] fwereade: My pleasure! Appreciated too. [10:21] niemeyer, I have a couple of CLs that should be short/sweet: https://codereview.appspot.com/6345071/ and https://codereview.appspot.com/6351068/ [10:23] niemeyer: Morning [10:23] TheMue: Heya [10:23] fwereade: Looking [10:39] fwereade: https://codereview.appspot.com/6345071/ done [10:43] niemeyer, cheers [10:43] fwereade: I'm not sure about the other one.. what's the benefit? [10:52] Aram: ping [10:54] Hmm, sh**, have to reboot. Back in a moment. [11:06] niemeyer, sorry, was making lunch [11:06] niemeyer, benefit is that we can make State.EnvironConfig return an environs.EnvironConfig [11:07] niemeyer, which really feels like the Right Thing now I've started implementing it [11:12] niemeyer, (but incorporating the move in the forthcoming CL for EnvironConfig felt a bit much) [11:12] fwereade: How about moving EnvironConfig to state instead? [11:13] niemeyer, don't think so offhand -- we also need NewConfig, and IMO that's unquestionably part of environs [11:13] niemeyer, besides, the environs are meant to not depend on state, right? [11:13] fwereade: Why do we need NewConfig on State? [11:14] fwereade: Or is the state supposed to not depend on environs? ;-) [11:14] niemeyer, because if we try to do environs.NewConfig from state we get an import cycle [11:14] fwereade: Why do we *need* NewConfig on state? [11:15] fwereade: [11:15] [niemeyer@gopher ..juju-core/state]% grep -r environs * [11:15] [niemeyer@gopher ..juju-core/state]% [11:16] niemeyer, sorry, don't know what happened there [11:16] niemeyer, because State.EnvironConfig wants to return an EnvironConfig, and all it has is a map[string]interface{} [11:17] fwereade_: Hmm.. something else seems wrong then [11:17] Thinking [11:18] fwereade_: Not sure if you've seen this, btw: [11:18] niemeyer> fwereade: [11:18] [niemeyer@gopher ..juju-core/state]% grep -r environs * [11:18] [niemeyer@gopher ..juju-core/state]% [11:18] niemeyer, yeah, tautologically so; environs was importing from state [11:19] fwereade_: Yes, which is a point worth bringing into the discussion [11:20] fwereade_: You say it's uncontroversial. It would be if we were simply taking a dependency down, but we're inverting a dependency [11:20] fwereade_: Some of the purpose of the Conn, for example, is precisely to integrate an environ with a state [11:20] niemeyer, environs is responsible for creating every known (non-test) instance of the types it imports from state [11:21] niemeyer, from a certain perspective, environs is the only "source" of those types [11:22] niemeyer, you were once very firm that environs must not depend on state -- do you recall something specific that changed your perspective? [11:23] niemeyer, another thought; if InstanceId were a type, that would be another one that would naturally live in environs and be used by state [11:24] fwereade_: That's not a relevant point.. the only purpose of state.Info is to provide it onto the state package as an argument to Dial. Of course it won't be *created* by the state package. [11:26] niemeyer, fair point indeed [11:29] niemeyer, the heart of it is that if we want State.EnvironConfig to return an environs.EnvironConfig, or even to be able to check the sanity of environment config changes, state needs to use environs [11:30] So, new ServicesWatcher for review is in. [11:30] fwereade_: I'm not sure we do.. let's look at the overall issue without an established dogma to make sure we're doing the right thing [11:32] fwereade_: Here is an idea.. [11:33] fwereade_: Configuration always comes in the form of keys, right? [11:33] niemeyer, yes [11:33] fwereade_: In state, in environments.yaml, and from the cmdline [11:33] niemeyer, agreed [11:34] fwereade_: We actually have infrastructure to deal with that information as keys even, in the form of the schema package [11:34] niemeyer, right... [11:34] fwereade_: So, it's at least interesting that we chose to transform that information onto a type so early on [11:35] fwereade_: What if.. [11:35] fwereade_: EnvironsConfig was not an interface.. [11:35] fwereade_: EnvironConfig, that is [11:35] fwereade_: But rather, *state.EnvironConfig [11:35] ? [11:36] niemeyer, how will that end up being noticeably different from the python-style data-bag? [11:37] fwereade_: I don't know what's the underlying statement you're making with that. The configuration of an environment *is* a data bag. [11:38] fwereade_: And this is quite bogus: [11:38] / EnvironConfig represents an environment's configuration. [11:38] type EnvironConfig interface { [11:38] // Open opens the environment and returns it. [11:38] Open() (Environ, error) [11:38] } [11:38] fwereade_: This is just wrong.. [11:38] niemeyer, ok, we followed this approach in python, and we ended up in a totally ridiculous situation, in which keys were added and used willy-nilly without even being used in the schema [11:39] fwereade_: The interface of an environment configuration cannot tell us anything about the configuration, but *opens* the environment [11:39] wtf? [11:39] fwereade_: Well, we don't want that to happen for sure [11:40] fwereade_: But that's a separate issue [11:40] fwereade_: and one that you just helped solving, in fact ;) [11:40] niemeyer, well, it is a big advantage of turning things into actual types as early as we can :) [11:41] fwereade_: I'm not sure.. that's exactly why I started with this: [11:41] niemeyer, how is the configuration of an environment anyone's business but the environ itself? there are, it is true, some common properties, but they can be added to the EnvironConfig interface as we feel the need for them [11:41] fwereade_: Here is an idea.. [11:41] fwereade_: Configuration always comes in the form of keys, right? [11:41] fwereade_: In state, in environments.yaml, and from the cmdline [11:41] fwereade_: We actually have infrastructure to deal with that information as keys even, in the form of the schema package [11:41] fwereade_: So, it's at least interesting that we chose to transform that information onto a type so early on [11:42] fwereade_: Aren't you trying to inject a dependency on environs onto the state just so you can get an EnvironConfig from it? [11:42] fwereade_: Clearly it seems to be someone else's business too [11:43] fwereade_: cmdline, state, config [11:43] niemeyer, well, the state has to store EnvironConfigs somehow, right? [11:44] fwereade_: The configuration of an environment has well known fields, and then other flexible fields [11:44] niemeyer, sure, I would like at some stage to split the common fields out into their own type, but that's orthogonal [11:44] fwereade_: What I'm suggesting is that we, actually, realize the EnvironConfig into exactly what it is [11:44] fwereade_: We can have methods on it [11:46] fwereade_: And can also have a Specific method, for example that returns all the unknown fields [11:46] fwereade_: (*without* the well known ones.. we handle those internally) [11:46] niemeyer, and how are we meant to have any sanity-checking on those unknown fields? [11:47] fwereade_: That's the environ's business to do [11:48] niemeyer, ok, so State.Set/UpdateEnvironConfig should just poo whatever it's given into /environment and damn the consequences? [11:48] * niemeyer waits until fwereade_ relaxes again [11:49] * fwereade_ is sorry, but this feels like a pretty fundamental change in direction of which I had no warning [11:49] fwereade_: Who, in the sense of which part of the code, knows what unknown fields are valid? [11:49] TheMue, something in environs [11:50] fwereade_: I think you're being slightly unfair [11:50] fwereade_: You're the one changing the direction of things [11:50] fwereade_: This is in state, today: [11:50] / EnvironConfig returns the current configuration of the environment. [11:50] func (s *State) EnvironConfig() (*ConfigNode, error) { [11:50] return readConfigNode(s.zk, zkEnvironmentPath) [11:50] } [11:50] fwereade_: We're "pooing" stuff with "damned consequences" in the current code base [11:51] niemeyer, yes, and I feel it's a serious problem [11:51] fwereade_: If it's in environs it doesn't seem unknown to me. environs only should handle well knows fields. If we have a need for more we should define a validator function type so that the provider/user of those fields could also provide a validator. [11:51] TheMue, so every time we create a State we pass in an EnvironConfigValidator? [11:51] fwereade_: Yes, so let's run through this with less emotion, ok? I'm not changing the direction of anything, and I'm not taking you by surprise on anything [11:51] niemeyer, you were very very clear in orlando that environs must not depend upon state [11:52] fwereade_: Only when changing the content of the EnvironConfig. [11:52] fwereade_: and it does not depend on state.. it depends on data [11:52] fwereade_: We can put EnvironsConfig onto the foobar package if you want [11:52] niemeyer, seeing environs using state, I thought "oh, that's just a mistake that has lain dormant because we hadn;t yet implemented enough to be aware of the problems" [11:53] fwereade_: I didn't say in Orlando "let's make state depend on environs" [11:53] fwereade_: Changing 6 by 12/2 won't help [11:54] niemeyer, ok, so your position is that state and environ should not know about one another at all? [11:54] fwereade_: Yes [11:54] niemeyer, but state should hold environment configuration? [11:55] fwereade_: Re-read that question again, and think through it in the broader sense of things for a moment [11:55] fwereade_: What *is* the state? [11:55] niemeyer, a description of the desired state of the environment? [11:56] fwereade_: Is it something that represents a state or is it something we only store convenient in ZK? [11:56] fwereade_: Exactly.. it's the information about the whole environment [11:56] TheMue, AFAICT it absolutely represents state... not sure what you're getting at [11:57] fwereade_: We can put the type elsewhere [11:57] * TheMue asks to reflect this topic from a distance. [11:57] fwereade_: But the state will necessarily have to *hold the state configuration*! [11:57] Erm [11:57] fwereade_: But the state will necessarily have to *hold the environment configuration*! [11:57] fwereade_: There's simply no option for it to not do it [11:57] fwereade_: It does today, already [11:58] niemeyer, yes, this is at the heart of my conviction that the state package is a natural consumer of the environs package [11:58] fwereade_: That's the fundamental disagreement [11:58] fwereade_: The environs package *talks to the environment* [11:58] fwereade_: There's zero reason for the state package to talk to the environment [11:58] niemeyer, which abstracts away the differing features of various environs such that they can be used by other code without them having to worry about the differences between them [11:58] fwereade_: My understanding has been that other packages (like environ) just *use* state. [11:58] fwereade_: The right thing for us to do is to break the dependency, both ways [11:59] niemeyer, ok, fine as far as it goes, wrt the types moved in the CL [12:00] niemeyer, but it seems to me that it is valuable for the state to be able to validate its environ config somehow [12:00] IMHO environs has to read the configuration from state and instantiate the according "factory" (dislike the word) which then uses the config data to do the real setup. [12:00] fwereade_: It's valuable to validate the environ config somehow, agreed [12:01] fwereade_: The state doesn't have to do that, though [12:01] niemeyer, and I don't see how it can do that without using code from the environs package [12:01] fwereade_: The state package doesn't have to do that [12:02] niemeyer: Validation has to be provided by the various environs, EnvironConfig only provide a mechanism. [12:02] TheMue: You can assume I understand that [12:02] niemeyer, I would prefer that it be *impossible* to set a broken environment config via the state package [12:03] niemeyer: Fine [12:03] niemeyer, I guess we could also pass a ConfigValidator into state.Open, or something..? [12:04] niemeyer, hmm, ok, actually, the fundamental thing I don't understand is whystate should not depend on environs [12:04] fwereade_: IMHO only when writing the config. [12:04] niemeyer, would you expand on that a little? [12:04] fwereade_: It feels like a slightly abstract goal to me. There are very few places the system gets unreliable configuration from the outside world. [12:05] TheMue, so it's not worth checking validity on the way out? only on the way in? [12:05] fwereade_: On the way out? [12:05] fwereade_: If it only can be written well validated, reading could be assumed as ok, yes. [12:06] TheMue: +1 [12:06] fwereade_: We're not checking anything else for brokeness every time we load from state.. (?) [12:08] niemeyer, aren't we? in general I think we return either errors or valid instances, rather than timebombs without errors [12:08] Exactly, only when during a following operation something weird is discovered an error is returned. This indeed can happen in operations depending on the config too (e.g. by changing it concurrently while it's already loaded by another part). [12:09] fwereade_: No.. if you get "poo (*&@(*#&(!" as a service name.. do we validate that? [12:09] fwereade_: This feels like a total departure [12:09] niemeyer, yeah, probably [12:09] niemeyer, could we return to "why should state not use environs" please? I think there's something I'm missing there [12:10] Errors always may and can happen, so it's more useful to keep our software handling those errors well. [12:11] fwereade_: It's a dependency thingy. environs uses state, state uses environs? Can't say exactly why, but I dislike this behavior. [12:11] TheMue, that's unpossible :) [12:11] fwereade_: Because it's much easier to keep things on track that way. The environs package and the specific implementations of environs have the goal of talking to an IaaS provider. [12:12] fwereade_: The state package has no business there. [12:12] fwereade_: If we start breaking that line, something is going wrong [12:12] fwereade_: That said, [12:12] fwereade_: I wouldn't be opposed to, for example, [12:12] niemeyer, I agree with that bit unquestionably :) [12:13] fwereade_: Introducing the environs/config package [12:13] fwereade_: and importing *that* from state [12:13] niemeyer, it's the other direction I don't see a fundamental problem with, because the state describes the environ and it seems odd to stipulate that it cannot know anything about the environ [12:13] niemeyer, ah-ha! [12:13] niemeyer, I can totally live with that [12:13] fwereade_: Woohay middle ground ;0 [12:13] ;) [12:14] :D [12:14] :D [12:15] niemeyer, so the Open stuff stays in environs, and all the other config stuff moves? [12:16] fwereade_: Yeah, we'll have to figure how to handle Open [12:16] fwereade_: I never liked the idea of EnvironConfig handling Open, fwiw [12:17] fwereade_: I think EnvironProvider.Open sounds sane.. [12:17] niemeyer, that sounds pretty plausible to me [12:17] niemeyer, I'll kick it around and see how it turns out [12:17] fwereade_: Btw, I have a bikesheddy desire of renaming env to envs [12:18] Erm [12:18] fwereade_: Btw, I have a bikesheddy desire of renaming environs to envs [12:18] and Environ to Env [12:18] :-) [12:18] Maybe some day [12:18] niemeyer, +1 but it feels like a distraction right now ;) [12:18] fwereade_: It is [12:19] niemeyer, other issue -- should we keep Info and AssignemntPolicy in state, and consider environs' usage of same to be an acceptable purity break? [12:20] niemeyer, if not, where do they go? [12:20] fwereade_: I think both of these belong in state [12:20] fwereade_: Inherently [12:20] fwereade_: Info we've talked about [12:20] fwereade_: AssignmentPolicy is logic fully implemented in state [12:20] niemeyer, yeah, I'm ok with it, just wanted to confirm [12:21] * fwereade_ breaks out the chainsaw and goes to work on environs :) [12:21] fwereade_: ROTFL [12:24] fwereade_: Be careful, don't forget the earmuffs and safety glasses. [13:07] So folks, will continue tomorrow. Have to do some final preparations before the birthday guests are coming. [13:15] TheMue: Oh, is it your birthday today? [14:29] fwereade_: Oh, btw, I was going to ask you earlier about mstate vs. the test reorganization [14:29] fwereade_: Have you had a chance to look at that? [14:51] niemeyer: pong, oops, different computer. [14:51] niemeyer: thanks for the reviews. [14:51] reviewing the reviews. [14:52] great, thanks. [14:58] niemeyer: I can answer the last question, I am duplicating william's changes. [14:59] niemeyer, yeah, I started talking to Aram and he was already on it :) [15:03] (so it was a short conversation :)) [15:16] Aram, fwereade_: Thanks! [15:59] * fwereade_ straightens up, bloody to the elbows [15:59] niemeyer, https://codereview.appspot.com/6355077 :) [15:59] fwereade_: Awesome! [15:59] fwereade_: I'll have a look right after lunch [15:59] niemeyer, cool, enjoy :) [16:00] fwereade_: Thanks! [17:54] fwereade_: The CL is not quite what I had in mind [17:55] fwereade_: Just thinking through about pros/cons now [18:05] fwereade_: https://gist.github.com/3061697 [18:05] fwereade_: Do we need anything else? [18:38] Aram: ping [18:39] pong niemeyer. [18:40] Aram: Yo [18:40] hey. [18:40] Aram: How're you doing there.. are you working or relaxing? [18:40] Aram: I was going to invite you for a call, but I know it's late [18:40] Aram: So it can wait if that's the case [18:41] yeah, I'm working, I've had some errands to do today, but I'm compensating this evening/night. [18:42] Aram: Awesome, can we have a quick sync up call then? [18:42] yes, yes, skype? [18:44] Aram: G+? [18:44] ok [18:44] Aram: Inviting [18:47] Aram: https://codereview.appspot.com/6356060/diff2/2001:6002/mstate/service.go [18:49] Aram: unit.Life() [18:50] Aram: Remove only Life() == Dead [18:50] Aram: SetLife.. [18:51] https://codereview.appspot.com/6356060/diff2/2001:6002/mstate/state.go?column_width=80