[00:50] davecheney: ping [00:52] ack [00:52] niemeyer: ack [00:52] davecheney: Yo [00:52] davecheney: Sorry, I actually decided to just note down in the review.. we can talk after that [00:53] no worries [01:04] davecheney: Please see what you think of https://codereview.appspot.com/6295067 [01:07] davecheney: I'm looking at https://codereview.appspot.com/6307071/diff/3007/environs/jujutest/livetests.go [01:07] davecheney: How come t.Env.AllInstances returns 1 instance, and t.Env.Instances right above returns two? [01:09] niemeyer: because the code above asks for id0 twice [01:09] AllInstances returns a Set of known instances [01:09] davecheney: ROTFL [01:09] maybe I should add a comment there, took me a while to figure that one out, I'm not even sure if it's correct [01:11] reguarding the rety branch [01:11] i'm just going to remove that comment [01:11] you are of course correct that calling p.Stop() will return nil [01:11] so that is a trap for someone who might add that fuctinoality (via a signal handler or something) [01:14] davecheney: I've added a note on https://codereview.appspot.com/6307071/ [01:14] davecheney: There's one assertion there that would be nice, but otherwise please feel free to go ahead an merge it now [01:14] niemeyer: thank you [01:15] davecheney: I'll likely move the branch over the day tomorrow to lp.net/juju-core (no /juju), so the least branches we have flying around the best [01:15] Even though this time it should really not be much trouble [01:15] Since reviews will remain valid and all [01:15] coiol [01:15] cool [01:30] Okay, almost clean queue [01:31] davecheney: I see a LGTM from rog on https://codereview.appspot.com/6294066/ [01:31] davecheney: I'll have a look at it tomorrow so we can get this in [01:31] sure [02:50] Okay, bed time here [02:50] davecheney: Have a good time there [02:50] niemeyer: will do, thanks again [03:30] aujuju: What is the correct way to set config options to a juju service unit with a file? [04:53] aujuju: Can't get juju to deploy, problem with ec2 keypair? [05:57] davecheney: mornin' [05:57] * rogpeppe is a bit hung over. [06:03] rogpeppe: hey, just responding to your follow up now [06:03] will be but a moment [06:04] https://www.youtube.com/watch?v=AJHCsVZGnNY << audio quality is utterly atrocious [07:12] davecheney: oh yeah, i still have to finish watching that. [07:12] davecheney: i learned one new thing: hex.Dumper. [07:43] rogpeppe, btw, while I remember, what's state.Unit.isPrincipal for? [07:44] rogpeppe, doesn't seem to be used; and Unit already has IsPrincipal()... [07:44] fwereade: i didn't think there is state.Unit.isPrincipal... [07:44] fwereade: let me check [07:44] fwereade: nope, i don't see that method [07:45] rogpeppe, field [07:45] rogpeppe, you added it recently [07:45] fwereade: ah! [07:45] fwereade: yes, it's so that when we're watching units for a machine, we can tell which units are principals without having to read the topology again for each one [07:46] fwereade: i made a mistake actually [07:46] fwereade: Unit.IsPrincipal should just return u.isPrincipal [07:47] fwereade: will fix asap [07:47] rogpeppe, yeah, indeed [07:47] rogpeppe, cool, thanks [07:47] fwereade: good catch, thanks [07:47] rogpeppe, wasn't sure if there might have been something else going on [07:47] cheers [07:47] fwereade: nope, just my forgetfulness [07:48] fwereade: BTW did you see my question on #juju? [07:48] ah sorry no [07:48] rogpeppe, I get to #juju rarely, when I run out of other work to do ;) [07:49] fwereade: i was playing around with writing some charms this morning and wondered what was the best way to wait until all of several relations had been joined [07:50] fwereade: like, for instance, if one relation has been joined, can i query the joined status of other relations? [07:51] rogpeppe, honestly I'm a bit unclear about the current intent of the various hook commands [07:51] rogpeppe, not really up on what jimbaker was doing just before last release [07:51] rogpeppe, bu I need to find out in detail soon, so I can implement them ;) [07:53] fwereade: :-) === TheRealMue is now known as TheMue [08:08] rogpeppe, can I run some ideas past you as a sort of crackfulness check please? [08:09] fwereade: please do [08:09] rogpeppe, stop me when you detect the whiff ;) [08:09] * rogpeppe sniffs carefully [08:10] rogpeppe, RelatedService is a good name for ServiceRelation; and its role and scope should be taken from the expected endpoint (of the service it's describing) [08:11] rogpeppe, RelatedService.RelationName(), however, should return the RelationName of the endpoint at *this* end of the relation, because that's how the related service is seen by the "local" service [08:11] fwereade: remind me what ServiceRelation represents again... [08:11] rogpeppe, ie the one on which we called RelatedServices() to get that RelatedService [08:11] rogpeppe, my best guess is that it represents what I just described [08:12] rogpeppe, but there's definitely something hinky about the types in play in the python [08:12] rogpeppe, I'm trying to come up with something that actually fits what we need to do [08:12] i haven't looked at those types. i think i had a mental block at "UnitRelation" which gustavo's renaming has helped with [08:13] rogpeppe, I'm actually starting to think that UnitRelation was a real misstep, that we don't want at all [08:13] rogpeppe, but that's a bit further down the crackfulness list [08:13] fwereade: we're talking about types that exist to hold the settings, right? [08:14] rogpeppe, well, zookeeper holds the settings... and I don't think anybody except the unit agent has any right to be looking directly at those settings any more [08:14] fwereade: because there's one collection of settings for each service for each relation for each unit [08:14] rogpeppe, yes [08:14] rogpeppe, but nobody should be looking at them directly IMO [08:15] fwereade: are these types not there specifically to enable what the unit agent needs to do? [08:15] rogpeppe, because they run the risk of destabilising the space-time continuum by violating causality [08:15] rogpeppe, *kinda* [08:15] fwereade: how is the unit agent going to read and change those settings without an API in state? [08:18] gaaah [08:19] lol [08:19] rogpeppe, did you see any of that, about RelatedGroup? [08:19] fwereade_: nope [08:19] rogpeppe, I'm going to return to my sorta-thread of ideas briefly, I think what I'm saying will make more sense with context [08:19] rogpeppe, next: RelatedGroup is a useful concept that deserves its own (simple) type [08:19] rogpeppe, a RelatedGroup really just wraps a path that's either /relations/rel-id or /relations/rel-id/container-id [08:19] rogpeppe, but it's a lot more convenient to pass around one of those and specify things like group.settingsPath(RoleProvider, unitId) [08:19] rogpeppe, ...thn it is to hand-hack those paths everywhere [08:19] fwereade_: last i saw from you was "*kinda*" BTW [08:19] rogpeppe, cool, I picked the right join point :) [08:20] rogpeppe, so, re RelatedGroup, this is IMO a useful concept that we didn't previously have a name for [08:20] rogpeppe, it's really the set of units that can (transitively) affect one another within a relation [08:21] rogpeppe, in a global relation it's just all units [08:21] rogpeppe, in a container-scoped one there's a RelatedGroup per container [08:21] fwereade_: so what kind of operations are you imagining on a RelatedGroup? [08:22] rogpeppe, .presencePath() and .settingsPath() mainly [08:22] fwereade_: is there anywhere in the current code that would be changed to use it? [08:22] rogpeppe, also perhaps .prepareJoin(role), which ensures that nodes have been created [08:23] rogpeppe, relationUnitWatcher would directly take presencePath and settingsPath which had been calculated from a RelatedGroup [08:23] rogpeppe, and finally, maybe, .Watch(...) [08:24] rogpeppe, but it is true that I have not yet figured out exactly what data the various bits of behaviour are best attached to [08:24] fwereade_: just so i have an idea, what would the type definition look like? [08:24] rogpeppe, it really just wraps a path [08:24] rogpeppe, that's it [08:24] fwereade_: type RelatedGroup string ? [08:25] rogpeppe, it'll need a zk.Conn, or State, or something too [08:25] rogpeppe, this is what I have atm: [08:25] / RelatedGroup represents the set of units within a specific relation that [08:25] / can (transitively) affect one another. For a globally-scoped relation, this [08:25] / includes all units of all services in the relation; for a container-scoped [08:25] / relation, there will be one RelatedGroup per container, containing all [08:25] / units of the related services that are running within that container. [08:25] type RelatedGroup struct { [08:26] zk *zookeeper.Conn [08:26] path string [08:26] } [08:27] rogpeppe, it's not a lot of data, but it draws together a lot of special-casing that would otherwise be smeared across the codebase [08:27] rogpeppe, there are several places in python that act differently depending on relation scope [08:27] fwereade_: could you point to a few examples in the python code, so i have an idea? [08:27] rogpeppe, just a sec [08:32] rogpeppe, oddly enough, mainly in state/relation.py -- there are several `if self._relation_scope == "container"`s [08:32] rogpeppe, there's a _self.container_path which seems kinda redundant but is also related [08:33] rogpeppe, basically everything involving relation unit presence or settings, whether reading or writing, needs to care about this concept (implicitly or explicitly) [08:34] fwereade_: i think i need to be reminded of how relations are laid out in zk [08:34] rogpeppe, I have vocab that will make things easier :) [08:35] rogpeppe, a "group" has (1) a settings subnode, which contains a unit-key-keyed node for every unit in the relation [08:36] rogpeppe, (2) a role subnode for each role in play in the relation, which contains a unit-key-keyed presence node for every unit actively playing that role in the relation [08:36] rogpeppe, sorry, in the *group* [08:36] rogpeppe, a globally coped relation contains just one group, which is the relation node in ZK [08:37] rogpeppe, a container-scoped relation has N container-key-keyed groups within the relation node [08:37] rogpeppe, the group is the unit of... er, influence? [08:38] rogpeppe, I am still slightly scrabbling around for vocabulary; let me know if you come up with improvements ;) [08:38] fwereade_: how about RelationScope ? [08:39] rogpeppe, yeah, except the name is taken and I don't know what to call the thing that already has that name [08:39] rogpeppe, and all the existing methods called RelationScope [08:39] fwereade_: well, we're actually talking about the same thing, right? [08:40] rogpeppe, not really, RelationScope is just "container" or "global" [08:40] rogpeppe, the thing we're talking about is about what units are actually within a given scope of a given relation [08:40] fwereade_: aren't those the two kinds of things you can have in a RelatedGroup? [08:41] fwereade_: it seems to me that the path in RelatedGroup is exactly expressing the scope of the relation [08:41] rogpeppe, I strongly agree that RelatedGroup is, intuitively, a "scope" [08:41] fwereade_: so, can we think of a way to join the two concepts? [08:41] rogpeppe, but I feel somewhat constrained naming-wise [08:41] rogpeppe, I hope so :) [08:42] rogpeppe, one possibility is to rename RelationScope to RelationScopeKind or something [08:42] fwereade_: vague thought: i wonder if a relation could return a RelationScope which looks like your RelatedGroup, but has a Kind method which returns what is currently RelationScope [08:43] ha! [08:43] rogpeppe, then I can [08:43] fwereade_: jinx, kinda [08:43] rogpeppe, the trouble is that I cannot necessarily just go from a relation to a relation scope, without knowing the container I'm talking about (in the case of a container-scoped relation, anyway) [08:44] rogpeppe, also there's no such type as a Relation ATM [08:44] rogpeppe, I'll add one if it seems worthwhile ofc ;) [08:44] fwereade_: there is actually [08:44] fwereade_: see relation.go:/^type Relation [08:45] rogpeppe, the way I currently have to get to a group is RelatedService.group(u *Unit) [08:45] rogpeppe, I deleted it [08:45] fwereade_: ah. [08:45] rogpeppe, it's entirely useless [08:46] rogpeppe, anyway, can we agree that I'm not *obviously* high, yet, although there cretainly are naming issues to be sorted out here? [08:47] fwereade_: i definitely think there's something to what you're saying, but i don't think it's just naming that's at issue. [08:47] rogpeppe, because the next step is to point out that a RelatedUnitsWatcher could be, in terms of vocabulary we've been discussing, a GroupRoleWatcher [08:47] rogpeppe, ah, ok, let's back up then [08:48] fwereade_: or rather, naming has semantic implications too [08:48] rogpeppe, oh hell yes [08:49] rogpeppe, I'm kinda hoping that naming fixes will fall out of the discussion [08:49] fwereade_: not sure about GroupRoleWatcher - that sounds like it's watching for changes in group roles [08:49] rogpeppe, for example with the scope idea that becomes a ScopeRoleWatcher, which feels much closer to what it's really doing [08:49] fwereade_: but it is actually watching for related units, no? [08:50] rogpeppe, yeah, it could indeed just be called a RelatedUnitsWatcher [08:50] rogpeppe, but it feels slightly important to make it clear that it's scoped [08:50] fwereade_: just that RelatedUnitsWatcher watches for related units within a particular scope, no? [08:51] rogpeppe, yeah, it's also reasonable to just ignore that entirely -- pass in a group/scope and not mention it at all in the type name [08:51] rogpeppe, probably a better idea all things considered [08:51] fwereade_: if we create the RelatedUnitsWatcher from a RelationScope, that would be obvious, perhaps [08:52] rogpeppe, yeah [08:53] rogpeppe, ok stepping back again a mo [08:53] fwereade_: type ScopeKind string? [08:54] rogpeppe, yeah, +1 [08:54] rogpeppe, a unit agent has a unit [08:54] rogpeppe, it can find out its service, and fro that get some RelatedServices [08:55] rogpeppe, given a unit and a RelatedService we can figure out the Scope [08:55] func (r *ServiceRelation) Scope(unit *Unit) RelationScope [08:55] rogpeppe, a Scope is useful both for watching related units and for signalling the unit agent's presence within the relation [08:55] rogpeppe, yeah [08:56] for globally scoped relations, unit could be nil [08:56] rogpeppe, doesn't matter I think [08:56] fwereade_: that's true. [08:56] i think [08:57] rogpeppe, I think the question we're asking is "what scope will this unit be in if it joins, or please give me an error of the unit can't join" [08:57] and finally... [08:57] rogpeppe, watching related units, and signalling ones presence in the relation, are profoundly intimately bound up [08:57] rogpeppe, such that we don't want a unit agent ever doing just one of those things [08:58] rogpeppe, by signalling presence, it's advertising that it's also watching and responding to the related units [08:59] sorry, parcel just arrived [08:59] rogpeppe, and I *think* the right high-level thing for this is `func (u*Unit) AgentJoin(s *RelatedService) (*SOMETHING, error)` [08:59] back now though [09:00] rogpeppe, where that SOMETHING is... I dunno, I've been calling it a JoinedService, but that's obviously wrong [09:00] rogpeppe, JoinedScope might be better [09:01] rogpeppe, (it's purpose is (1) to provide access to a RelatedUnitsWatcher's Changes channel, and (2) to maintain a pinger on the approriate path so that SOMETHINGs on other unit agents detect it) [09:01] rogpeppe, and if either of those tasks fail both should fail [09:01] func (u*Unit) AgentJoin(s *RelatedService) (*presence.Pinger, *someWatcherType, error) ? [09:02] rogpeppe, -1 [09:02] rogpeppe, if one of those things dies the other should too [09:02] rogpeppe, if anything that type is a RelationUnit [09:03] rogpeppe, but it's something whose mere *existence* has broad impact on other processes [09:03] rogpeppe, so I'm very reluctant to use that name ;) [09:04] rogpeppe, ScopePresence? JoinedRelation? JoinedScope? JoinedService? [09:04] rogpeppe, Joined is IMO a good word [09:04] fwereade_: i don't mind if we let the unit agent do some work, rather than pushing everything into a state type [09:04] rogpeppe, the unit agent will be doing *plently* of work [09:05] fwereade_: it's very easy for the unit agent to do: defer pinger.Stop() [09:05] rogpeppe, and to react to its death as well? [09:05] rogpeppe, to clean up one if the other dies, and vice versa? [09:05] rogpeppe, sound to me like a job for a type honestly [09:05] fwereade_: yeah, we'll already have a loop with a select in [09:07] rogpeppe, and it will also be simple to express Depart() and Abandon()? [09:07] fwereade_: but i'm sure a type could work well too [09:10] rogpeppe, ok, I think I'll go with ScopeKind string and RelationScope struct {*zk.Conn, string} [09:10] rogpeppe, see what falls out of that [09:10] fwereade_: i think that sounds good. [09:12] rogpeppe, TheMue: since I'm making this change, I wonder how you feel about s/RelationRole/RoleKind/ throughout as well [09:12] rogpeppe, TheMue: for consistency's sake if nothing else [09:13] fwereade_: that sounds good to me too [09:13] rogpeppe, TheMue: (also, for methods/fields currently called RelationRole/RelationScope, name them Role and Scope unless that causes ambiguity?) [09:13] fwereade_: and i'm thinking that the methods on ServiceRelation don't really benefit from having "Relation" prefixes [09:13] jinx [09:13] rogpeppe, RelationName kinda does [09:14] fwereade_: i don't mind ServiceRelation.Name [09:14] rogpeppe, but i want to call that type RelatedService :) [09:14] ah [09:15] rogpeppe, which IMO has more appropriate connotations [09:15] rogpeppe, but then, hmm. [09:15] fwereade_: in which case, yeah, RelationName is better [09:15] rogpeppe, similarly, in that case, RelationScope [09:16] rogpeppe, but Role is something that really is on the service not the relation so that should change [09:16] rogpeppe, it's more things like [09:16] fwereade_: i don't mind Scope on RelatedService actually [09:16] fwereade_: because services don't have scopes [09:16] rogpeppe, true, and the scope is shared by both ends of the relation [09:16] fwereade_: so it's obvious that it's about the Related bit of the name [09:17] rogpeppe, cool [09:17] fwereade_: i vote for a small CL cleaning up these issues, then another one renaming RelationScope to ScopeKind [09:18] rogpeppe, yeah, that sounds sensible [09:18] fwereade_: as they're orthogonal and the latter is more controversial [09:18] rogpeppe, I'll think on it a touch more though [09:18] k [09:19] rogpeppe, most of this morning's conversation is *really* about me figuring what order I can make CLs in to maximise my chances of getting all this stuff in ;) [09:19] fwereade_: ah, but you've now changed *what* you're gonna put in! [09:20] fwereade_: and i, for one, think it's a really nice step forward [09:20] rogpeppe, the types haven't changed, I think, just the names :) [09:20] fwereade_: You know the current Py code best. If there's no conflict with Role and Scope I'm fine with it. [09:20] rogpeppe, (from what I was originally intending, that is; it's definitely changed from the py in some ways ;)) [09:20] fwereade_: having ScopeKind hanging off RelationScope is new, i think [09:22] fwereade_: yeah. but the names make so much difference. when it was "RelatedGroup" i was, like, "where are the things in the group?". but now it's a "RelationScope", i see it as a container and it makes sense to me. [09:23] rogpeppe, yeah, tyvm for helping out with that [09:23] fwereade_: even though the underlying representation might be identical [09:23] rogpeppe, that was what I was looking for, and what I got :) [09:24] fwereade_: np. i would never have got as far as you did! [09:24] rogpeppe, I'm lucky to have undergone the baptism by fire that was the restartable-unit-agent work in the python [09:24] rogpeppe, that was enough that figuring out the real details here was.. possible [09:25] fwereade_: just had a silly naming idea [09:25] rogpeppe, oo, go on [09:26] fwereade_: so we've got these "scopes" right? so when we've got a language implementation, what do we call the actual implementation of scopes? "frames". [09:26] fwereade_: sadly it doesn't really work [09:26] rogpeppe, I don't think it quite does :( [09:26] rogpeppe, I culd call them continuations I suppose, if I really want someone to stab me [09:27] fwereade_: actually, maybe just RelationSettings might be appropriate [09:27] rogpeppe, -1 [09:28] fwereade_: as it's the settings that are scoped [09:28] rogpeppe, (1) they're presence as well (2) the only thing that should actually be reading the settings is the RelatedUnitsWatcher anyway [09:30] fwereade_: yeah [09:30] fwereade_: i'm just slightly concerned that the word "scope" implies something abstract rather than concrete [10:10] moin [10:20] Aram: Moin. Hehe, you adopted the usual greeting here in Northern Germany. [10:29] Lunchtime [11:14] fwereade_: ping [11:15] rogpeppe, pong [11:15] fwereade_: i wonder if you can help me thinking about a good way to test this new package: http://paste.ubuntu.com/1042221/ [11:16] fwereade_: i can mock up start commands like the upstart package does [11:16] fwereade_: but i don't think it provides any useful assurance of anything [11:17] fwereade_: i'm considering a "-root" flag to the test, for tests that need to run as root. [11:17] fwereade_: then we can test it for real. [11:17] rogpeppe, yeah, it's tricky; in the python we had exactly that [11:17] fwereade_: a "-root" flag? [11:17] rogpeppe, and it always felt a little icky but I never saw any other way to really verify it [11:17] rogpeppe, well, a USE_SUDO env var [11:18] rogpeppe, but basically yes [11:18] fwereade_: it's particularly an issue for the LXC-manipulating code [11:18] rogpeppe, yeah, that's the main place we had it for [11:18] fwereade_: ok, that's useful, thanks [11:18] rogpeppe, I think I had it for the upstart stuff in the local provider as well [11:19] fwereade_: i think i'll go for a "-root" flag and require that the user id be root if it's set [11:20] rogpeppe, if you're going to require that, can't you just check for root and explicitly skip the tests that require it if not? [11:20] fwereade_: i thought of that, but i'm not sure i want the tests manipulating my global state without me explictly asking them to do so [11:20] rogpeppe, IMO by running them as root you're explicitly asking them to do so [11:21] fwereade_: sometimes i'm running a root shell without being aware of it... [11:21] fwereade_: but maybe it's ok [11:21] rogpeppe, ah, fair enough [11:21] fwereade_: and it means that i could do sudo go test ./... [11:21] rogpeppe, I *think* I've broken myself of that habit even when I really want to [11:22] * rogpeppe hates sudo [11:22] rather, i hate the requirement that so many things run as root [11:23] rogpeppe, ha, yeah [11:24] rogpeppe, TheMue: I have a thought about topoRelationService [11:24] * rogpeppe is all ears [11:24] rogpeppe, at the moment a topoRelation has Services, which holds topoRelationServices keyed on service key [11:25] rogpeppe, tRS has RelationName and Role fields [11:25] rogpeppe, each taken from the appropriate endpoint of the apropriate service [11:25] rogpeppe, I contend that even here, the relation name should be taken from the opposite endpoint [11:26] rogpeppe, because, in the context of a relation, the important thing about a service is what the *other* end thinking it's called [11:26] fwereade_: what is the relation name BTW? is it a key? [11:26] rogpeppe, *db*-relation-joined [11:27] rogpeppe, *blog*-relation-changed [11:27] rogpeppe, etc [11:27] fwereade_: ok, thought so, just checking [11:27] rogpeppe, it's the name used within a charm to refer to the other end of the relation [11:28] rogpeppe, so it seems stupid for a service to store (1) what it does and (2) what it thinks the other end's called [11:28] rogpeppe, when it could store (1) what it does and (2) what it's called (from the perspective of the rest of the relation) [11:29] rogpeppe, it's non-obvious but I'm becoming convinced that the obvious solution is crack [11:31] * rogpeppe is looking at the code [11:32] TheMue, when you return, I would also appreciate your opinion of the foregoing [11:36] fwereade_: hmm. this stuff is barely used at the moment. how do you anticipate it being used? [11:37] fwereade_: (currently there's just a single call to topology.Relation [11:37] ) [11:37] rogpeppe, well, I have this idea the a RelatedService is {relationKey string, serviceKey string, relater RelationEndpoint} [11:38] fwereade_: what's an "endpoint" again? [11:38] rogpeppe, where "relater" is the endpoint of the service at the "local" end of the relation [11:38] rogpeppe, relation name, interface, role, scope [11:38] ok [11:39] rogpeppe, oh and service name [11:41] fwereade_: am i wrong that the only thing in that endpoint tuple that can't be derived from relationKey is the role? [11:41] rogpeppe, also the name [11:42] fwereade_: that comes from the relationKey and role too, doesn't it [11:42] rogpeppe, which needs to come from the topoRelationService, and which I contend is the wrong way round at the moment [11:42] rogpeppe, but, yes; it's pretty trivial to construct the RelatedService above, given a topology and a relation key [11:42] fwereade_: i'm thinking that a RelatedService is more {relationKey string, role RelationRole} [11:43] rogpeppe, sure, it could be, but the fields I described are the ones that are (1) convenient to access when constructing and (2) used subsequently [11:44] fwereade_: i'm trying to think fundamentals, not optimisation. [11:44] rogpeppe, if we only stored what you suggest it would be doable but a bloody hassle [11:44] fwereade_: really? [11:45] fwereade_: well, of course it'd need to store the topology too [11:45] rogpeppe, rooting around in the topology, looping through relation Services dicts trying to find the data it's looking for [11:45] rogpeppe, I'd really much rather collect all the relevant info than the bare minimum necessary to infer that info [11:45] rogpeppe, (otherwise why have types at all? :p) [11:46] rogpeppe, this would be problematic if relations could change after being created, but the fact of a relation and its endpoints is not something that changes over the lifetime of that relation [11:46] fwereade_: i guess i was trying to get to the bottom of what is *necessary* for a RelatedService [11:47] rogpeppe, it could equally be relationKey, serviceKey [11:48] rogpeppe, we can infer role from service+relation just as we can infer service from relation+role [11:48] fwereade_: ah, that seems more logical, given the name [11:48] fwereade_: ah, except that relationKey isn't visible outside state [11:49] rogpeppe, we don't have to expose it [11:49] ;) [11:50] rogpeppe, I don't think we really want to expose much on a RelatedService at all tbh [11:50] fwereade_: what do we get from RelatedService that we don't get from Scope? [11:50] rogpeppe, I certainly don't need anything public *yet* [11:51] rogpeppe, scope covers both ends [11:51] rogpeppe, relatedservice is about one end only [11:51] fwereade_: i'm wondering about: func (svc *Service) Relation(u *Unit) *Scope [11:51] ah [11:51] rogpeppe, why would I care about that? [11:52] rogpeppe, the only actual use case is for the unit agent to loop over the related services and respond to changes in each [11:52] rogpeppe, for that I just want a list of RelatedServices, each of which describes what I need to know to join a relation with it [11:52] * rogpeppe is starting to see [11:53] * fwereade_ is hugely relieved [11:53] rogpeppe, I've been a bit nervous about all this -- it is definitely different to the python, but I think it is a projection of the same problem into a simpler space [11:53] :-) [11:54] rogpeppe, and it's far from immediately obvious that that's the case ;) [11:54] fwereade_: the zk representation is pretty much the same though, right? [11:55] rogpeppe, I need to make another pass over the topology code, but yes, I think so [11:55] rogpeppe, the zk representation of the actual live relations is identical [11:55] rogpeppe, but I think the code representation of that representation is much nicer like this [11:56] fwereade_: dumb question: why isn't a RelatedService just a Service? [11:56] rogpeppe, I think that it will lead to a much cleaner clearer data flow [11:56] rogpeppe, because a service doesn't in itself have a role/relname/scope [11:56] rogpeppe, it has a whole bunch of relations, only one of which we're interested in at a time [11:57] fwereade_: but you could have something like: func (svc *Service) Join(*Service) error, though, no? [11:57] rogpeppe, I *think* that it's units which have to join services [11:58] fwereade_: oh yeah [11:58] rogpeppe, but I may be missing context... what would that do? [11:58] rogpeppe, at first glance that really looks like a different version of AddRelation(endpoints) [11:59] rogpeppe, but not specific enough to do anything, because there may be more than one valid pair of endpoints between the services [11:59] fwereade_: yeah, i've realised that [11:59] fwereade_: i'm just wondering if there's some way of doing without RelatedService [12:00] rogpeppe, I thought there was at one stage but the code said it wanted it [12:00] rogpeppe, (I understood the code to be saying it wanted it ;)) [12:00] fwereade_: so i'm just trying to think about what the unit agent is doing [12:01] rogpeppe, I'm expecting pseudocode: [12:02] rogpeppe, for rs in self.unit.service.related_services: self.unit.AgentJoin(rs) [12:02] rogpeppe, and therefrom marshal the changes coming out of the JoinedService, or whatever we call it, into actual hook executions [12:02] fwereade_: i was toying with the idea of: func (u *Unit) Join(svc1, svc2 *Service) [12:02] rogpeppe, which is a separate and not entirely trivial thing [12:03] rogpeppe, what are the two services for? [12:03] rogpeppe, surely we can infer one of those from the unit [12:03]