/srv/irclogs.ubuntu.com/2012/12/04/#juju-dev.txt

TheMueGood morning.06:59
=== TheMue_ is now known as TheMue
fwereade_TheMue, heyhey07:35
TheMuefwereade_: Hiya07:35
TheMuefwereade_: 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:53
fwereade_TheMue, cool07:56
fwereade_TheMue, I'll take a look when it's there :)07:56
TheMuefwereade_: I'll notify you, it's a small one.07:56
TheMuefwereade_: Here you are: https://codereview.appspot.com/6875053/08:05
rogpeppe1mornin' folks08:33
fwereade_rogpeppe1, heyhey08:35
=== rogpeppe1 is now known as rogpeppe
TheMuerogpeppe: Hi09:05
TheMuerogpeppe: Would you also take a look at https://codereview.appspot.com/6875053/ ?09:06
rogpeppeTheMue: will do09:06
TheMuerogpeppe: Thx09:06
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:02
TheMuefwereade_: The second one will create the unitd's when a machined is created before returning to the main firewaller loop.10:04
TheMuefwereade_: See http://irclogs.ubuntu.com/2012/11/30/%23juju-dev.html at the end.10:04
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 ports10:17
fwereade_TheMue, and so the window doesn't matter, so long as we don't hold inconsistent data10:18
fwereade_TheMue, but wait...10:19
fwereade_TheMue, isn't the window between initialization of globalPortOpen and globalPortsRef still enough to cause the bug?10:20
TheMuefwereade_: Good question, that I asked myself.10:21
fwereade_TheMue, I think it's just narrowed the window to the time between initGlobalPorts and the relevant machineLifeChanged10:23
TheMuefwereade_: I already thought about a solution where the whole tree of machined, unitd and serviced is initially started when the firewaller starts.10:23
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 solution10:24
TheMuefwereade_: 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:25
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 loop10:26
TheMuefwereade_: 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:28
fwereade_TheMue, ah, great, I misunderstood :)10:29
TheMuefwereade_: So before entering the loop the fw is up and running. ;)10:29
* TheMue steps out for a few minutes, have to fetch medicine from the pharmacy. 10:34
Arammorning.10:54
jammorning Aram11:06
niemeyerGood morning juju-devs11:26
fwereade_niemeyer, heyhey11:28
TheMueniemeyer: Hiya11:30
TheMueniemeyer: 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:39
TheMueniemeyer: What do you think about the idea of starting all machined's etc initially, before entering the firewaller loop?11:40
TheMueniemeyer: 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:41
mgzwallyworld_: looks from the nova code that what gets used when filtering is just whatever webob gives for GET:11:43
mgzsearch_opts = {}11:43
mgzsearch_opts.update(req.GET)11:43
niemeyerTheMue: So there were two different changes, right?11:43
mgz...11:43
mgzstatus = search_opts.pop('status', None)11:44
niemeyerTheMue: 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
niemeyerTheMue: and that was supposed to be the second CL?11:44
mgzah, webob has a neat MultiDict thing... which is not used by this code11:45
wallyworld_\o/11:45
wallyworld_not :-(11:45
mgzwallyworld_: so, it will just pick one at dictionary order random it seems11:45
TheMueniemeyer: 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_awesome11:45
TheMueniemeyer: The both CLs narrow it, but it's still a window there.11:46
niemeyerTheMue, 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 hamstrung11:46
niemeyerTheMue: What's the gap?11:46
mgzright, I'm almost tempted to say supporting this level of (server-side) filtering is not useful11:47
wallyworld_well, i think it may have some use11:48
TheMueniemeyer: 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
TheMueniemeyer: Did you meant to immediatelly start the machined? Then I'll change it and I'm happy. ;)11:48
niemeyerTheMue: Initialized from where?11:48
niemeyerTheMue: No, I'm still trying to understand what's the issue.. I can't possibly make a suggestion before that :)11:49
TheMueniemeyer: 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:50
TheMueniemeyer: It's so far in https://codereview.appspot.com/6875053/.11:51
niemeyerTheMue: 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:52
niemeyerTheMue: Why are we doing this a second time, with less information?11:53
TheMueniemeyer: Yes, that's why I thought about starting the machined's already there. I would like to do so independent of the mode.11:54
niemeyerTheMue: It's not independent of the mode, because currently open ports, and how to open or close the ports, depends on the mode11:57
niemeyerTheMue: And so does port referencing11:57
niemeyerTheMue: Do you understand what the gap that William described actually is?11:58
TheMueniemeyer: 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.11:59
TheMueniemeyer: 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:00
niemeyerTheMue: Why is that a problem? How does the bug take place?12:01
niemeyerTheMue?12:05
TheMueniemeyer: Yeah, still thinking.12:05
TheMueniemeyer: With the CL a machined retrieves it's correct status regarding the ports (but with addition I/O).12:06
niemeyerTheMue: It's still not clear what you think the problem is, and how the bug takes place12:09
TheMueniemeyer: I've got to admit that I'm not sure anymore.12:10
TheMueniemeyer: 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 openness12:11
niemeyerTheMue: I think it's important to understand the semantics of the problem itself so we can drive towards a good solution12:13
TheMuefwereade_: 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:13
niemeyerTheMue: I don't quite see what that means12:14
niemeyerTheMue: The problem is simpler than it sounds12:15
TheMueniemeyer: 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, state12:15
niemeyerTheMue: There's a point in the system where we observe the actual state, as the provider reports it12:15
niemeyerTheMue: Can you pinpoint that location?12:16
TheMuefwereade_: 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 consistent12:16
fwereade_TheMue, you can't get separate sets of ports in spearate places and assume they match12:17
TheMuefwereade_: Exactly.12:17
TheMueniemeyer: Sorry, can't follow.12:18
TheMueniemeyer: We start by watching the machines.12:18
niemeyerTheMue: No, we don't12:18
niemeyerTheMue: 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:19
TheMueniemeyer: We retrieve it from Environ.OpenPorts() and INstance.OpenPort(), depending of the mode.12:20
niemeyerTheMue: Yes, let's leave aside instance mode for a while12:21
niemeyerTheMue: What's the line?12:21
TheMueniemeyer: Eh, in the code?12:22
niemeyerTheMue: Yes, in the code12:22
TheMueniemeyer: It's in Firewaller.initGlobalMode(), used to check which ports are already open.12:23
niemeyerTheMue: What's the line number?12:23
TheMueniemeyer: In the CL it's 197.12:23
niemeyerTheMue: No, it's not12:23
TheMueniemeyer: Exactly, just seen. On moment.12:23
TheMueniemeyer: I meant Ports() in 165.12:24
TheMueniemeyer: The other one opens them.12:24
niemeyerTheMue: Very well, now let's say that port 123 is open there12:24
niemeyerTheMue: How do we tell we should close it or not?12:24
TheMueniemeyer: 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
niemeyerTheMue: Cool12:26
niemeyerTheMue: 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 initGlobalMode12:27
niemeyerTheMue: When do we close port 123?12:27
TheMueniemeyer: 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:29
niemeyerTheMue: THat's not it12:32
niemeyerTheMue: We do watch the units12:32
niemeyerTheMue: Why is it not working even though we do watch the units?12:34
TheMueniemeyer: When the machined is started.12:34
TheMueniemeyer: The machined is watching its units, but at that moment the machined isn't running, so it doesn't watch the unit.12:35
TheMueniemeyer: Or what do I miss?12:36
niemeyerTheMue: Nope.. that's not it12:36
niemeyerTheMue: That's exactly what I'm trying to figure12:36
niemeyerTheMue: Port 123 is in initial ports.. what else knows that port 123 is open?12:37
TheMueniemeyer: IMHO per machined in line 495 we start watching the units of the machine.12:38
TheMueniemeyer: The state knows. That's why we compare the environment to the state.12:39
niemeyerTheMue: No, it doesn't12:39
niemeyerTheMue: The unit has closed port 123 at the end of initGlobalMode, remember?12:39
TheMueniemeyer: Yes.12:40
niemeyerTheMue: So.. port 123 is open.. and it shouldn't be.. how do we know to close it12:43
TheMueniemeyer: Hmm, if we watch the unit we should receive an event and look what parts a unit has opened.12:46
TheMueniemeyer: That's line 573.12:46
niemeyerTheMue: Will port 123 be there?12:47
TheMueniemeyer: A correct unitd should know it still, OpenedPorts() doesn't return it, so the change will be sent to the Firewaller.12:48
niemeyerTheMue: Why? Where is port 123 in that logic?12:49
TheMueniemeyer: 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:51
niemeyerTheMue: Which variable/code line will we find port 123 in?12:54
TheMueniemeyer: And via the chain flushUnits() -> flushMachine() -> flushGlobalPorts() a diff is done and 123 should be closed.12:54
TheMueniemeyer: change in 573 doesn't contain it anymore while fw.globalOpenPorts still has it as open.12:56
niemeyerTheMue: and why does it matter? See line 57412:57
TheMueniemeyer: unitd knows its ports and the opened ones in state are not the same.12:59
niemeyerTheMue: Why are they not the same?  Port 123 was closed in the state at the end of initGobalMode, remember?12:59
TheMueniemeyer: Just found by that way that we dont use unitData.ports here.13:00
niemeyer<niemeyer> TheMue: Why are they not the same?  Port 123 was closed in the state at the end of initGobalMode, remember?13:03
TheMueniemeyer: 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:03
TheMueniemeyer: Why did you repeated the sentence?13:04
niemeyerTheMue: Because it's been four minutes13:04
TheMueniemeyer: OK.13:05
TheMueniemeyer: Will remember.13:05
niemeyerTheMue: So, is the issue clear now?13:06
TheMueniemeyer: I hope so. But that's exactly why I wanted to start the machined etc before the loop is starting.13:07
TheMueniemeyer: With the data from state.13:08
* fwereade_ => lunch13:08
* TheMue 's family wait for him to come to lunch too.13:08
niemeyerTheMue: Argh13:08
niemeyerTheMue: The issue isn't starting machined early with data from state13:09
niemeyerTheMue: Okay, have a good lunch..13:09
niemeyerfwereade_: And you too13:09
TheMueniemeyer: Maybe the sentence is just to short and I'll outline it after lunch.13:10
niemeyerTheMue: No, it's missing the point..13:10
niemeyerTheMue: The bug isn't about how early you start machined with data from state13:10
niemeyerTheMue: The bug is the lack of correlation with the known open ports13:11
niemeyerTheMue: That's what we spent the last 2 hours going over13:11
TheMueniemeyer: 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
niemeyerTheMue: Glad to hear it, thanks13:13
=== flaviamissi_ is now known as flaviamissi
TheMueSo, back again.13:37
=== marrusl_ is now known as marrusl
niemeyerLunch time here too14:13
TheMueniemeyer: Enjoy.14:27
fwereadeniemeyer, btw, I am having a spot of bother with AddUnitSubordinateTo -- I don't really think that's the right API14:41
fwereadeniemeyer, I'm leaning towards something like RelationUnit.EnsureSubordinate instead14:41
fwereadeniemeyer, does that strike you as obviously mad?14:41
fwereadeoh, bother, lunch14:42
fwereadeTheMue, rogpeppe: sniff test on the above?14:42
TheMuefwereade: Yes, second one reads better.14:47
fwereadeTheMue, cheers14:47
niemeyerfwereade: I don't have as much context as you do. Is there any motivation for the change?14:54
fwereadeniemeyer, 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 exists14:56
niemeyerfwereade: (2) is somewhat obvious, given that the feature is in development14:57
fwereadeniemeyer, (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
niemeyerfwereade: (3) doesn't sound like an issue.. it can be a well defined error14:57
niemeyerfwereade: I find it quite appropriate that adding units is done in the same place always, and with similar interfaces14:57
fwereadeniemeyer, I dunno, the gulf between a principal service and a subordinate service is pretty significant14:58
niemeyerfwereade: The two only functions that do that today are right next to one another, and they share the same implementatin14:58
niemeyerfwereade: That seems very compelling14:58
fwereadeniemeyer, if I try to keep them using the same implementation, it gets ugly fast14:59
fwereadeniemeyer, the ops and assertions end up reasonably different, and determining the failure is *very* different14:59
niemeyerfwereade: I'm definitely missing background.. you don't have to try to do that. It's already in place14:59
fwereadeniemeyer, well15:01
fwereadeniemeyer, a method called AddUnitSubordinateTo does exist15:01
fwereadeniemeyer, but it doesn't have any checks for dupe subordinates15:01
fwereadeniemeyer, so, really, it's not in place15:02
niemeyerfwereade: Right.. that's the assertion we talked about yesterday15:02
fwereadeniemeyer, yes; that bit is easy; but correct abort handling STM to get kinda ugly15:02
niemeyerfwereade: Which seems pretty easy to add in addUnit, in the location that already exists to add a subordinate15:02
niemeyerfwereade: It won't get any less ugly if you move that logic elsewhere, it sounds like15:02
niemeyerfwereade: All the common logic for adding a unit that exists on this function sounds sensible, and necessary15:03
fwereadeniemeyer, 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
fwereadeniemeyer, or are we just arguing implementation details?15:05
niemeyerfwereade: We're surely arguing implementation details, given that we're talking about logic placement15:06
fwereadeniemeyer, ok, that is not something I think I can honestly speak to until I've tried 2 or 3 different styles15:06
niemeyerfwereade: I'm -1 on duplicating logic without a clear reason15:07
fwereadeniemeyer, right15:07
niemeyerfwereade: and moving it away from other logic that looks very much alike15:07
niemeyerfwereade: EnsureSubordinate sounds like something that can trivially be built upon AddUnitSubordinateTo15:08
niemeyerfwereade: err := addUnit; err == AlreadyThere { Oh, okay. }15:08
fwereadeniemeyer, ok, but then we have a useful public state method and a useless one, for doing the same thing15:08
fwereadeniemeyer, which feels kinda bloaty15:08
fwereadeniemeyer, but perhaps you have some extra use in mind for AUST?15:09
niemeyerfwereade: Okay, so let's not have EnsureSubordinate.. because that's the trivial one that woudl duplicate logic15:09
fwereadeniemeyer, can you explain the use case for the AddUnitSubordinateTo method?15:10
niemeyerfwereade: 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
fwereadeniemeyer, 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
niemeyerfwereade: I don't understand the question.. the answer would be self-obvious15:10
niemeyerfwereade: It adds a unit that is a subordinate to a principal15:11
fwereadeniemeyer, right, and when do we need to do this?15:11
niemeyerfwereade: When we want to add a subordinate unit15:11
fwereadeniemeyer, ISTM that you are assuming that there are a multiplicity of situations in which we want to do this15:12
niemeyerfwereade: No, I'm not assuming anything..15:12
niemeyerfwereade: 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
niemeyerfwereade: It sounds to me like straightforward design15:13
fwereadeniemeyer, right -- expect it's seriously wrong in at least two ways15:13
niemeyerfwereade: Okay?15:14
fwereadeniemeyer, and it is not a remotely convenient thing to do in the situation in which it's required15:15
niemeyerfwereade: Sorry, how is it seriously wrong, and how is it not remotely convenient15:15
fwereadeniemeyer, 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 discussed15:16
niemeyerfwereade: So AddUnit is completely wrong too?15:16
fwereadeniemeyer, no -- AddUnit is AFAICT ok, although I haven't been looking at that side of it closely15:17
niemeyerfwereade: Why? We can add anything to anything as well, without any care about relation state15:17
fwereadeniemeyer, why should AddUnit care about relation state?15:17
niemeyerfwereade: Well, there are peer relations too as well, right?15:18
fwereadeniemeyer, so what?15:18
niemeyerfwereade: Well, exactly.. :)15:18
niemeyerfwereade: Subordinates aren't different..15:18
fwereadeniemeyer, we've already made the decision that broken peer relations are just fine15:18
fwereadeniemeyer, and that the user mustn't touch them15:18
fwereadeniemeyer, I consider this to be crack, but anyway15:19
fwereadeniemeyer, but still -- why would the existence or otherwise of a peer relation impact whether it's ok to add a unit?15:19
niemeyerfwereade: Sorry, I don't understand what we've decided regarding broken peer relations15:19
niemeyerfwereade: But okay, let's leave that aside15:19
niemeyerfwereade: The relation state can be completely inconsistent after you added that unit15:19
niemeyerfwereade: There should be a peer relation, and there isn't15:19
niemeyerfwereade: This will be eventually established as the uniter runs15:20
fwereadeniemeyer, whaaaaa?15:20
niemeyerfwereade: I'm trying to understand why you think that's so much different from the case of subordinates15:20
niemeyerfwereade: What within AddUnitSubordinateTo and addUnit do you think shouldn't be there?15:21
fwereadeniemeyer, is it OK to have one unit of mongodb deployer?15:21
niemeyerfwereade: Why is that wrong?15:21
fwereades/deployer/deployed/15:21
niemeyerfwereade: Obviously15:22
niemeyerfwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?15:22
fwereadeniemeyer, can we back up?15:22
niemeyerfwereade: That's what I'm trying to do15:22
fwereadeniemeyer, because you have thrown me a complete curevball regarding peer relations15:22
niemeyerfwereade: Yes, sorry, please ignore me there15:23
niemeyerfwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?15:23
fwereadeniemeyer, 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
niemeyerfwereade: Can we back up a bit?15:23
niemeyerfwereade: What within AddUnitSubordinateTo and addUnit you believe shouldn't be there?15:23
fwereadeniemeyer, where on earth did that question come from? I am talking about *deficiencies* in AUST15:24
fwereadeniemeyer, things that are *not* there15:24
niemeyerfwereade: It came from my mind15:24
fwereadeniemeyer, well, please, what does it have to do with what I have been saying?15:24
niemeyer<fwereade> niemeyer, right -- expect it's seriously wrong in at least two ways15:24
niemeyerfwereade: I'm trying to understand what's so dramatically wrong about it15:25
niemeyerfwereade: Because I've been pointing out that what's there is necessary, it's close to related logic, etc15:25
niemeyerfwereade: It feels like we've been talking across each other15:25
fwereadeniemeyer, ok, so what does it mean for S1 to have a subordinate of S2 when there is no relation between S1 and S2?15:25
fwereadeniemeyer, and, what does it mean for S1 to have a unit when S1 is not in a peer relation?15:26
niemeyerfwereade: It means that there are two units, one is subordinate of the other, and they'll both be deployed15:26
niemeyerfwereade: S1 and S2 will live in the same container15:26
niemeyerfwereade: and unless a relation is established between them, nothing else will happen15:26
fwereadeniemeyer, why would we deploy a subordinate in the first place without there being a relation between them?15:27
niemeyerfwereade: 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 users15:27
niemeyerfwereade: I believe everything actually does work, even if there's no relation between them15:27
fwereadeniemeyer, 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
niemeyerfwereade: It's a sane building block, and one that seems to work pretty well thus far15:28
niemeyerfwereade: That's how we implement the UI, yep15:28
niemeyerfwereade: Feels great15:28
fwereadeniemeyer, ok, but it sounds like you have some use case for subordinates that are not in relations with their principals?15:29
niemeyerfwereade: Maybe there's another reason why you believe OMG THATS TERRIBLE WE'LL ALL DIE if that method exists15:29
niemeyerfwereade: Which is what I've been trying to extract so far15:29
niemeyerfwereade: No, I don't have a reason to believe that the current logic is completely broken, yet.. that's all15:29
niemeyerfwereade: We have tests, we have methods.. methods work well so far.. Python seems to work as well to some degree.. etc15:30
niemeyerfwereade: You seem to think that the logic that is there is actually okay too, actually15:30
niemeyerfwereade: But there's more logic that we'll need, to make things work well15:30
niemeyerfwereade: Which sounds fine too15:30
niemeyerfwereade: So perhaps what I don't understand is the strong feeling that things are totally broken15:31
niemeyerfwereade: So, what's the case? EnsureSubordinate.. what would it do that AddUnitSubordinateTo can't be doing?15:35
fwereadeniemeyer, 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 subordinate15:39
fwereadeniemeyer, but I am still confused about a number of the things you have been saying15:40
niemeyerfwereade: Okay, please just submit a review then.. I really don't have nearly enough context and am probably on crack15:41
fwereadeniemeyer, 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 ;p15:41
niemeyerfwereade: 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:42
niemeyerfwereade: 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
fwereadeniemeyer, would you explain again, though, what use cases you imagine for subordinates that are not in relations with their principals?15:43
fwereadeniemeyer, because I have seen subordinates and relations as essentially inseparable15:43
fwereadeniemeyer, and I fear we're talking Big Redesign if you have plans for arbitrary subordinates without relations15:43
niemeyerfwereade: What I pointed out is that this was a deliberate choice to simplify comprehension.. I guess we did a good job at that.15:44
niemeyerfwereade: No, I don't have any impending plans at all.15:45
niemeyerfwereade: Inside my own mind I just kept the original reasoning that these concepts are, in fact, orthogonal. Maybe that's my mistake.15:45
fwereadeniemeyer, ok, cool -- then ISTM that a subordinate that is not in a relation with its principal is essentially corrupt state15:45
niemeyerfwereade: Sure, that sounds like a good way to put it for all valid purposes.15:46
fwereadeniemeyer, 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:47
fwereadeniemeyer, (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
niemeyerfwereade: Yes, I'm happy to have an API that takes that perspective.15:49
fwereadeniemeyer, 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:53
fwereadeniemeyer, ISTM that RelationUnit, which incorporates (1) and (2) and has easy access to (3), is a good place to expose this functionality15:54
fwereadeniemeyer, 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 on15:54
fwereadeniemeyer, so ISTM that `ru.EnterScope(); ru.EnsureSubordinate()` is moderately sensible15:55
niemeyerfwereade: Yep, sounds fine15:56
fwereadeniemeyer, (part of me wants to roll it into EnterScope, but that really is mixing concerns, so let's not go there)15:56
niemeyerfwereade: Agreed, some separation is beneficial15:56
fwereadeniemeyer, 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 AUST15:58
niemeyerfwereade: Fair enough15:58
fwereadeniemeyer, 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
niemeyerfwereade: Yep, I can understand that15:58
niemeyerfwereade: I'll just note that all of the logic that is currently in addUnit remains needed15:59
fwereadeniemeyer, indeed -- I shall be extra careful to bear that in mind :)15:59
niemeyerand so does the logic in AddSubordinate16:00
fwereadeniemeyer, you will find no disagreement on that front here16:00
niemeyerfwereade: Cool, so we're in sync I think16:00
fwereadeniemeyer, yeah, agreed -- thanks :)16:01
fwereadeniemeyer, how would you feel about an exploration of my whining about missing peer relations now? :)16:01
niemeyerfwereade: I think it's unnecessary at this point.. I suggest celebrating our agreement and trying to push that front forward16:02
fwereadeniemeyer, ok, fair enough :)16:02
* niemeyer breaks for an errand17:05
rogpeppei think i've written something quite nice, but i've got to the end of the day.18:30
rogpeppespiky spike though.18:30
rogpeppeg'night all, see you tomorrow.18:30
mrammMade it through my 20 hours of flights, and none the worse except for one phone with a broken screen21:59
mrammso if you need to contact me email and IRC work, and I'll be heading out to get a new phone tomorrow21:59

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