[06:59] <TheMue> Good morning.
[07:35] <fwereade_> TheMue, heyhey
[07:35] <TheMue> fwereade_: Hiya
[07:53] <TheMue> fwereade_: Will push the first CL for the firewaller in a few moments. It contains the port init in global mode for machined. Looks good.
[07:56] <fwereade_> TheMue, cool
[07:56] <fwereade_> TheMue, I'll take a look when it's there :)
[07:56] <TheMue> fwereade_: I'll notify you, it's a small one.
[08:05] <TheMue> fwereade_: Here you are: https://codereview.appspot.com/6875053/
[08:33] <rogpeppe1> mornin' folks
[08:35] <fwereade_> rogpeppe1, heyhey
[09:05] <TheMue> rogpeppe: Hi
[09:06] <TheMue> rogpeppe: Would you also take a look at https://codereview.appspot.com/6875053/ ?
[09:06] <rogpeppe> TheMue: will do
[09:06] <TheMue> rogpeppe: Thx
[10:02] <fwereade_> TheMue, afaict this change makes the window smaller, but does not fix the bug... please remind me what the second branch will be?
[10:04] <TheMue> fwereade_: The second one will create the unitd's when a machined is created before returning to the main firewaller loop.
[10:04] <TheMue> fwereade_: See http://irclogs.ubuntu.com/2012/11/30/%23juju-dev.html at the end.
[10:17] <fwereade_> TheMue, how will that change affect this one?
[10:17] <fwereade_> TheMue, ah, ok I get it now -- we're never sending port *changes* out of the unitds, just the current set of ports
[10:18] <fwereade_> TheMue, and so the window doesn't matter, so long as we don't hold inconsistent data
[10:19] <fwereade_> TheMue, but wait...
[10:20] <fwereade_> TheMue, isn't the window between initialization of globalPortOpen and globalPortsRef still enough to cause the bug?
[10:21] <TheMue> fwereade_: Good question, that I asked myself.
[10:23] <fwereade_> TheMue, I think it's just narrowed the window to the time between initGlobalPorts and the relevant machineLifeChanged
[10:23] <TheMue> fwereade_: I already thought about a solution where the whole tree of machined, unitd and serviced is initially started when the firewaller starts.
[10:24] <fwereade_> TheMue, I think that's the only viable solution, isn't it?
[10:24] <fwereade_> TheMue, saves an awful lot of bandwidth too ;)
[10:24] <fwereade_> TheMue, but I agree that it is not entirely a trivial solution
[10:25] <TheMue> fwereade_: Yes, and it works independent of the mode. In case of a watcher event it's checked that the according *d is started. If not then it's done then.
[10:26] <fwereade_> TheMue, ah, maybe I'm confused... ISTM that the only generally sane way to do it is to start watchers and consume their initial events to build up the state *before* handling anything in the FW's main loop
[10:28] <TheMue> fwereade_: Oh, no, that's not the idea. You see today in global mode that we scan the state once. And that scan could also be used to start all machind's, unitd's and serviced's. And later events only check if they are already covered (*d exists) or if it's a change (add/remove).
[10:29] <fwereade_> TheMue, ah, great, I misunderstood :)
[10:29] <TheMue> fwereade_: So before entering the loop the fw is up and running. ;)
[10:34]  * TheMue steps out for a few minutes, have to fetch medicine from the pharmacy. 
[10:54] <Aram> morning.
[11:06] <jam> morning Aram
[11:26] <niemeyer> Good morning juju-devs
[11:28] <fwereade_> niemeyer, heyhey
[11:30] <TheMue> niemeyer: Hiya
[11:39] <TheMue> niemeyer: The first firewaller CL regarding the machined.ports is in. But fwereade_ and I still see that there's a window between the init of the global ports and the machine life events. So even if the current branch runs stable so far testing it multiple times in a loop there's still a risk.
[11:40] <TheMue> niemeyer: What do you think about the idea of starting all machined's etc initially, before entering the firewaller loop?
[11:41] <TheMue> niemeyer: So they are not lazy started by the initial watcher events. The events are then used to check if the goroutine for the entity is already running, if it has to ben started or if it has to be stopped and removed.
[11:43] <mgz> wallyworld_: looks from the nova code that what gets used when filtering is just whatever webob gives for GET:
[11:43] <mgz> search_opts = {}
[11:43] <mgz> search_opts.update(req.GET)
[11:43] <niemeyer> TheMue: So there were two different changes, right?
[11:43] <mgz> ...
[11:44] <mgz> status = search_opts.pop('status', None)
[11:44] <niemeyer> TheMue: When you say you and William still see a window, is that the window we discussed previously?
[11:44] <wallyworld_> mgz: so if the request params have more than one status value?
[11:44] <niemeyer> TheMue: and that was supposed to be the second CL?
[11:45] <mgz> ah, webob has a neat MultiDict thing... which is not used by this code
[11:45] <wallyworld_> \o/
[11:45] <wallyworld_> not :-(
[11:45] <mgz> wallyworld_: so, it will just pick one at dictionary order random it seems
[11:45] <TheMue> niemeyer: We discussed it today. The second CL will add the init of the unitd's for each machined. But that starts with the first events for machines. And there is still a gap until those events arrive.
[11:45] <wallyworld_> awesome
[11:46] <TheMue> niemeyer: The both CLs narrow it, but it's still a window there.
[11:46] <niemeyer> TheMue, fwereade_: Isn't that exactly what we discussed before?
[11:46] <wallyworld_> mgz: so until the openstack side of things does something sensible, we are hamstrung
[11:46] <niemeyer> TheMue: What's the gap?
[11:47] <mgz> right, I'm almost tempted to say supporting this level of (server-side) filtering is not useful
[11:48] <wallyworld_> well, i think it may have some use
[11:48] <TheMue> niemeyer: Then I maybe misinterpreted it. I now init machind.ports when a new machined is created. And that is done when a machine event is received.
[11:48] <TheMue> niemeyer: Did you meant to immediatelly start the machined? Then I'll change it and I'm happy. ;)
[11:48] <niemeyer> TheMue: Initialized from where?
[11:49] <niemeyer> TheMue: No, I'm still trying to understand what's the issue.. I can't possibly make a suggestion before that :)
[11:50] <TheMue> niemeyer: The machined is started in machineLifeChanged(). In global mode it retrieves the machines units and their ports to init the ports field and the ref count.
[11:51] <TheMue> niemeyer: It's so far in https://codereview.appspot.com/6875053/.
[11:52] <niemeyer> TheMue: Right, there's already a moment where you obtain all open ports for all machines in global mode, which means we already know what all the open ports are, and all the units, and all the machines.
[11:53] <niemeyer> TheMue: Why are we doing this a second time, with less information?
[11:54] <TheMue> niemeyer: Yes, that's why I thought about starting the machined's already there. I would like to do so independent of the mode.
[11:57] <niemeyer> TheMue: It's not independent of the mode, because currently open ports, and how to open or close the ports, depends on the mode
[11:57] <niemeyer> TheMue: And so does port referencing
[11:58] <niemeyer> TheMue: Do you understand what the gap that William described actually is?
[11:59] <TheMue> niemeyer: OK, I have to specify it more. Starting the machined's etc would be done based on the initial state but for sure the port handling depends on the mode.
[12:00] <TheMue> niemeyer: The gap is between the init of the global ports and the first machine life event. Here it may happen that the status of the firewaller doesn't match to the reallity anymore.
[12:01] <niemeyer> TheMue: Why is that a problem? How does the bug take place?
[12:05] <niemeyer> TheMue?
[12:05] <TheMue> niemeyer: Yeah, still thinking.
[12:06] <TheMue> niemeyer: With the CL a machined retrieves it's correct status regarding the ports (but with addition I/O).
[12:09] <niemeyer> TheMue: It's still not clear what you think the problem is, and how the bug takes place
[12:10] <TheMue> niemeyer: I've got to admit that I'm not sure anymore.
[12:11] <TheMue> niemeyer: I'm currently walking different paths through the state (order of adding/removing units) to see where it would break.
[12:11] <fwereade_> TheMue, niemeyer: I could easily be wrong -- but ISTM that the fundamental problem is that the port refcounts are initialised using different data to that used to initialize their openness
[12:13] <niemeyer> TheMue: I think it's important to understand the semantics of the problem itself so we can drive towards a good solution
[12:13] <TheMue> fwereade_: Yes, I already looked here too. But the problem is to be sure, that a later change has already been catched by the initial state scanning or if it is an independent event.
[12:14] <niemeyer> TheMue: I don't quite see what that means
[12:15] <niemeyer> TheMue: The problem is simpler than it sounds
[12:15] <TheMue> niemeyer: Any hint is welcome.
[12:15] <fwereade_> TheMue, isn't the problem that the initial scan does not fully initialize the firewaller? the ideal is that *any* time you get an event in from the watcher you can check it against known-sane, and complete, state
[12:15] <niemeyer> TheMue: There's a point in the system where we observe the actual state, as the provider reports it
[12:16] <niemeyer> TheMue: Can you pinpoint that location?
[12:16] <TheMue> fwereade_: That's my idea to fully initialize it before going into event handling. I wrote it on Friday.
[12:16] <fwereade_> TheMue, ok, but the state you use to initialise it must itself be consistent
[12:17] <fwereade_> TheMue, you can't get separate sets of ports in spearate places and assume they match
[12:17] <TheMue> fwereade_: Exactly.
[12:18] <TheMue> niemeyer: Sorry, can't follow.
[12:18] <TheMue> niemeyer: We start by watching the machines.
[12:18] <niemeyer> TheMue: No, we don't
[12:19] <niemeyer> TheMue: There's a point in the system where we observe the actual ports that are open, as the provider reports it. Can you pinpoint that location?
[12:20] <TheMue> niemeyer: We retrieve it from Environ.OpenPorts() and INstance.OpenPort(), depending of the mode.
[12:21] <niemeyer> TheMue: Yes, let's leave aside instance mode for a while
[12:21] <niemeyer> TheMue: What's the line?
[12:22] <TheMue> niemeyer: Eh, in the code?
[12:22] <niemeyer> TheMue: Yes, in the code
[12:23] <TheMue> niemeyer: It's in Firewaller.initGlobalMode(), used to check which ports are already open.
[12:23] <niemeyer> TheMue: What's the line number?
[12:23] <TheMue> niemeyer: In the CL it's 197.
[12:23] <niemeyer> TheMue: No, it's not
[12:23] <TheMue> niemeyer: Exactly, just seen. On moment.
[12:24] <TheMue> niemeyer: I meant Ports() in 165.
[12:24] <TheMue> niemeyer: The other one opens them.
[12:24] <niemeyer> TheMue: Very well, now let's say that port 123 is open there
[12:24] <niemeyer> TheMue: How do we tell we should close it or not?
[12:26] <TheMue> niemeyer: After collecting the open ports in state we are diffing to see, which one are to open and which one are to close.
[12:26] <niemeyer> TheMue: Cool
[12:27] <niemeyer> TheMue: Now, let's say that we finish running initGlobalMode.. port 123 was still open at that time, but the unit closes it right then, at the end of initGlobalMode
[12:27] <niemeyer> TheMue: When do we close port 123?
[12:29] <TheMue> niemeyer: We should do it after the last unit using 123 is removed, but at that moment we don't watch the units and so we don't get aware that its ports isn't needed anymore.
[12:32] <niemeyer> TheMue: THat's not it
[12:32] <niemeyer> TheMue: We do watch the units
[12:34] <niemeyer> TheMue: Why is it not working even though we do watch the units?
[12:34] <TheMue> niemeyer: When the machined is started.
[12:35] <TheMue> niemeyer: The machined is watching its units, but at that moment the machined isn't running, so it doesn't watch the unit.
[12:36] <TheMue> niemeyer: Or what do I miss?
[12:36] <niemeyer> TheMue: Nope.. that's not it
[12:36] <niemeyer> TheMue: That's exactly what I'm trying to figure
[12:37] <niemeyer> TheMue: Port 123 is in initial ports.. what else knows that port 123 is open?
[12:38] <TheMue> niemeyer: IMHO per machined in line 495 we start watching the units of the machine.
[12:39] <TheMue> niemeyer: The state knows. That's why we compare the environment to the state.
[12:39] <niemeyer> TheMue: No, it doesn't
[12:39] <niemeyer> TheMue: The unit has closed port 123 at the end of initGlobalMode, remember?
[12:40] <TheMue> niemeyer: Yes.
[12:43] <niemeyer> TheMue: So.. port 123 is open.. and it shouldn't be.. how do we know to close it
[12:46] <TheMue> niemeyer: Hmm, if we watch the unit we should receive an event and look what parts a unit has opened.
[12:46] <TheMue> niemeyer: That's line 573.
[12:47] <niemeyer> TheMue: Will port 123 be there?
[12:48] <TheMue> niemeyer: A correct unitd should know it still, OpenedPorts() doesn't return it, so the change will be sent to the Firewaller.
[12:49] <niemeyer> TheMue: Why? Where is port 123 in that logic?
[12:51] <TheMue> niemeyer: unitData has a slice with the ports it uses. If it receives a unit change, and closing port 123 is a unit change, it retrieves the opened ports from state (without 123). That will be sent to the firewaller.
[12:54] <niemeyer> TheMue: Which variable/code line will we find port 123 in?
[12:54] <TheMue> niemeyer: And via the chain flushUnits() -> flushMachine() -> flushGlobalPorts() a diff is done and 123 should be closed.
[12:56] <TheMue> niemeyer: change in 573 doesn't contain it anymore while fw.globalOpenPorts still has it as open.
[12:57] <niemeyer> TheMue: and why does it matter? See line 574
[12:59] <TheMue> niemeyer: unitd knows its ports and the opened ones in state are not the same.
[12:59] <niemeyer> TheMue: Why are they not the same?  Port 123 was closed in the state at the end of initGobalMode, remember?
[13:00] <TheMue> niemeyer: Just found by that way that we dont use unitData.ports here.
 TheMue: Why are they not the same?  Port 123 was closed in the state at the end of initGobalMode, remember?
[13:03] <TheMue> niemeyer: So th first event of the unit watch, which is after the closing of port 123, initializes it without that port, they are the same and nothing is raised.
[13:04] <TheMue> niemeyer: Why did you repeated the sentence?
[13:04] <niemeyer> TheMue: Because it's been four minutes
[13:05] <TheMue> niemeyer: OK.
[13:05] <TheMue> niemeyer: Will remember.
[13:06] <niemeyer> TheMue: So, is the issue clear now?
[13:07] <TheMue> niemeyer: I hope so. But that's exactly why I wanted to start the machined etc before the loop is starting.
[13:08] <TheMue> niemeyer: With the data from state.
[13:08]  * fwereade_ => lunch
[13:08]  * TheMue 's family wait for him to come to lunch too.
[13:08] <niemeyer> TheMue: Argh
[13:09] <niemeyer> TheMue: The issue isn't starting machined early with data from state
[13:09] <niemeyer> TheMue: Okay, have a good lunch..
[13:09] <niemeyer> fwereade_: And you too
[13:10] <TheMue> niemeyer: Maybe the sentence is just to short and I'll outline it after lunch.
[13:10] <niemeyer> TheMue: No, it's missing the point..
[13:10] <niemeyer> TheMue: The bug isn't about how early you start machined with data from state
[13:11] <niemeyer> TheMue: The bug is the lack of correlation with the known open ports
[13:11] <niemeyer> TheMue: That's what we spent the last 2 hours going over
[13:13] <TheMue> niemeyer: Last sentence before lunch: The bug is clear and I thank you for the help, really. It's only my thought of how to solve it.
[13:13] <niemeyer> TheMue: Glad to hear it, thanks
[13:37] <TheMue> So, back again.
[14:13] <niemeyer> Lunch time here too
[14:27] <TheMue> niemeyer: Enjoy.
[14:41] <fwereade> niemeyer, btw, I am having a spot of bother with AddUnitSubordinateTo -- I don't really think that's the right API
[14:41] <fwereade> niemeyer, I'm leaning towards something like RelationUnit.EnsureSubordinate instead
[14:41] <fwereade> niemeyer, does that strike you as obviously mad?
[14:42] <fwereade> oh, bother, lunch
[14:42] <fwereade> TheMue, rogpeppe: sniff test on the above?
[14:47] <TheMue> fwereade: Yes, second one reads better.
[14:47] <fwereade> TheMue, cheers
[14:54] <niemeyer> fwereade: I don't have as much context as you do. Is there any motivation for the change?
[14:56] <fwereade> niemeyer, the motivation is (1) I think EnsureSubordinate is the natural way to do this (2) nobody uses AddUnitSubordinateTo (3) the name AddUnitSubordinateTo (to me) implies that failure to add a unit should be an error, while EnsureSubordinate implies that it's fine if a subordinate already exists
[14:57] <niemeyer> fwereade: (2) is somewhat obvious, given that the feature is in development
[14:57] <fwereade> niemeyer, (ok, tests use AddUnitSubordinateTo, but often they use it wrong -- it's not sane to have 2 subordinates of the same service with the same principal)
[14:57] <niemeyer> fwereade: (3) doesn't sound like an issue.. it can be a well defined error
[14:57] <niemeyer> fwereade: I find it quite appropriate that adding units is done in the same place always, and with similar interfaces
[14:58] <fwereade> niemeyer, I dunno, the gulf between a principal service and a subordinate service is pretty significant
[14:58] <niemeyer> fwereade: The two only functions that do that today are right next to one another, and they share the same implementatin
[14:58] <niemeyer> fwereade: That seems very compelling
[14:59] <fwereade> niemeyer, if I try to keep them using the same implementation, it gets ugly fast
[14:59] <fwereade> niemeyer, the ops and assertions end up reasonably different, and determining the failure is *very* different
[14:59] <niemeyer> fwereade: I'm definitely missing background.. you don't have to try to do that. It's already in place
[15:01] <fwereade> niemeyer, well
[15:01] <fwereade> niemeyer, a method called AddUnitSubordinateTo does exist
[15:01] <fwereade> niemeyer, but it doesn't have any checks for dupe subordinates
[15:02] <fwereade> niemeyer, so, really, it's not in place
[15:02] <niemeyer> fwereade: Right.. that's the assertion we talked about yesterday
[15:02] <fwereade> niemeyer, yes; that bit is easy; but correct abort handling STM to get kinda ugly
[15:02] <niemeyer> fwereade: Which seems pretty easy to add in addUnit, in the location that already exists to add a subordinate
[15:02] <niemeyer> fwereade: It won't get any less ugly if you move that logic elsewhere, it sounds like
[15:03] <niemeyer> fwereade: All the common logic for adding a unit that exists on this function sounds sensible, and necessary
[15:05] <fwereade> niemeyer, moving up a level for a sec -- are you -1 on the very notion of RelationUnit.EnsureSubordinate(), on the basis that there's already some code that does something a bit like what we want?
[15:05] <fwereade> niemeyer, or are we just arguing implementation details?
[15:06] <niemeyer> fwereade: We're surely arguing implementation details, given that we're talking about logic placement
[15:06] <fwereade> niemeyer, ok, that is not something I think I can honestly speak to until I've tried 2 or 3 different styles
[15:07] <niemeyer> fwereade: I'm -1 on duplicating logic without a clear reason
[15:07] <fwereade> niemeyer, right
[15:07] <niemeyer> fwereade: and moving it away from other logic that looks very much alike
[15:08] <niemeyer> fwereade: EnsureSubordinate sounds like something that can trivially be built upon AddUnitSubordinateTo
[15:08] <niemeyer> fwereade: err := addUnit; err == AlreadyThere { Oh, okay. }
[15:08] <fwereade> niemeyer, ok, but then we have a useful public state method and a useless one, for doing the same thing
[15:08] <fwereade> niemeyer, which feels kinda bloaty
[15:09] <fwereade> niemeyer, but perhaps you have some extra use in mind for AUST?
[15:09] <niemeyer> fwereade: Okay, so let's not have EnsureSubordinate.. because that's the trivial one that woudl duplicate logic
[15:10] <fwereade> niemeyer, can you explain the use case for the AddUnitSubordinateTo method?
[15:10] <niemeyer> fwereade: I have both AddUnit and AddUnitSubordinateTo open up in my screen, in the *same* terminal window.. they look very much alike, and share an implementation.
[15:10] <fwereade> niemeyer, I have been asking various people about this for months, and nobody has given me a reason other than "er, the python is like that"
[15:10] <niemeyer> fwereade: I don't understand the question.. the answer would be self-obvious
[15:11] <niemeyer> fwereade: It adds a unit that is a subordinate to a principal
[15:11] <fwereade> niemeyer, right, and when do we need to do this?
[15:11] <niemeyer> fwereade: When we want to add a subordinate unit
[15:12] <fwereade> niemeyer, ISTM that you are assuming that there are a multiplicity of situations in which we want to do this
[15:12] <niemeyer> fwereade: No, I'm not assuming anything..
[15:13] <niemeyer> fwereade: There's a method in state that allows adding a unit, and there's a method for adding a unit that is a subordinate of a principal..
[15:13] <niemeyer> fwereade: It sounds to me like straightforward design
[15:13] <fwereade> niemeyer, right -- expect it's seriously wrong in at least two ways
[15:14] <niemeyer> fwereade: Okay?
[15:15] <fwereade> niemeyer, and it is not a remotely convenient thing to do in the situation in which it's required
[15:15] <niemeyer> fwereade: Sorry, how is it seriously wrong, and how is it not remotely convenient
[15:16] <fwereade> niemeyer, it is seriously wrong in that there is no verification of relation state -- you can add anything to anything -- and in the way we've already discussed
[15:16] <niemeyer> fwereade: So AddUnit is completely wrong too?
[15:17] <fwereade> niemeyer, no -- AddUnit is AFAICT ok, although I haven't been looking at that side of it closely
[15:17] <niemeyer> fwereade: Why? We can add anything to anything as well, without any care about relation state
[15:17] <fwereade> niemeyer, why should AddUnit care about relation state?
[15:18] <niemeyer> fwereade: Well, there are peer relations too as well, right?
[15:18] <fwereade> niemeyer, so what?
[15:18] <niemeyer> fwereade: Well, exactly.. :)
[15:18] <niemeyer> fwereade: Subordinates aren't different..
[15:18] <fwereade> niemeyer, we've already made the decision that broken peer relations are just fine
[15:18] <fwereade> niemeyer, and that the user mustn't touch them
[15:19] <fwereade> niemeyer, I consider this to be crack, but anyway
[15:19] <fwereade> niemeyer, but still -- why would the existence or otherwise of a peer relation impact whether it's ok to add a unit?
[15:19] <niemeyer> fwereade: Sorry, I don't understand what we've decided regarding broken peer relations
[15:19] <niemeyer> fwereade: But okay, let's leave that aside
[15:19] <niemeyer> fwereade: The relation state can be completely inconsistent after you added that unit
[15:19] <niemeyer> fwereade: There should be a peer relation, and there isn't
[15:20] <niemeyer> fwereade: This will be eventually established as the uniter runs
[15:20] <fwereade> niemeyer, whaaaaa?
[15:20] <niemeyer> fwereade: I'm trying to understand why you think that's so much different from the case of subordinates
[15:21] <niemeyer> fwereade: What within AddUnitSubordinateTo and addUnit do you think shouldn't be there?
[15:21] <fwereade> niemeyer, is it OK to have one unit of mongodb deployer?
[15:21] <niemeyer> fwereade: Why is that wrong?
[15:21] <fwereade> s/deployer/deployed/
[15:22] <niemeyer> fwereade: Obviously
[15:22] <niemeyer> fwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?
[15:22] <fwereade> niemeyer, can we back up?
[15:22] <niemeyer> fwereade: That's what I'm trying to do
[15:22] <fwereade> niemeyer, because you have thrown me a complete curevball regarding peer relations
[15:23] <niemeyer> fwereade: Yes, sorry, please ignore me there
[15:23] <niemeyer> fwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?
[15:23] <fwereade> niemeyer, why is the existence or otherwise of a peer relation relevant to whether or not it's ok to add a unit of a service?
[15:23] <niemeyer> fwereade: Can we back up a bit?
[15:23] <niemeyer> fwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?
[15:24] <fwereade> niemeyer, where on earth did that question come from? I am talking about *deficiencies* in AUST
[15:24] <fwereade> niemeyer, things that are *not* there
[15:24] <niemeyer> fwereade: It came from my mind
[15:24] <fwereade> niemeyer, well, please, what does it have to do with what I have been saying?
 niemeyer, right -- expect it's seriously wrong in at least two ways
[15:25] <niemeyer> fwereade: I'm trying to understand what's so dramatically wrong about it
[15:25] <niemeyer> fwereade: Because I've been pointing out that what's there is necessary, it's close to related logic, etc
[15:25] <niemeyer> fwereade: It feels like we've been talking across each other
[15:25] <fwereade> niemeyer, ok, so what does it mean for S1 to have a subordinate of S2 when there is no relation between S1 and S2?
[15:26] <fwereade> niemeyer, and, what does it mean for S1 to have a unit when S1 is not in a peer relation?
[15:26] <niemeyer> fwereade: It means that there are two units, one is subordinate of the other, and they'll both be deployed
[15:26] <niemeyer> fwereade: S1 and S2 will live in the same container
[15:26] <niemeyer> fwereade: and unless a relation is established between them, nothing else will happen
[15:27] <fwereade> niemeyer, why would we deploy a subordinate in the first place without there being a relation between them?
[15:27] <niemeyer> fwereade: Relations are completely orthogonal to whether something is subordinate or not.. we've decided to tweak the UI to make that common so that we could explain less to users
[15:27] <niemeyer> fwereade: I believe everything actually does work, even if there's no relation between them
[15:28] <fwereade> niemeyer, I thought that the existence of any subordinate unit was predicated on a locally scoped relation between the subordinate service and the principal service?
[15:28] <niemeyer> fwereade: It's a sane building block, and one that seems to work pretty well thus far
[15:28] <niemeyer> fwereade: That's how we implement the UI, yep
[15:28] <niemeyer> fwereade: Feels great
[15:29] <fwereade> niemeyer, ok, but it sounds like you have some use case for subordinates that are not in relations with their principals?
[15:29] <niemeyer> fwereade: Maybe there's another reason why you believe OMG THATS TERRIBLE WE'LL ALL DIE if that method exists
[15:29] <niemeyer> fwereade: Which is what I've been trying to extract so far
[15:29] <niemeyer> fwereade: No, I don't have a reason to believe that the current logic is completely broken, yet.. that's all
[15:30] <niemeyer> fwereade: We have tests, we have methods.. methods work well so far.. Python seems to work as well to some degree.. etc
[15:30] <niemeyer> fwereade: You seem to think that the logic that is there is actually okay too, actually
[15:30] <niemeyer> fwereade: But there's more logic that we'll need, to make things work well
[15:30] <niemeyer> fwereade: Which sounds fine too
[15:31] <niemeyer> fwereade: So perhaps what I don't understand is the strong feeling that things are totally broken
[15:35] <niemeyer> fwereade: So, what's the case? EnsureSubordinate.. what would it do that AddUnitSubordinateTo can't be doing?
[15:39] <fwereade> niemeyer, it would hopefully have the bugs fixed; but the main point is that it would be attached to an object we have available at the point we need it, and not require us to do a ridiculous little dance extracting info from the RU at the point we need to actually deploy a subordinate
[15:40] <fwereade> niemeyer, but I am still confused about a number of the things you have been saying
[15:41] <niemeyer> fwereade: Okay, please just submit a review then.. I really don't have nearly enough context and am probably on crack
[15:41] <fwereade> niemeyer, I dunno, you're saying things that are surprising to me -- one of us probably is, I agree, but I'm not yet willing to assume it's you ;p
[15:42] <niemeyer> fwereade: I don't mind changes myself, as long as they are improvements. A contentious point will be code duplication, but I'm sure you can avoid that.
[15:43] <niemeyer> fwereade: Sorry, please ignore the half of what I said that you didn't understand. It really doesn't matter. If we have good APIs for doing what we do need to do, we can change stuff later if needed.
[15:43] <fwereade> niemeyer, would you explain again, though, what use cases you imagine for subordinates that are not in relations with their principals?
[15:43] <fwereade> niemeyer, because I have seen subordinates and relations as essentially inseparable
[15:43] <fwereade> niemeyer, and I fear we're talking Big Redesign if you have plans for arbitrary subordinates without relations
[15:44] <niemeyer> fwereade: What I pointed out is that this was a deliberate choice to simplify comprehension.. I guess we did a good job at that.
[15:45] <niemeyer> fwereade: No, I don't have any impending plans at all.
[15:45] <niemeyer> fwereade: Inside my own mind I just kept the original reasoning that these concepts are, in fact, orthogonal. Maybe that's my mistake.
[15:45] <fwereade> niemeyer, ok, cool -- then ISTM that a subordinate that is not in a relation with its principal is essentially corrupt state
[15:46] <niemeyer> fwereade: Sure, that sounds like a good way to put it for all valid purposes.
[15:47] <fwereade> niemeyer, ok, so: do we then agree that the fact that AUST does not verify relation membership, and just adds a subordinate to an arbitrary principal, is wrong?
[15:49] <fwereade> niemeyer, (ofc I would like to make the concerns entirely orthogonal, but I don't see a way to separate them that is not buggy)
[15:49] <niemeyer> fwereade: Yes, I'm happy to have an API that takes that perspective.
[15:53] <fwereade> niemeyer, ok, cool; so sane subordinate creation depends on (1) a locally scoped relation (2) a principal unit of that relation (and (3) the subordinate service itself)
[15:54] <fwereade> niemeyer, ISTM that RelationUnit, which incorporates (1) and (2) and has easy access to (3), is a good place to expose this functionality
[15:54] <fwereade> niemeyer, and that given a RelationUnit, what I (as the uniter) really want to do is to make sure that any subordinates that should exist do, and just move on
[15:55] <fwereade> niemeyer, so ISTM that `ru.EnterScope(); ru.EnsureSubordinate()` is moderately sensible
[15:56] <niemeyer> fwereade: Yep, sounds fine
[15:56] <fwereade> niemeyer, (part of me wants to roll it into EnterScope, but that really is mixing concerns, so let's not go there)
[15:56] <niemeyer> fwereade: Agreed, some separation is beneficial
[15:58] <fwereade> niemeyer, since this is the *only* known situation in which it is meaningful to add a subordinate, I am -1 on the existence of duplicate functionality in the public API, ie AUST
[15:58] <niemeyer> fwereade: Fair enough
[15:58] <fwereade> niemeyer, I have no official position on which bits of code should or should not be common; I will only discover that in the course of further exploration ;)
[15:58] <niemeyer> fwereade: Yep, I can understand that
[15:59] <niemeyer> fwereade: I'll just note that all of the logic that is currently in addUnit remains needed
[15:59] <fwereade> niemeyer, indeed -- I shall be extra careful to bear that in mind :)
[16:00] <niemeyer> and so does the logic in AddSubordinate
[16:00] <fwereade> niemeyer, you will find no disagreement on that front here
[16:00] <niemeyer> fwereade: Cool, so we're in sync I think
[16:01] <fwereade> niemeyer, yeah, agreed -- thanks :)
[16:01] <fwereade> niemeyer, how would you feel about an exploration of my whining about missing peer relations now? :)
[16:02] <niemeyer> fwereade: I think it's unnecessary at this point.. I suggest celebrating our agreement and trying to push that front forward
[16:02] <fwereade> niemeyer, ok, fair enough :)
[17:05]  * niemeyer breaks for an errand
[18:30] <rogpeppe> i think i've written something quite nice, but i've got to the end of the day.
[18:30] <rogpeppe> spiky spike though.
[18:30] <rogpeppe> g'night all, see you tomorrow.
[21:59] <mramm> Made it through my 20 hours of flights, and none the worse except for one phone with a broken screen
[21:59] <mramm> so if you need to contact me email and IRC work, and I'll be heading out to get a new phone tomorrow