| rogpeppe | davecheney: morning | 06:09 |
|---|---|---|
| davecheney | rogpeppe: morning ! | 06:10 |
| davecheney | how's it going | 06:10 |
| rogpeppe | not too bad | 06:10 |
| davecheney | rogpeppe: here's one for ya! https://codereview.appspot.com/6862050/ | 06:12 |
| davecheney | rogpeppe: look at this http://code.google.com/p/rietveld/issues/detail?id=406#c2 | 06:22 |
| rogpeppe | davecheney: re the above CL: why not just rename serialisedLogger to logger and lose the interface type? | 06:40 |
| rogpeppe | davecheney: (i was planning to run the race detector on our tests today too actually - nice idea) | 06:41 |
| davecheney | rogpeppe: yeah, we can do that | 06:41 |
| davecheney | rogpeppe: yes, i've been running it on the core state/* and worker classes | 06:42 |
| davecheney | we're pretty good | 06:42 |
| rogpeppe | davecheney: that's a relief to hear | 06:42 |
| rogpeppe | davecheney: although it would be interesting to run some of the tests concurrently. | 06:43 |
| davecheney | oh boy, don't even think about GOMAXPROCS > 1 | 06:43 |
| rogpeppe | i meant concurrently rather than in parallel, but that too | 06:43 |
| davecheney | oooooooooooh, you mean all the state tests at once ? | 06:44 |
| rogpeppe | davecheney: well, obviously that wouldn't currently be possible, but yeah, that kind of thing | 06:48 |
| rogpeppe | davecheney: gocheck should allow the equivalent of t.Parallel() (if i remember that name right) | 06:49 |
| rogpeppe | http://golang.org/pkg/testing/#T.Parallel | 06:49 |
| rogpeppe | i really think that gocheck could use some modernisation | 06:50 |
| rogpeppe | davecheney: you've got a review on that gocheck CL | 06:52 |
| davecheney | rogpeppe: ta muchly | 06:55 |
| fwereade | mornings | 06:55 |
| TheMue | Mornings from a snowy Oldenburg too | 06:57 |
| fwereade | TheMue, I forget, did I discuss the meaning of a Dying Machine with you? I'm trying to find someone who can explain in what circumstances a Machine should be able to become so, and how the consequences of that make sense | 07:23 |
| fwereade | TheMue_, I forget, did I discuss the meaning of a Dying Machine with you? I'm trying to find someone who can explain in what circumstances a Machine should be able to become so, and how the consequences of that make sense | 07:25 |
| === TheMue_ is now known as TheMue | ||
| fwereade | TheMue, ping | 07:54 |
| TheMue | fwereade: Pong. | 07:54 |
| fwereade | TheMue, I think you might have missed: | 07:55 |
| fwereade | <fwereade> TheMue, I forget, did I discuss the meaning of a Dying Machine with you? I'm trying to find someone who can explain in what circumstances a Machine should be able to become so, and how the consequences of that make sense | 07:55 |
| TheMue | fwereade: Oh, yes, I missed. Strange. | 07:55 |
| fwereade | TheMue, because the "obvious" meaning is that the machine should start to shut itself down when it becomes Dying | 07:56 |
| fwereade | TheMue, but actually, if it has units, it can't do that: it'll be blocked for as long as they exist, which could be forever | 07:56 |
| fwereade | TheMue, but the machine will from that point on be kinda broken -- eg you won't be able to assign new units to it | 07:57 |
| fwereade | TheMue, and the Alive=>Dying change is irrevocable | 07:57 |
| fwereade | TheMue, and it really doesn't seem like a smart idea to ever put machines in that state | 07:57 |
| TheMue | fwereade: So far following, yes. | 07:58 |
| fwereade | TheMue, can you think of an alternative conception of what should happen? or, if not, do you then agree that a Dying machine is somewhere between meaningless and harmful? | 07:59 |
| TheMue | fwereade: To take a step back, I always had my problems with the idea of setting lifecyle a state to trigger a reaction. | 08:01 |
| TheMue | fwereade: My natural feeling is more vice versa. | 08:01 |
| fwereade | TheMue, that's interesting, would you expand on how you'd prefer to see it? | 08:01 |
| TheMue | fwereade: I somehow tell a machine to to stop. During this, it sets its state to Daying to represent this. And at the end it sets its state to Dead. | 08:01 |
| TheMue | fwereade: So the lifecycle state is only a representing and informal information for others to see what's going on. | 08:02 |
| fwereade | TheMue, I thought that was the model we did follow? except with the first bit slightly compressed -- ie that setting something to Dying is precisely how we communicate that it should stop | 08:02 |
| TheMue | fwereade: Yes, and that's what I dislike. | 08:03 |
| TheMue | fwereade: Because it isn't dying, we want it to do so. | 08:03 |
| TheMue | fwereade: And itself can only tell: "Hey, guys, I'm dying." | 08:04 |
| TheMue | fwereade: But that's only a personal preference and I won't start this discussion again. ;) | 08:04 |
| fwereade | TheMue, if it's annoying for you to go over it again then I'll gladly drop it, but I'm interested in your perspective here | 08:06 |
| fwereade | TheMue, what do you expect will be watching for the Dying status? | 08:06 |
| TheMue | fwereade: No, it's not annoying. But we have an agreement, so that should be changed. | 08:07 |
| fwereade | TheMue, ofc it's tedious to discuss this generally, because the various lifecycle entities are much more different than I think we ever imagined :/ | 08:07 |
| TheMue | fwereade: Currently I don't know for whom it could be interesting to watch the Dying status, but it could be interesting if I'm iterating and only want those with status Alive. | 08:08 |
| TheMue | fwereade: Yes, exactly. | 08:08 |
| fwereade | TheMue, I don't *think* that's a sane operation, but I could easily be wrong | 08:09 |
| fwereade | TheMue, yeah, I'm sure I'm wrong somewhere, just because there are so many possibilities :) | 08:09 |
| TheMue | fwereade: But let's come back to the current way to do so and what you've discovered. | 08:10 |
| fwereade | TheMue, heh, much of this will be messy, I don't think I've ever tried to express it all at once before, but here goes | 08:10 |
| fwereade | TheMue, so: the detect-self-Dying=>cleanup-and-set-self-Dead model is very clear when it comes to entities that have agents, ie units and machines | 08:12 |
| fwereade | TheMue, notwithstanding my belief that it isn't *actually* sane wrt machines | 08:12 |
| fwereade | TheMue, but in any case it's definitely different to service and relation lifecycle management | 08:13 |
| fwereade | TheMue, because those entities rely on agents for other entities to manage them | 08:13 |
| TheMue | fwereade: Yep. | 08:14 |
| fwereade | TheMue, (and in fact units depend on machine agents (or sometimes other unit agents), and machine agents depend on other machine agents (ie those running provisioners) | 08:14 |
| fwereade | TheMue, so, putting it that way, maybe it's not as different as I thought: ie *every* entity has its lifecycle partly managed by something else | 08:15 |
| fwereade | TheMue, it's just that some of them don't have their own agents at all, so their lifecycles have to be exclusively managed by other things | 08:15 |
| fwereade | TheMue, but, nonetheless | 08:16 |
| fwereade | TheMue, Relation has already lost EnsureDead on the basis that that is not a meaningful change | 08:16 |
| fwereade | TheMue, actually, it's lost EnsureDying too | 08:17 |
| fwereade | TheMue, it just has Destroy | 08:17 |
| fwereade | TheMue, are you familiar with that change and its context? | 08:17 |
| fwereade | TheMue, (it should also be noted that there's no such thing as a Dead relation -- the possible paths are Alive -> removed and Alive -> Dying -> removed) | 08:18 |
| TheMue | fwereade: Not yet, but I'm simply following what you're saying here and raise a question when I need more background (or at least I think so). | 08:18 |
| TheMue | fwereade: Sounds reasonable. | 08:19 |
| fwereade | TheMue, the issue is basically that the point at which a relation can be removed is the point at which the last relation unit leaves scope | 08:20 |
| fwereade | TheMue, we can't enter scope if any of the involved entities are dying | 08:21 |
| fwereade | TheMue, and in fact no entity will take it on itself to just remove a Dying relation -- if the relation is Dying when the uniter first sees it, it just ignores it | 08:22 |
| TheMue | fwereade: Hmm, that's a bit too short and fast. | 08:22 |
| TheMue | fwereade: Could you please explain it differently. | 08:22 |
| fwereade | TheMue, I'll try :) | 08:22 |
| fwereade | TheMue, a relation exists, and we don't want it to any more | 08:23 |
| fwereade | TheMue, the "easy" answer is "set Dying, let the system handle it" | 08:23 |
| TheMue | fwereade: That's the UI, but how shall the system do it. | 08:24 |
| fwereade | TheMue, but if there are no units currently participating in that relation, there is no entity with responsibility for that relation's death | 08:24 |
| fwereade | TheMue, it would ordinarily be the last unit | 08:24 |
| fwereade | TheMue, to leave scope | 08:24 |
| fwereade | TheMue, but if none are in scope this isn't sane | 08:24 |
| fwereade | TheMue, so, in the absence of a CA, just setting Dying is not good enough | 08:25 |
| TheMue | fwereade: Why does the relation exist if there's no unit in scope? | 08:25 |
| fwereade | TheMue, because someone related two services before any of the services machines were provisioned, for example | 08:26 |
| fwereade | TheMue, relation scope is how units express their personal participation in a relation | 08:26 |
| fwereade | TheMue, a scope is meaningless without a relation, but a relation on its own is just an expression of how we would like the units of certain services to communicate | 08:27 |
| TheMue | fwereade: Ah, yes, forgot it. | 08:27 |
| fwereade | TheMue, so, EnsureDying is useless | 08:28 |
| fwereade | TheMue, and for similar reasons EnsureDead is useless, except even more so, because by definition a relation cannot become Dead while there is some unit in scope -- ie some unit willing to take responsibility for finally removing it | 08:29 |
| fwereade | TheMue, so the actual lifecycle management (apart from creation) is managed by some slightly icky transactions in RU.EnterScope, RU.LeaveScope, and R.Destroy | 08:30 |
| fwereade | TheMue, and nothing therein ever sets the relation to Dead -- once it's safe to make it Dead, it's safe to destroy it entirely | 08:30 |
| fwereade | TheMue, making sense so far? | 08:31 |
| TheMue | fwereade: Yes, can follow. | 08:31 |
| fwereade | TheMue, ok, now apply what I have just said to Service | 08:31 |
| fwereade | TheMue, I *think* that the only sane way to manage service is as above | 08:32 |
| fwereade | TheMue, ie via careful txn management in S.AddUnit, S.RemoveUnit, and (mooted) s.Destroy | 08:32 |
| fwereade | TheMue, for service, this renders EnsureDying and EnsureDead meaningless | 08:33 |
| fwereade | TheMue, and in fact the Dead state is again meaningless for a Service | 08:33 |
| TheMue | fwereade: It feels more natural than fiddling with life states. | 08:33 |
| fwereade | TheMue, yeah, hopefully so :) | 08:34 |
| fwereade | TheMue, the trouble is that the life model as we originally designed it does not apply to: | 08:34 |
| fwereade | TheMue, 1) relations -- no meaningful Dead | 08:34 |
| fwereade | TheMue, 2) services -- no meaningful Dead | 08:35 |
| fwereade | TheMue, 3) machines -- no meaningful Dying | 08:35 |
| fwereade | TheMue, 4) and units -- because Alive/Dying/Dead is (according to niemeyer) not expressive enough to accommodate forced removal | 08:35 |
| fwereade | TheMue, and my big personal problem is that we still seem to have this idea that lifecycles are agreed, and make sense, and have already been implemented | 08:36 |
| TheMue | fwereade: I don't hear you anymore. *whistling* | 08:36 |
| fwereade | TheMue, when in fact they aren't, they don't, and they haven't | 08:36 |
| fwereade | TheMue, haha :) | 08:37 |
| TheMue | fwereade: Somehow you're hitting the nerve. Never thought about it so concentrated. | 08:37 |
| fwereade | TheMue, if you can see any holes in the above, please poke them, because I'm going to have to muster the emotional energy to bring this up with niemeyer at some point soon, and yu know how it can be -- one missed detail means he dismissed your whole argument | 08:38 |
| TheMue | fwereade: For me the life discussion is difficult. I prefer to collect facts/arguments/etc in a document and discuss that. It's simpler to refer to. | 08:43 |
| fwereade | TheMue, (ofc I don't mean to say he always does that -- but it is a risk I am loath to assume) | 08:43 |
| rogpeppe | fwereade: for me, the big advantage of Dying is that the entity enters a shutdown phase, meaning that other things can't interact with it while it's dying (which might take several transactions) | 09:08 |
| fwereade | rogpeppe, yeah, I know that's what we think it means | 09:10 |
| fwereade | rogpeppe, in practice, leaving aside the niche use case in which we render a machine crippled for an arbitrary length of time, Dying STM not to be a useful state for a machine... or, if it is, it's one we can only enter when the machine has no units | 09:11 |
| fwereade | rogpeppe, but we certainly don't enforce that anywhere | 09:11 |
| fwereade | rogpeppe, I'd be perfectly happy with that restriction, but it's pretty inconsistent with everything else | 09:12 |
| rogpeppe | fwereade: the problem is: when can you remove a machine? | 09:12 |
| rogpeppe | fwereade: and i guess the answer is: when it has no units assigned | 09:12 |
| fwereade | rogpeppe, when and only when it has no units assigned, I think | 09:12 |
| rogpeppe | fwereade: ok, in this case we can consider dying/dead redundant because they're compressed into one transaction ("remove iff no units") | 09:13 |
| fwereade | rogpeppe, I think we still need Dead, so the provisioner can trash the instance and remove the machine | 09:14 |
| rogpeppe | fwereade: ok, EnsureDead iff no units | 09:15 |
| rogpeppe | fwereade: which presumably the Machine.EnsureDead checks anyway | 09:15 |
| rogpeppe | fwereade: ha, it doesn't | 09:16 |
| rogpeppe | fwereade: but it should | 09:16 |
| fwereade | rogpeppe, yeah, like just about every other bit f lifecycle that we'v "implemented" :/ | 09:16 |
| rogpeppe | fwereade: can we check for no units and set to dead all in one tranaction? | 09:17 |
| fwereade | rogpeppe, think so, yeah | 09:17 |
| rogpeppe | fwereade: well, actually, given dying and dead, it's not a problem | 09:18 |
| rogpeppe | fwereade: we don't need to do that | 09:18 |
| rogpeppe | fwereade: but, as you've said, perhaps it doesn't really make sense to tag a machine to die in the future | 09:19 |
| rogpeppe | fwereade: a unit could live its entire life on a Dying machine | 09:19 |
| rogpeppe | fwereade: which seems kinda cool in a sf kinda way :-) | 09:19 |
| fwereade | rogpeppe, yeah, exactly -- and I'm not even saying that use case never applies | 09:20 |
| fwereade | rogpeppe, just that I'm not sure it should be our primary focus | 09:20 |
| rogpeppe | fwereade: i still think that even if we don't leverage all the states, dying and dead is still a useful way to think about removing entities | 09:21 |
| fwereade | rogpeppe, ok, but... Dying is (currently) useless for machines | 09:22 |
| fwereade | rogpeppe, Dead is meaningless for relations and services | 09:22 |
| rogpeppe | fwereade: even if for some things, we just use Remove instead of EnsureDead, and for others we don't use Dying at all. | 09:22 |
| fwereade | rogpeppe, and niemeyer wants an extra lifecycley flag for units to accommodate --force | 09:22 |
| rogpeppe | fwereade: i'm still not sure about --force and how it should be done. there are quite a few edge cases there that i'm wary of. | 09:23 |
| fwereade | rogpeppe, well, yeah -- IMO the only sane way is with a single transaction, but meh, niemeyer wants a flag so he's probably going to get a flag | 09:24 |
| rogpeppe | fwereade: can't it be a flag on the unit rather than an extra lifecycle state that everything needs to deal with? | 09:24 |
| fwereade | rogpeppe, yeah, but IMO that kicks the last leg out from underthe idea that there is anything usefull common about lifecycle handling | 09:25 |
| rogpeppe | fwereade: i don't think so. the dying/dead distinction is still useful there | 09:25 |
| fwereade | rogpeppe, the existence of the states as they is, IMO, just fine | 09:25 |
| fwereade | rogpeppe, but in practice no entity follows the model we've kidded ourselves that they all do | 09:26 |
| fwereade | rogpeppe, the only features that STM to apply across the board are: | 09:26 |
| fwereade | * lifecycle state can only advance | 09:26 |
| fwereade | * entity lifecycles can sometimes skip states | 09:26 |
| fwereade | * no agent/entity is responsible for removing itself | 09:26 |
| rogpeppe | that seems fine to me | 09:27 |
| rogpeppe | fwereade: it doesn't say that lifecycle state is irrelevant | 09:27 |
| fwereade | rogpeppe, when did I say it was? | 09:27 |
| fwereade | rogpeppe, the states are fine | 09:27 |
| fwereade | rogpeppe, but we're operating under a shared delusion that we've understood the problem | 09:27 |
| rogpeppe | fwereade: ah, i thought you were objecting to Dying and Dead | 09:27 |
| fwereade | rogpeppe, no -- just conceits like EnsureDying and EnsureDead | 09:28 |
| rogpeppe | fwereade: they're just setting flags. i'd never expected them to be anything other than part of the solution. | 09:28 |
| fwereade | rogpeppe, but just setting a flag is almost always crackful | 09:29 |
| rogpeppe | fwereade: yeah, quite possibly. i think maybe they should not be exported | 09:29 |
| rogpeppe | fwereade: then each entity gets methods that are appropriate | 09:29 |
| fwereade | rogpeppe, I'm pretty sure they shouldn't even exist -- they're premature abstraction that's just trying to wedge everything into the same model, that doesn't actually ever apply to anything | 09:30 |
| rogpeppe | fwereade: such as Remove (for a machine - only works if no units) | 09:30 |
| fwereade | rogpeppe, I think entity.Destroy is sensible | 09:30 |
| fwereade | rogpeppe, to go with the agreed CLI terminology | 09:30 |
| fwereade | rogpeppe, that's what we have for relations anyway | 09:30 |
| fwereade | rogpeppe, and what I *think* we need for services | 09:30 |
| rogpeppe | fwereade: i dunno. i think EnsureDying is reasonable for things where the entity *will* take a while to clear up. | 09:31 |
| fwereade | rogpeppe, and how do yu determine that to be the case? | 09:31 |
| rogpeppe | fwereade: but maybe Destroy is appropriate there too. | 09:31 |
| rogpeppe | fwereade: well, if you're removing a unit, it's going to take a while for the unit to actually shut down | 09:32 |
| rogpeppe | fwereade: but if there's no responsible entity yet, perhaps it *should* be removed immediately | 09:32 |
| fwereade | rogpeppe, indeed so, but even then I'm not sure | 09:33 |
| rogpeppe | fwereade: and Destroy could return before the entity is actually destroyed | 09:33 |
| fwereade | rogpeppe, I am pretty sure about Relation.Destroy and Service.Destroy | 09:33 |
| fwereade | rogpeppe, yeah, absolutely | 09:33 |
| fwereade | rogpeppe, (and that neither of those should have EnsureD* methods) | 09:34 |
| fwereade | rogpeppe, machines and units I am still very unsure about | 09:34 |
| fwereade | rogpeppe, I think that if a unit has an asisgned machine, we have to set it to Dying and let the MA make the more complex decisions | 09:35 |
| rogpeppe | fwereade: i vote for having Destroy on all of them, and have Dying/Dead as an implementation detail (and something that's potentially watchable) | 09:35 |
| rogpeppe | fwereade: definitely. a unit needs to be able to clean itself up, that's the whole point of all thise | 09:35 |
| rogpeppe | this | 09:35 |
| rogpeppe | fwereade: i think Life makes sense, but EnsureDead/Dying not as much (or at all, perhaps) | 09:36 |
| fwereade | rogpeppe, ok, I think we are in agreement | 09:36 |
| rogpeppe | fwereade: but the difficulty here is the distinction between Destroy and Remove. | 09:38 |
| rogpeppe | fwereade: but... maybe we don't make that distinction | 09:40 |
| rogpeppe | fwereade: we just make Destroy idempotent | 09:40 |
| fwereade | rogpeppe, I think it's ok -- Destroy might cause removal right away or it might not, but should always succeed absent corrupt state or network problems; removal | 09:40 |
| rogpeppe | fwereade: so when a uniter sees Dying, it'll clean up stuff, then call Destroy again | 09:40 |
| fwereade | rogpeppe, I surely hope not | 09:41 |
| rogpeppe | fwereade: and Destroy will itself call ensureDying on itself | 09:41 |
| rogpeppe | fwereade: ? | 09:42 |
| fwereade | rogpeppe, I don't understand what you're expecting Destroy to do -- but I *think* that while the unit is still assigned to a machine, it should do nothing, and the machine is the thing responsible for calling RemoveUnit once the unit is dead | 09:43 |
| fwereade | rogpeppe, so the unit does somehow cause itself to become Dead | 09:43 |
| fwereade | rogpeppe, but that is, I think, not as simple as justsetting a flag | 09:43 |
| rogpeppe | fwereade: if the user calls juju remove-unit, what should that call in state? | 09:45 |
| fwereade | rogpeppe, Unit.Destroy() | 09:45 |
| rogpeppe | fwereade: " while the unit is still assigned to a machine, it should do nothing" | 09:45 |
| rogpeppe | fwereade: so how does remove-unit work then? | 09:45 |
| fwereade | rogpeppe, calling it *again* should do nothing, because no interesting state has changed apart from the unit being Dying rather than Dead, which I don't think is relevant | 09:46 |
| rogpeppe | fwereade: ok, so how does the unit actually get removed from state? | 09:47 |
| fwereade | rogpeppe, it depends... either by Destroy (if it's not assigned to anything and therefore has no responsible entity) or by the MA once the unit has set itself to Dead | 09:48 |
| rogpeppe | fwereade: how does the MA do it? | 09:48 |
| fwereade | rogpeppe, RemoveUnit? | 09:48 |
| rogpeppe | fwereade: so we've got Destroy, which *may* call RemoveUnit, and RemoveUnit, which is *sometimes* appropriate to call | 09:49 |
| rogpeppe | fwereade: i'm wondering if we need RemoveUnit | 09:49 |
| fwereade | rogpeppe, we definitely need RemoveUnit, how else are we going to handle service removal? | 09:50 |
| fwereade | rogpeppe, (that's not implemented either ofc :/) | 09:50 |
| rogpeppe | fwereade: what if Destroy removes the unit when it can? | 09:50 |
| fwereade | rogpeppe, it does -- in that case, that also has to be responsible for service cleanup | 09:51 |
| rogpeppe | fwereade: why's that? | 09:51 |
| fwereade | rogpeppe, who else is going to do it? | 09:51 |
| rogpeppe | fwereade: (removing a unit never removes a service, does it?) | 09:51 |
| fwereade | rogpeppe, it should | 09:51 |
| rogpeppe | fwereade: i disagree | 09:51 |
| rogpeppe | fwereade: it's perfectly ok to have a service with no units | 09:51 |
| fwereade | rogpeppe, no shit | 09:52 |
| rogpeppe | fwereade: sure, we can have that right now | 09:52 |
| fwereade | rogpeppe, it's not ok to have a dying service with no units because nothing else is going ot clean it up | 09:52 |
| fwereade | rogpeppe, service is basically the same as relation | 09:52 |
| rogpeppe | fwereade: ah, if the service is dying, sure | 09:52 |
| fwereade | rogpeppe, we need the same txn dance in AddUnit/RemoveUnit/Destroy as we have for EnterScope/LeaveScope/Destroy with relations | 09:52 |
| rogpeppe | fwereade: so what's wrong with having Unit.Destroy also responsible for service cleanup, if it finds that the service is dying. | 09:54 |
| rogpeppe | ? | 09:54 |
| fwereade | rogpeppe, (sorry, I am feeling extremely grumpy about this, because I feel like I've been misled repeatedly about the impact of various decisions and that I have basically spent the last 3 months clearing up the shit that has been thrown over the wall to me under pretence of being "finished", and AFAICT I am going to have to basically rewrite state in order to complete subordinates) | 09:55 |
| fwereade | rogpeppe, Destroy can do that if the unit's unassigned, yes, but it's not the only thing that might need to do that | 09:55 |
| rogpeppe | fwereade: to be fair, i think nobody understood the problem that well before, and it's difficult to write code to solve someone else's future problem. | 09:56 |
| fwereade | rogpeppe, in practice it's going to be something like unit.removeOps() that gets composed into bigger txns in a couple of places | 09:57 |
| rogpeppe | fwereade: it's just like clearing the last refcount, yes? when anything that can be blocking a dying thing from become dead becomes dead itself, it must try to destroy the things that it might be blocking | 09:57 |
| fwereade | rogpeppe, yeah, exactly | 09:57 |
| fwereade | rogpeppe, this is not a non-obvious consequence of the model | 09:57 |
| fwereade | rogpeppe, it is something I was talking about back in lisbon 1 | 09:58 |
| rogpeppe | fwereade: it's actually very similar to the refcounting i was talking about a long time ago | 09:58 |
| fwereade | rogpeppe, and I think I am somewhat legitimately pissed off that the problem has just been ignored | 09:58 |
| rogpeppe | fwereade: i always felt a bit fuzzy (in a not good way) about the lifecycle stuff, but i felt i didn't have enough handle on the actual problem to propose something better. | 09:59 |
| rogpeppe | fwereade: how about this: delete all Remove* calls, and replace them with Destroy calls. | 10:00 |
| rogpeppe | fwereade: also remove all EnsureDead and EnsureDying methods | 10:00 |
| rogpeppe | fwereade: then Destroy always has a clear responsibility - move towards removing the entity from state. | 10:01 |
| fwereade | rogpeppe, I don't think I'm willing to put my weight behind an alternative one-size-fits-all solution -- I think that's the fundamental prblem | 10:01 |
| rogpeppe | fwereade: and any entities that might also be in the process of destruction that depend on it. | 10:02 |
| rogpeppe | fwereade: can you think of anything that can't be made fit that model, in a reasonably sane way? | 10:03 |
| fwereade | rogpeppe, different kinds of unit destruction? | 10:03 |
| fwereade | rogpeppe, especially since niemeyer has rejected the txn idea for that | 10:03 |
| rogpeppe | fwereade: i really think that's something that's a mode of dying that can be implemented as a flag on the unit itself. | 10:04 |
| fwereade | rogpeppe, and wants to add new lifecycle flags and have multiple responsible entites running concurrently instead | 10:04 |
| fwereade | rogpeppe, well, that is a possibility | 10:05 |
| rogpeppe | fwereade: i think that whether to use transactions or not is up to the implementation of each entity | 10:05 |
| rogpeppe | fwereade: i think Destroy as a catch-all entry point could make sense. | 10:06 |
| rogpeppe | fwereade: because that's what we're actually trying to do, after all | 10:06 |
| fwereade | rogpeppe, I just don't get in what universe it is sane to have two different pieces of code fighting over which one is responsible for the backing state | 10:06 |
| rogpeppe | fwereade: (well, we'd probably have ForceDestroy on units) | 10:06 |
| fwereade | rogpeppe, as an entry point, yeah, I think I'm +1 | 10:07 |
| rogpeppe | fwereade: i think we're inevitably going to have two different pieces of code fighting over responsibility, because agents can become temporarily (and perhaps permanently) divorced from the state they're supposed to be responsible for | 10:08 |
| rogpeppe | fwereade: and that's the big issue around --force | 10:08 |
| fwereade | rogpeppe, right -- but we agreed a long time ago that the unit should watch for its own Deadness, and that that would be the signal that it was to Stop Doing Everything | 10:09 |
| fwereade | rogpeppe, Life=Dead is, explicitly, the responsibility handover point | 10:09 |
| rogpeppe | fwereade: sure. just as Dying is such a signal, right? | 10:09 |
| rogpeppe | fwereade: (is there any difference between Dying and Dead there, in fact?) | 10:10 |
| fwereade | rogpeppe, if we set a unit to Dead the worst that will happen is one spurious error and restart before it cleanly shuts down | 10:10 |
| fwereade | rogpeppe, if we do some funky dying flag then we have two entities fighting over who gets to do what | 10:10 |
| fwereade | rogpeppe, and I refuse to believe that that is a sensible way to manage things | 10:10 |
| rogpeppe | fwereade: do you think you can do the entire unit clean up (leaving all relations) in a single transaction? | 10:11 |
| rogpeppe | pw | 10:11 |
| fwereade | rogpeppe, I think it'll be hard, and ugly, but I don't think it'll be impossible, or noticeably uglier than any other options | 10:12 |
| rogpeppe | fwereade: surely all you need to do is make leaving scope idempotent? | 10:13 |
| rogpeppe | fwereade: then who cares if two entities are doing it at the same time? | 10:13 |
| rogpeppe | fwereade: and it doesn't need to be hard, or ugly. | 10:13 |
| rogpeppe | fwereade: and it's a sufficiently rare case that efficiency is of no concern at all | 10:14 |
| fwereade | rogpeppe, it offends me that a unit agent can have state that it depends on secretly whipped out from under its feet by another entity | 10:16 |
| rogpeppe | fwereade: but that's what force is all about, surely? | 10:16 |
| rogpeppe | fwereade: and it's not great, but sometimes it's the only option | 10:17 |
| fwereade | rogpeppe, it's about not giving the charm notice to clean up -- not about fucking up our internal state | 10:17 |
| fwereade | rogpeppe, I think that if there's some way we can implement it without dropping ideas of ownership out of the window, we should do so | 10:17 |
| rogpeppe | fwereade: we're talking force majeur here. that's not an issue, i believe. | 10:17 |
| rogpeppe | fwereade: this will only be used when we really don't care about the unit's internal state any more | 10:18 |
| fwereade | rogpeppe, I may be misunderstanding what you mean by "unit's internal state", but I don't think we can ever say we don't care about it... | 10:19 |
| rogpeppe | fwereade: if i use remove-unit --force, surely that's giving license to the system to remove the unit regardless of its current state. | 10:20 |
| fwereade | rogpeppe, yeah -- but that doesn't mean we can ignore that state, the removal still needs to keep state consistent | 10:21 |
| rogpeppe | fwereade: even if you use transactions, you're still potentially whipping state out from under the unit agent's feet | 10:21 |
| fwereade | rogpeppe, yeah, in a way that uniter has been designed to handle from the beginning | 10:22 |
| fwereade | rogpeppe, because we discussed this, and agreed it, and so I implemented it | 10:22 |
| rogpeppe | fwereade: so what are you worried that it *won't* deal with correctly? | 10:23 |
| rogpeppe | fwereade: when a unit is dying, what needs to happen before it actually dies? is it just that it needs to leave relation scopes? | 10:23 |
| fwereade | rogpeppe, and deal with its subordinates | 10:23 |
| fwereade | rogpeppe, which will themselves have scopes to leave | 10:24 |
| rogpeppe | fwereade: so if a unit finds itself dying, it sets its subords to dying and waits for them to die before dying itself? | 10:26 |
| fwereade | rogpeppe, the necessary txn will not be entirely trivial to compose, but I really think that adding more states for more entities and distributing the cleanup responsibilities amongst more entities (each of which could themselves fail at arbitrary times) is a bad move | 10:27 |
| fwereade | rogpeppe, I think that setting a unit to Dying should also set its subordinates to Dying | 10:27 |
| rogpeppe | fwereade: is that possible to do in one transaction? | 10:28 |
| fwereade | rogpeppe, I think a unit should not be able to become dead while it has subordinates... | 10:28 |
| fwereade | rogpeppe, don't see why not | 10:28 |
| fwereade | rogpeppe, what problems are you anticipating? | 10:28 |
| * rogpeppe reminds himself of the transaction capabilities | 10:30 | |
| rogpeppe | fwereade: don't you need to know the doc id that will be updated before starting the transaction? | 10:32 |
| rogpeppe | fwereade: so you can't do an equivalent of "UPDATE life=Dying WHERE principal==myUnit" | 10:32 |
| fwereade | rogpeppe, no: I do it for everything in doc.Principals, assert no changes to what I composed the txn from, and retry on abort | 10:34 |
| rogpeppe | fwereade: ah, so you could assert that subordinates haven't changed (by looking in unitDoc.Subordinates) | 10:35 |
| rogpeppe | fwereade: so, just to be clear, what state *have* you anticipated changing under the feet of the uniter? | 10:40 |
| fwereade | rogpeppe, Life becoming Dead | 10:42 |
| fwereade | rogpeppe, this can cause arbitrary failures if we're unlucky, but I *think* that's fine, because when we come up again we handle Dead correctly | 10:42 |
| fwereade | rogpeppe, basically, when the unit becomes Dead, it will complete its current operation if possible, and not do anything else ever again | 10:44 |
| rogpeppe | fwereade: so to make that work, you need to set dead and remove all relations, all in the same transaction? | 10:44 |
| rogpeppe | s/remove/leave/ | 10:44 |
| fwereade | rogpeppe, yeah | 10:45 |
| fwereade | rogpeppe, but the relations are not the hard bit | 10:45 |
| fwereade | rogpeppe, well, they're not trivial | 10:45 |
| rogpeppe | fwereade: oh yeah, and set subords to dead and leave all their relations too | 10:45 |
| rogpeppe | fwereade: all in the same transaction | 10:45 |
| fwereade | rogpeppe, but nor are they going to be impossible to handle -- it's doing all that for N units that really bothers me | 10:45 |
| rogpeppe | fwereade: istm this might be pushing the boundaries of the transaction system a little | 10:45 |
| fwereade | rogpeppe, do you have any idea what those boundaries actually are? how many ops is too many? | 10:47 |
| rogpeppe | fwereade: i suppose the uniter could see that the "force" flag is set, and treat that as the same as it currently sees Dead | 10:47 |
| fwereade | rogpeppe, yeah, indeed, but niemeyer was -1 on that for no reason I could determine | 10:47 |
| rogpeppe | fwereade: no idea at all. just that a thousand might pushing it, and isn't beyond the bounds of possibility | 10:48 |
| rogpeppe | fwereade: apart from anything else, it'll act as a bottleneck in the system until the whole transaction goes through | 10:48 |
| fwereade | rogpeppe, yeah, I don't have a clear enough idea of the details to know what sort of impact that'll have | 10:50 |
| rogpeppe | fwereade: it seems like something we should avoid if we can | 10:51 |
| fwereade | rogpeppe, yeah, maybe, I just feel like the alternatives are being handwaved a bit | 10:54 |
| rogpeppe | fwereade: you're right, they are, 'cos we don't know the uniter details like you do :-) | 10:55 |
| fwereade | rogpeppe, heh, yeah | 10:57 |
| rogpeppe | fwereade: istm that if there was something the uniter could see that it would treat as it currently treats Dead but isn't *actually* Dead, then things could work ok | 10:57 |
| rogpeppe | fwereade: and it wouldn't change too many of the current uniter assumptions | 10:57 |
| fwereade | rogpeppe, yeah, but niemeyer rejected that suggestion :/ | 10:57 |
| rogpeppe | fwereade: perhaps you didn't phrase it in a sufficiently sympathetic way :-) | 10:58 |
| fwereade | rogpeppe, it's really tedious doing development with all these invisible walls | 10:58 |
| rogpeppe | fwereade: agreed. did you ever look at my api spike branch BTW? | 10:58 |
| fwereade | rogpeppe, I did, but I didn't quite get my head around it properly I'm afraid :( | 10:59 |
| rogpeppe | fwereade: that's fine. you're not the only one. | 10:59 |
| fwereade | rogpeppe, :) | 10:59 |
| rogpeppe | fwereade: probably means it really is crack after all | 10:59 |
| rogpeppe | fwereade: the rpc thing is perhaps too magical | 10:59 |
| rogpeppe | fwereade: but it does make it *really* easy to write rpc servers... | 11:00 |
| fwereade | rogpeppe, I'm not sure -- I just wasn't able to flush my other thoughts and properly engage with it | 11:01 |
| rogpeppe | fwereade: gustavo's not keen; i'm not entirely surprised. | 11:02 |
| Aram | I've looked at your branch, though not as thoroughly to write a review. I like the way you are supposed to use the abstractions provided. feels very easy to me. I can't comment on the abstraction implementation yet, though. | 11:04 |
| rogpeppe | Aram: the implementation is still (deliberately) very sketchy currently, although still reasonably neat. in particular it doesn't deal with any kind of concurrency at all yet., | 11:05 |
| rogpeppe | Aram: thanks for the response though - that's the first positive remark i've had yet! | 11:06 |
| Aram | I don't think your branch is overkill. I believe that the problem at hand is not trivial, and doing it ad hoc is opening a can of worms. using your thing I'm sure we could change it in time as needed, but an ad hoc wrapper is bound by its initial implementation. | 11:08 |
| rogpeppe | Aram: that's my thought too. but building general mechanisms and then using them seems to frowned upon in these parts. | 11:09 |
| rogpeppe | Aram: there are a few things the hierarchical nature of the rpc thingy makes potentially possible too - caching being one of them. and you can get a reasonable-looking HTTP interface for free too, even if you decide to use gob for agent communication. | 11:11 |
| Aram | going to buy some bread. | 11:13 |
| niemeyer | Morning all | 11:14 |
| niemeyer | Aram: Have a good, erm.. bread? :) | 11:14 |
| rogpeppe | niemeyer: hiya | 11:14 |
| niemeyer | rogpeppe: Yo | 11:14 |
| niemeyer | fwereade: Pingous | 11:15 |
| fwereade | niemeyer, heyhey | 11:15 |
| niemeyer | fwereade: Heya | 11:15 |
| fwereade | niemeyer, how's it going? | 11:15 |
| niemeyer | fwereade: I think a machine should be Dyiable (ugh) when there are still units | 11:15 |
| niemeyer | Erm | 11:15 |
| niemeyer | Should not | 11:15 |
| rogpeppe | niemeyer: +1 | 11:16 |
| niemeyer | fwereade: I thought we already enforced that constraint | 11:16 |
| fwereade | niemeyer, cool, I think the consequences are unpleasantr | 11:16 |
| niemeyer | fwereade: But perhaps it got lost in the changes | 11:16 |
| fwereade | niemeyer, state enforces almost no constraints :/ | 11:16 |
| fwereade | niemeyer, ok, that is an overstatement | 11:16 |
| niemeyer | fwereade: Really? .. Okay :-) | 11:16 |
| rogpeppe | niemeyer: we've never set any constraints on setting something to Dying, only Dead, i think. | 11:16 |
| niemeyer | rogpeppe: The constraint was on removal | 11:17 |
| niemeyer | rogpeppe: I think | 11:17 |
| niemeyer | At least I have fresh memories (perhaps fake) that it was impossible to remove a machine with units in it | 11:17 |
| rogpeppe | niemeyer: that's not quite the same thing as setting it to dying though | 11:17 |
| niemeyer | rogpeppe: It actually is, in terms of intent | 11:18 |
| niemeyer | rogpeppe: I mean what we previously did as "remove" and what we now do as "set to dying" | 11:18 |
| niemeyer | if topology.has_machine(internal_id): | 11:19 |
| niemeyer | if topology.machine_has_units(internal_id): | 11:19 |
| niemeyer | raise MachineStateInUse(machine_id) | 11:19 |
| niemeyer | Memories aren't entirely fake, at least | 11:19 |
| rogpeppe | niemeyer: yeah, it doesn't look like we make any checks like that currently | 11:20 |
| rogpeppe | niemeyer: even in RemoveMachine | 11:20 |
| rogpeppe | niemeyer: which is definitely wrong | 11:20 |
| rogpeppe | niemeyer: if we can't set a machine to dying unless it has no units, do we actually need Dying and Dead for a machine? we could just remove it. | 11:21 |
| rogpeppe | niemeyer: (assuming removal will fail if the machine has units) | 11:21 |
| niemeyer | rogpeppe: I don't understand how that constraint changes things with regards to the machine lifecycle | 11:22 |
| niemeyer | rogpeppe: It's still not alright to remove state under agents that are running | 11:22 |
| rogpeppe | niemeyer: true | 11:23 |
| niemeyer | - if hasUnits { | 11:25 |
| niemeyer | - return fmt.Errorf("machine has units") | 11:25 |
| niemeyer | - } | 11:25 |
| niemeyer | - return t.RemoveMachine(key) | 11:25 |
| niemeyer | RemoveMachine was actually implemented like that | 11:26 |
| rogpeppe | niemeyer: in cmd/juju ? | 11:26 |
| niemeyer | rogpeppe: In state | 11:26 |
| niemeyer | This was lost with the implementation of lifecycle support | 11:26 |
| rogpeppe | niemeyer: ah. that's racy of course, but still. i'm surprised we didn't have tests for that behaviour | 11:27 |
| niemeyer | rogpeppe: No, it actually wasn't | 11:27 |
| fwereade | niemeyer, rogpeppe: I think removal is the PA's responsibility, so we do need Dead | 11:27 |
| rogpeppe | niemeyer: ah, 'cos it was in a retry loop? | 11:27 |
| niemeyer | rogpeppe: Yep | 11:27 |
| niemeyer | fwereade: Yeah, the constraint is a minor red-herring that we do need still | 11:28 |
| rogpeppe | could someone remind me again why agents can remove their own entities? | 11:28 |
| rogpeppe | s/can/can't/ | 11:28 |
| niemeyer | rogpeppe: A unit container doesn't magically go away because it removed the entity, same thing for a machine | 11:28 |
| niemeyer | rogpeppe: While the resource still exists, the respective management object should also exist | 11:29 |
| rogpeppe | niemeyer: ok, that's a useful maxim | 11:29 |
| fwereade | niemeyer, rogpeppe: and I'm not even certain that Dying is *really* meaningless -- but I don't know what a machine agent's responsibility is other than to set itself to Dead | 11:29 |
| niemeyer | fwereade: Agreed, it can easily receive attributes that have to be accomplished before death | 11:30 |
| fwereade | niemeyer, rogpeppe: given that we agree that it shoudn't have units when it's set to Dying | 11:30 |
| rogpeppe | fwereade: it provides a hook for the machine agent to do something more sophisticated later | 11:30 |
| niemeyer | We haven't really invested much in the machine management so far | 11:30 |
| niemeyer | fwereade: Well, actually.. | 11:31 |
| fwereade | niemeyer, rogpeppe: so I *think* that for machines we have constraints on EnsureDying, and a Machiner that just sets itself to Dead at an opportune moment, and that can be extended in future | 11:31 |
| niemeyer | fwereade: Removing units! | 11:31 |
| fwereade | niemeyer, ah, we don't agree that? | 11:31 |
| fwereade | niemeyer, the units must be removed before the machine is set to Dying, mustn't they? | 11:31 |
| fwereade | niemeyer, so, yeah, the machiner won't want to set itself to dead while it still has units deployed | 11:32 |
| fwereade | niemeyer, but it won't become Dying until that process is underway | 11:32 |
| niemeyer | fwereade: It's a good question, actually | 11:32 |
| niemeyer | fwereade: But that's perhaps an easier first step | 11:33 |
| fwereade | niemeyer, would you expand? sounds like I'm missing something :) | 11:33 |
| niemeyer | fwereade: In theory it would be fine to set a machine to dying when all units are also dying | 11:33 |
| fwereade | niemeyer, ah, yes, good point | 11:33 |
| fwereade | niemeyer, I don't think that hurts us much, though, we still need to wait to recall all our units before we can become dead | 11:34 |
| fwereade | niemeyer, whoops no that's crack | 11:34 |
| fwereade | niemeyer, they won't be removed from state until they're recalled | 11:34 |
| niemeyer | fwereade: Sure, but how's that an issue? | 11:35 |
| fwereade | niemeyer, with the current model, we need to wait for that to happen before we can make the machine Dying, so we dodge that problem | 11:35 |
| niemeyer | fwereade: Right | 11:35 |
| fwereade | niemeyer, if we allow dying machine with dying units, then we need to hang around | 11:35 |
| niemeyer | fwereade: Which is easier, agreed | 11:35 |
| niemeyer | fwereade: Yep | 11:35 |
| niemeyer | We don't have to do that now | 11:35 |
| fwereade | niemeyer, you know, I think we do, for the uniter with subordinates | 11:36 |
| fwereade | niemeyer, maybe I'm wrong, but I think that setting a unit to dying should also set all it subordinates to dying | 11:41 |
| niemeyer | fwereade: +1 | 11:41 |
| niemeyer | fwereade: Or, | 11:41 |
| niemeyer | fwereade: It can reject, but that sounds a bit weird | 11:42 |
| niemeyer | fwereade: Setting subordinates to dying is probably the right thing to do | 11:42 |
| Aram | niemeyer, bread with sweet crispy bacon goodness. | 11:42 |
| fwereade | niemeyer, I'm pretty sure that's wrong -- the only other way to get rid of the subordinates is to kill the relation, isn't it? | 11:42 |
| niemeyer | fwereade: Indeed | 11:43 |
| fwereade | niemeyer, cool | 11:43 |
| niemeyer | Aram: Poor bread! ;-) | 11:44 |
| * rogpeppe should probably have some breakfast too | 11:46 | |
| mramm | good morning all | 12:54 |
| fwereade | mramm, heyhey | 12:57 |
| TheMue | mramm: Morning | 12:57 |
| mramm | who all is planning to come to this morning's team meeting? | 12:59 |
| mramm | I will add you to the invite | 12:59 |
| fwereade | mramm, I am | 12:59 |
| dimitern | mramm: me too | 12:59 |
| Aram | me too | 13:05 |
| Aram | mramm, can you add me | 13:07 |
| Aram | ? | 13:07 |
| Aram | fwereade, dimitern ^ ^ | 13:08 |
| dimitern | Aram: i'm not sure how? | 13:08 |
| Aram | ping mark | 13:09 |
| mramm | done | 13:09 |
| Aram | thx | 13:09 |
| mramm | dimitern: you are loved | 13:12 |
| dimitern | :) | 13:12 |
| Aram | mac os x sucks | 13:14 |
| mramm | aram: http://mergy.org/2012/12/irecognition-i-am-no-longer-apples-target-market/ | 13:21 |
| fwereade | niemeyer, btw, there was something else I vaguely wanted to mention re LXC: AIUI, we *can* run, eg, precise i386 containers on quantal amd64... and this strikes me as an interesting complication for constraints (and the Deployer...) that I had not hitherto considered | 13:47 |
| fwereade | niemeyer, but I don't think I'm in a position to anticipate it properly atm, and I guess I can't usefully worry about it until we have some actual containers to use | 13:47 |
| fwereade | niemeyer, just running the host OS will be a lot simpler and get us a long way | 13:48 |
| niemeyer | fwereade: Agreed. I think the very first thing we should do is to not even worry about containers, but just do plain co-location | 14:05 |
| fwereade | niemeyer, +1 | 14:05 |
| niemeyer | fwereade: I did consider the use of containers across releases before, but I did the same as you, and thought "Yeah, we should support that.", but discarded any kind of deep thinking as premature | 14:05 |
| fwereade | niemeyer, yeah, I'd only really considered it re local provider before | 14:06 |
| fwereade | niemeyer, rogpeppe, TheMue: https://codereview.appspot.com/6862053 is a rough proposal for making state testing a little clearer: it's not ready to go in, because it we're going to use it we should do so consistently, but I think the general idea is can be judged as it stands | 14:27 |
| rogpeppe | fwereade: looking | 14:27 |
| fwereade | niemeyer, rogpeppe, TheMue: some tests it helps with more than others ofc | 14:27 |
| TheMue | fwereade: Will hava a look | 14:27 |
| rogpeppe | fwereade: have you changed any of the test semantics in that CL? | 14:30 |
| fwereade | rogpeppe, I am confident that everything that was tested before is still tested, but the details have changed in a couple of places | 14:31 |
| rogpeppe | fwereade: i'd be happier to check this large CL if the only changes, for this round at least, were just changing the existing tests to use the new primitives. | 14:32 |
| rogpeppe | fwereade: otherwise i feel i'm trying to check two things at once, and there are quite a lot of changes. | 14:32 |
| fwereade | rogpeppe, true -- the other side of it is that I tried to do that, but some of the tests were too painful to maintain like that :) | 14:35 |
| rogpeppe | fwereade: how about a CL that just changes the tests that you feel are worth changing, and reserve refactoring the other ones for another CL (maybe several)? | 14:36 |
| fwereade | rogpeppe, if the general idea is roughly accepted then when it's ready I imagine I will do it as a series of single-test-file CLs on top of one that introduces the vocabulary | 14:36 |
| rogpeppe | fwereade: ah, ok | 14:36 |
| rogpeppe | fwereade: in which case i'm +0.5. i can see it makes the tests briefer, but it's also less clear to see what's going on each individual test. | 14:37 |
| fwereade | rogpeppe, so not entirely without pain -- but I *think* it's easier to mentally check equivalence between X and Z directly than between X and Y and Y and Z in two steps | 14:37 |
| fwereade | rogpeppe, that is interesting to me | 14:37 |
| fwereade | rogpeppe, because my experience has been that the sheer number of characters in play is the biggest obstacle to understanding a test I'm not familiar with | 14:38 |
| rogpeppe | fwereade: maybe you're right. and the test primitives do have quite intuitive mappings to underlying operations | 14:39 |
| fwereade | rogpeppe, and ISTM that there's enough repetition that learning a small simple vocabulary once is a clear win, even when some of the verbs only save a line | 14:39 |
| rogpeppe | fwereade: yeah. | 14:39 |
| TheMue | I'm off for now, will return later. | 14:41 |
| niemeyer | fwereade: Sent a few comments | 14:43 |
| fwereade | niemeyer, cool, thanks | 14:43 |
| niemeyer | fwereade: I'm being asked for lunch, so will have to continue afterwards | 14:43 |
| fwereade | niemeyer, np, enjoy :) | 14:43 |
| rogpeppe | niemeyer: here's a bare-minimum API CL, just to get us off the ground. hopefully we can agree on at least this much as a starting point. https://codereview.appspot.com/6888048 | 16:20 |
| niemeyer | rogpeppe: Looking | 16:20 |
| niemeyer | rogpeppe: That's a great bootstrap | 16:28 |
| rogpeppe | niemeyer: cool | 16:29 |
| rogpeppe | niemeyer: it's still entirely compatible with what i did before, of course :-) :-) | 16:29 |
| niemeyer | rogpeppe: I bet :) | 16:29 |
| niemeyer | fwereade: What is a deployer.Context? | 16:32 |
| niemeyer | fwereade: I'm a bit confused by the fact there's a DeployerName | 16:32 |
| niemeyer | fwereade: I see some debate there as well, so maybe we'd benefit from a quick exchange on it | 16:33 |
| fwereade | niemeyer, I'm really sorry -- cath's aunt is just about to arrive -- I will try to grab you later if that's ok? | 16:34 |
| niemeyer | fwereade: Oh, surely | 16:34 |
| niemeyer | fwereade: I'll see if I can comment on the CL itself, actually, in a way that does not bring confusing | 16:34 |
| niemeyer | confusion | 16:34 |
| fwereade | niemeyer, cheers :) | 16:34 |
| fwereade | niemeyer, lovely, thanks | 16:34 |
| rogpeppe | niemeyer: DeployerName is the entity that the deployer is working on behalf of. in practice it will be the machine's entity name or a principal unit's entity name. | 16:37 |
| niemeyer | rogpeppe: I understand that much.. I don't understand why it's part of the interface it's in | 16:37 |
| rogpeppe | niemeyer: it's so that the deployer can know if it should should remove a deployed unit from the state when it disappears from the watcher list | 16:38 |
| rogpeppe | niemeyer: i think | 16:38 |
| rogpeppe | niemeyer: i chatted with fwereade a bit ago about this | 16:39 |
| fwereade | niemeyer, sorry, back briefly -- the main reason is that I don't want to pass the deployer name (and maybe state info) into every context method | 16:41 |
| fwereade | niemeyer, it seemed more obfuscatory than helpful to do so, despite the slightly odd arrangement | 16:41 |
| fwereade | niemeyer, the Context is the thing that knows all about how to deploy; the Deployer just tells it what to deploy | 16:42 |
| fwereade | niemeyer, maybe it would be cleaner to just drop it from the interface and duplicate it in the Deployer | 16:43 |
| niemeyer | fwereade: I'm still catching up on the details of how the interfaces are used, so I don't know for sure, but I'll add a note in either case just to let you know of how it felt | 16:45 |
| niemeyer_ | fwereade: Review is out | 17:11 |
| niemeyer_ | fwereade: It's great step forward, thanks | 17:11 |
| fwereade | niemeyer_, awesome, cheers, all looks good -- really off for now :) | 17:19 |
| fwereade | niemeyer_, (and, yeah, Manager works for me, it's just a name I basically dismiss out of hand :)) | 17:20 |
| rogpeppe | dimitern: you've got a review: https://codereview.appspot.com/6877054/ | 17:33 |
| dimitern | rogpeppe: thanks! | 17:33 |
| dimitern | rogpeppe: in fact, I want to get rid of the interface altogether, but first I'll finish the refactoring of the tests | 17:34 |
| rogpeppe | dimitern: +1 | 17:34 |
| dimitern | I'm making them more granular with a few ensureExists, ensureNotExist and remove - taking interface{} and calling the appropriate underlying API | 17:35 |
| rogpeppe | dimitern: in general it's best to avoid functions/methods that use interface{}. why do you need it here? | 17:35 |
| dimitern | rogpeppe: I'll glad if you take a look once I propose the changes (tomorrow probably) | 17:35 |
| dimitern | I have this http://paste.ubuntu.com/1415180/ | 17:37 |
| dimitern | rogpeppe: ^^ | 17:37 |
| rogpeppe | dimitern: i'm not convinced that's much gain over small simple statically typed functions tbh | 17:38 |
| dimitern | rogpeppe: well, if I don't have this then I end up with these huge test cases (Add/Has/Get/Remove in one, to be idempotent) | 17:40 |
| rogpeppe | dimitern: i'm not arguing against helper functions, just that it's better IMHO to go with static typing where you can. something like this, for example? http://paste.ubuntu.com/1415205/ | 17:48 |
| dimitern | rogpeppe: ok, can you explain why this is better? I'm not arguing, just curious | 17:49 |
| rogpeppe | dimitern: then each function is evidently trivial | 17:49 |
| rogpeppe | dimitern: it's better because the compiler will tell you when you get it wrong | 17:50 |
| dimitern | rogpeppe: I see, so it's better to write more code if it's static vs. less more generic code with type checks | 17:51 |
| dimitern | more, but simpler, that is | 17:51 |
| rogpeppe | dimitern: yeah. but in this case it might actually be less code in some case too | 17:52 |
| rogpeppe | dimitern: ensureNotExist is 23 lines but as separate functions (not counting blank lines) it's 20 | 17:53 |
| dimitern | rogpeppe: ok, I like it :) I'll change it | 17:53 |
| rogpeppe | dimitern: every time i see "interface{}" i have to do a double-take, and wonder what's going on and what might be really happening at runtime. | 17:54 |
| dimitern | rogpeppe: like unhandled cases and unknown types, in addition - probably some run-time reflection overhead | 17:55 |
| rogpeppe | dimitern: definitely. although performance is no issue here, of course. | 17:56 |
| rogpeppe | dimitern: BTW your ensureExists function is a bit odd for the name it's given, especially paired with ensureNotExist. ensureExists makes sure that something exists; ensureNotExist checks if something doesn't currently exist. i'd probably call it "add" :-) | 17:57 |
| dimitern | rogpeppe: I was unsure what's the proper name myself :) but yeah, add or create seems more appropriate given what it does | 17:59 |
| rogpeppe | niemeyer_: i've just been thinking about how we might manage our migration to using the API | 18:26 |
| rogpeppe | niemeyer_: i think we can do it quite nicely in an incremental way and reap some of the benefits early on | 18:27 |
| rogpeppe | niemeyer_: my thought is to implement jujud server, and get it running, even though we have no (well, one!) API calls yet. | 18:27 |
| rogpeppe | niemeyer_: then add an APIState field to juju.Conn, and have juju.NewConn connect to the API state too. | 18:28 |
| rogpeppe | fwereade: how does that sound to you? | 18:29 |
| rogpeppe | mramm: i think this is similar to the approach you were suggesting, and i think it's actually not much problem to do. | 18:30 |
| mramm | rogpeppe: yea, that's what I was thinking about in general | 18:31 |
| mramm | having an incremental migration path for clients | 18:31 |
| rogpeppe | then we can gradually migrate anything that uses juju.Conn to use calls in Conn.APIState (juju status might be a nice early candidate, as it would be significantly speeded up by the API) | 18:31 |
| rogpeppe | finally, state/api can be renamed to state, and the existing State API can be made a private part of it. | 18:33 |
| rogpeppe | anyway, it's eod for me now | 18:33 |
| rogpeppe | mramm, all: g'night | 18:34 |
| niemeyer_ | rogpeppe: What woudl be APIState? | 18:53 |
| niemeyer_ | rogpeppe: Either way, big +1 on the overall idea of having it working end-to-end from the get-go | 18:53 |
| rogpeppe | niemeyer_: APIState would be the type i've defined as api.State | 19:25 |
| === niemeyer_ is now known as niemeyer | ||
| mramm | I will likely be about 5 minutes late for our team meeting... Can somebody else start up the google hangout? | 21:53 |
| davecheney | hello, is it meeting time ? | 22:05 |
| wallyworld | davecheney: i started a hangout https://plus.google.com/hangouts/_/01030a125de04a2af6a02a552ef3090109b438d5?authuser=0&hl=en | 22:06 |
| wallyworld | mramm: ^^^^ | 22:06 |
| davecheney | wallyworld: can you add me as a super friend on g+, https://plus.google.com/u/1/103860539990722557379/posts | 22:09 |
| wallyworld | sure | 22:09 |
| davecheney | two secs | 22:12 |
| davecheney | sound is AFU | 22:12 |
| mramm | kk | 22:12 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!