| davecheney | fwereade: aloha! | 06:50 |
|---|---|---|
| fwereade | davecheney, heyhey | 06:50 |
| fwereade | davecheney, how's it going? | 06:50 |
| davecheney | good | 06:50 |
| davecheney | spent the weekend looking at ARM assembly | 06:51 |
| davecheney | so a change is as good as a holiday | 06:51 |
| fwereade | davecheney, excellent :) | 06:54 |
| TheMue | Morning. | 07:33 |
| fwereade | heya TheMue | 07:33 |
| fwereade | TheMue, ping | 08:05 |
| TheMue | fwereade: pong | 08:05 |
| fwereade | TheMue, figuring out what I should work on; wondering how the relation stuff is going | 08:05 |
| TheMue | fwereade: One moment please, have to change rooms. | 08:06 |
| fwereade | TheMue, np | 08:06 |
| TheMue | fwereade: So, back again. My wife has been in the home office before. ;) | 08:10 |
| TheMue | fwereade: Currently the State.AddRelation() and State.RemoveRelation() are in review and short before LGTM. | 08:11 |
| TheMue | fwereade: But Roger and Gustavo discussed and want to keep open, that a service might have relations of the same role mltiple times in future. | 08:12 |
| TheMue | fwereade: So I'll now change the topology for it and afterwards the AddRelation() and RemoveRelation() to use this new model. | 08:12 |
| fwereade | TheMue, ok, cool, so not *imminent* -- cheers :) | 08:14 |
| TheMue | fwereade: Next step will then be to enhance AddRelation() by a better verification of the endpoints. Today it has the same checks as the Py code, but it's not enough. | 08:14 |
| TheMue | fwereade: What is not so imminent? | 08:16 |
| fwereade | TheMue, sorry, got diverted; I mean that a usable state.Relation type is still some way off | 08:32 |
| TheMue | fwereade: There's enough to do, yes. | 08:33 |
| fwereade | TheMue, cool; I should probably do something that doesn't depend on it then :) | 08:33 |
| TheMue | fwereade: Or participate. ;) | 08:33 |
| fwereade | TheMue, if you think the work can be usefully divided that would be cool -- what's the plan? | 08:34 |
| TheMue | fwereade: That's the problem, there is no real plan. I'm just doing the functionality top-down. I've started with the relation manager, those methods move into our new State. | 08:35 |
| TheMue | fwereade: Next step would be relation type by type. | 08:35 |
| TheMue | fwereade: I've seen that the service relation has a lot of functionality. | 08:36 |
| fwereade | TheMue, I could happily try to do some rudimentary types | 08:36 |
| fwereade | TheMue, see how far I can take them | 08:36 |
| fwereade | TheMue, what's the latest stable-ish unmerged relations branch? if you think some will go in soon, it seems sensible to branch from there | 08:37 |
| TheMue | fwereade: The latest is go-state-remove-relation. But it has a line of prerequisites. | 08:38 |
| fwereade | TheMue, and is that likely to be merged by the time I have a new type to propose on top of it? and not to change too much between now and then? | 08:39 |
| TheMue | fwereade: So maybe you should wait until the all are in the trunk. | 08:39 |
| TheMue | fwereade: I hope to keep the API stable, as well as the types so far. | 08:40 |
| TheMue | fwereade: Right now only the topology below is changed. | 08:40 |
| fwereade | TheMue, ok; in that case I can probably just go from trunk, right? | 08:40 |
| TheMue | fwereade: In trunk there's the current topology, but not yet the relation types as well as Add and Remove. | 08:41 |
| fwereade | TheMue, I'm just re-checking your current proposals to get myself more up to speed | 08:42 |
| TheMue | fwereade: Yes, take a look. I think both will go in today or tomorrow. | 08:43 |
| TheMue | fwereade: Just wait for the LGTM. The Add… had a pre-LGTM by Gustavo. | 08:43 |
| fwereade | TheMue, hmm, so I should maybe branch from add-relation and add functionality to ServiceRelation? | 08:43 |
| TheMue | fwereade: Sounds like a plan. ;) In Remove… it only has one method more to return the according Relation. | 08:44 |
| fwereade | TheMue, cool, I'll ping you if I have any trouble | 08:45 |
| TheMue | fwereade: Cheers. | 08:46 |
| TheMue | fwereade: You'll see that those zkXyz are now topoXyz, as we talked about in Oakland. | 08:46 |
| fwereade | TheMue, sweet | 08:46 |
| fwereade | TheMue, just mailed juju-dev; would apreciate your thoughts | 10:34 |
| TheMue | fwereade: Will take a look. | 10:35 |
| TheMue | fwereade: To summarize: Interface changes (with checks) are ok, but no subordinate changes. Am I right? | 10:38 |
| fwereade | TheMue, that's what I *think* we should do | 10:38 |
| fwereade | TheMue, you tell me whether that's right :) | 10:39 |
| TheMue | fwereade: Hehe, then you have to help me with subordinates before. Where do we find it in our model? | 10:40 |
| fwereade | TheMue, we don't really yet | 10:40 |
| fwereade | TheMue, I think that I need to incorporate them before we can move much further ahead on the relations | 10:41 |
| TheMue | fwereade: Could you please tell me more about subordinates? | 10:41 |
| fwereade | TheMue, I'm just changing Service.AddUnit to take (container *Unit) and seeing where I go from there | 10:41 |
| fwereade | TheMue, ok, I'll try | 10:41 |
| fwereade | TheMue, charms can be subordinate | 10:42 |
| fwereade | TheMue, if they are, their characteristics change | 10:42 |
| fwereade | TheMue, when deployed, no units are created | 10:42 |
| TheMue | fwereade: Do you have an example? | 10:42 |
| fwereade | TheMue, when added to a relation, units of the subordinate are deployed inside the containers of every unit of the principal service | 10:43 |
| fwereade | TheMue, rsyslog would make a good subordinate charm | 10:43 |
| fwereade | TheMue, there are a number of other consequences which I only really discovered today | 10:44 |
| fwereade | TheMue, and am not quite sure I have a handle on | 10:44 |
| fwereade | TheMue, so stop me if anything sounds like crack | 10:44 |
| fwereade | TheMue, one of the issues is that the "connections" are different to in a normal relation | 10:45 |
| fwereade | TheMue, in a normal relation between P and Q, every unit of P can see every unit of Q, and react to settings changes; and vice-versa | 10:45 |
| TheMue | fwereade: It's ok so far. I only try to understand it right. Typically I've used to work a lot with my colleagues around a whiteboard. I need to visualize it. | 10:45 |
| fwereade | TheMue, in a subordinate relation, each unit of P sees only the unit of Q that it shares a container with | 10:46 |
| fwereade | TheMue, and settings changes are processed entirely within, and do not leak beyond, that container | 10:46 |
| fwereade | TheMue, (everything is still mediated by ZK, obviously; but a settings change for one unit of P will not e noted by a unit of Q outside its container | 10:47 |
| fwereade | TheMue, so... when storing unit relations, we have different ZK paths to the settings etc | 10:48 |
| TheMue | fwereade: IC | 10:48 |
| fwereade | TheMue, we currently handle the /relations/rel-id case, which is what we want globally; but we also need /relations/rel-id/unit-id (I think) | 10:49 |
| * fwereade goes to check | 10:49 | |
| fwereade | TheMue, yeah; and the unit-id I referred to above is specifically the unit id of the principal (ie the one that "owns" the container | 10:51 |
| fwereade | ) | 10:51 |
| TheMue | fwereade: In Py there's code handling /relations/rid/cid/role | 10:51 |
| TheMue | fwereade: Where cid is the container id | 10:51 |
| fwereade | TheMue, and also /relations/rid/role, right? | 10:51 |
| TheMue | fwereade: For global relations, yes. | 10:52 |
| fwereade | TheMue, AIUI the structure of the nodes underneath that should be similar | 10:52 |
| fwereade | TheMue, does that seem right to you? | 10:52 |
| fwereade | TheMue, look for _get_scope_path and how it's used | 10:53 |
| TheMue | fwereade: Thought it is, but not yet covered the container scoped relations. | 10:54 |
| fwereade | TheMue, I'll proceed as though it is; can't be sure of every use of it yet but we'll see how it goes | 10:55 |
| fwereade | TheMue, the topology also changes to accommodate knowledge of what container a given unit is actually deployed | 10:56 |
| fwereade | TheMue, in | 10:56 |
| fwereade | TheMue, anyway, I'm just making those changes to begin with | 10:56 |
| TheMue | fwereade: So this is also a topology change to the current Py version? | 10:58 |
| fwereade | TheMue, no, it's part of the topology already in py | 10:58 |
| TheMue | fwereade: OK, then I got you wrong, sorry. | 10:58 |
| TheMue | Lunchtime | 11:04 |
| Aram | heyhey. | 11:54 |
| TheMue | Aram: Hiya. | 12:11 |
| hazmat | g'morning | 12:16 |
| fwereade | Aram, hazmat: heyhey | 12:25 |
| fwereade | heya niemeyer | 12:39 |
| niemeyer | Yo | 12:39 |
| niemeyer | ! | 12:39 |
| fwereade | niemeyer, TheMue, Aram: conundrum follows | 12:51 |
| fwereade | in the topology, in python, we store Machine and Container for each unit | 12:52 |
| fwereade | only one of those can ever be valid | 12:52 |
| fwereade | I just got an urge to store Location, which is just a unit key or a machine key (and we can tell which is which, right?) | 12:53 |
| fwereade | this feels kinda evil, but so does allowing the content to be insane by having both or neither set | 12:53 |
| fwereade | niemeyer, TheMue, Aram: opinions? ^^ | 12:53 |
| fwereade | niemeyer, TheMue, Aram: I'm leaning towards storing them under separate keys and just being careful about what we write, but... | 12:54 |
| niemeyer | fwereade: Having a single key feels bad indeed | 12:55 |
| fwereade | niemeyer, TheMue, Aram: hmm, *at the moment* only one can ever be correct | 12:55 |
| fwereade | niemeyer, TheMue, Aram: forget I said anything :) | 12:55 |
| niemeyer | fwereade: They're different fields meaning different things.. having to guess what the field means based on the value isn't great | 12:55 |
| fwereade | niemeyer, indeed | 12:55 |
| fwereade | niemeyer, we could still have garbage set in that one field anyway | 12:55 |
| TheMue | fwereade: Forgotten. ;) | 12:55 |
| niemeyer | fwereade: can you please expand? Maybe I misunderstand | 12:56 |
| fwereade | niemeyer, I'm agreeing -- the extra guard against insanity of the icky magic field doesn't eliminate the possibility of insanity anyway, so so even that is not an argument for the icky magic field over two separate fields | 12:57 |
| * fwereade should go and borrow a cuddly toy from laura to ask these questions to before polluting irc :) | 12:57 | |
| niemeyer | fwereade: Hehe. Jamu used to call that "talking to the bear" :-) | 12:58 |
| fwereade | niemeyer, yeah, it's a noble technique, we had a cat called mutex at resolver who sometimes served that purpose | 12:58 |
| TheMue | fwereade: Hehe, or take your pet to talk to (if you got one). Cats are good listeners. | 12:59 |
| fwereade | TheMue, just a cuddly cat actually | 12:59 |
| fwereade | TheMue, I'm not sure a real cat would have the patience anyway ;) | 12:59 |
| TheMue | fwereade: I'm sure -- it doesn't. *lol* | 12:59 |
| niemeyer | fwereade: That's a great name for a cat :) | 13:03 |
| fwereade | niemeyer, isn't it :) | 13:04 |
| TheMue | niemeyer: One question about the topo service inversion. Shall I open the combination of roles even if e not yet use them or shall I keep the current validation for provider/requirer or peer? | 13:33 |
| niemeyer | TheMue: The validation is fine | 13:33 |
| niemeyer | TheMue: I mean, it will have to be adapted, of course | 13:33 |
| niemeyer | TheMue: Given the map change | 13:33 |
| TheMue | niemeyer: Sure, but I'll keep it and later we can extend it. OK. | 13:34 |
| niemeyer | TheMue: But we don't have to pretend it's general right now.. we just don't want to shoot ourselves in the foot | 13:34 |
| TheMue | niemeyer: Could hurt, indeed. ;) | 13:34 |
| TheMue | niemeyer: So, change is done. Influence on State.AddRelation() is small, only three lines. | 14:04 |
| fwereade | niemeyer, TheMue: https://codereview.appspot.com/6268050 has subordinate units | 14:21 |
| niemeyer | fwereade: Woohay | 14:23 |
| niemeyer | TheMue: Woohay too :) | 14:23 |
| TheMue | :D | 14:23 |
| fwereade | niemeyer, er, there'll be a small diff upcoming, missed a signature change in the rest of the project | 14:23 |
| fwereade | niemeyer, should be a trivial additional file changed | 14:23 |
| fwereade | niemeyer, ie shouldn't block sane reviewing if you're of a mind :) | 14:24 |
| niemeyer | fwereade: Not yet.. still trying to catch up on post-sprint stuff | 14:25 |
| niemeyer | fwereade: But I'll get there | 14:25 |
| fwereade | niemeyer, no worries :) | 14:25 |
| fwereade | niemeyer, btw, tyvm for reviewing what you did during the sprint, hope it didn't take too much time away | 14:27 |
| niemeyer | fwereade: No problem. It didn't take much time away because I've used the evenings for the largest chunks | 14:28 |
| fwereade | niemeyer, jolly good, thanks :) | 14:28 |
| fwereade | niemeyer, TheMue, Aram: oh, blast: I've put myself in a situation in which I need two prereqs for the next branch | 14:30 |
| fwereade | niemeyer, TheMue, Aram: did we ever come up with a nice way of dealing with this? | 14:30 |
| niemeyer | fwereade: Is any of them small enough that I can review quickly? | 14:30 |
| niemeyer | fwereade: There's no way to handle that automatically with current infra | 14:31 |
| niemeyer | fwereade: Launchpad doesn't support it | 14:31 |
| fwereade | niemeyer, one of them is TheMue's add-relation branch | 14:31 |
| niemeyer | fwereade: Hmm | 14:31 |
| fwereade | niemeyer, which IIRC has an LGTM from you already actually | 14:31 |
| niemeyer | fwereade: I thought that was in already | 14:31 |
| TheMue | fwereade: There's only a final lgtm missing. | 14:31 |
| niemeyer | Link? | 14:31 |
| TheMue | niemeyer: I only proposed it for the final one after the last open points. | 14:32 |
| TheMue | niemeyer: https://codereview.appspot.com/6223055/ | 14:32 |
| TheMue | niemeyer, fwereade: I'm ready for submitting it. | 14:33 |
| fwereade | TheMue, if it has an LGTM you should be good to go unless you disagreed with any of the points in the LGTM | 14:34 |
| fwereade | TheMue, I have been known to be too hesitant on this front :) | 14:34 |
| * TheMue likes clean LGTMs. ;) | 14:35 | |
| * fwereade lives in hope of them, and is most joyful when they come to pass | 14:35 | |
| niemeyer | TheMue: LGTM | 14:35 |
| TheMue | niemeyer: Ah, thx, here it comes. | 14:35 |
| fwereade | niemeyer, thanks, sorry context switch :( | 14:36 |
| niemeyer | fwereade point applies in this case, FWIW.. your branch reflected the agreement of the last review | 14:36 |
| niemeyer | fwereade: No problem, happy to prevent blockage | 14:36 |
| TheMue | niemeyer: Updated the latest propose (topology). Now the new trunk containing State.AddRelation() is merged, the changes adopted and all lamps are green. | 15:01 |
| niemeyer | TheMue: Beautiful, thank you | 15:02 |
| niemeyer | ! | 15:02 |
| TheMue | niemeyer: np :D | 15:02 |
| niemeyer | mramm: ping | 20:31 |
| mramm | pong | 21:45 |
| mramm | neimeyer: pong | 21:46 |
| niemeyer | mramm: Heya | 22:00 |
| niemeyer | mramm: Are you subscribed to juju-dev? | 22:01 |
| niemeyer | davecheney: ping | 22:02 |
| davecheney | niemeyer: ack | 22:18 |
| davecheney | hang on | 22:18 |
| davecheney | let me change terminals | 22:18 |
| davecheney | x has shat itself | 22:18 |
| davecheney | niemeyer: back | 22:21 |
| niemeyer | davecheney: Heya | 22:21 |
| niemeyer | davecheney: Morning | 22:21 |
| davecheney | yes! finally it wasn't raining, so it was my first chance to go for a ride before work for weeks! | 22:21 |
| davecheney | niemeyer: how was your trip ? | 22:21 |
| niemeyer | davecheney: Oh, congrats! :) | 22:21 |
| niemeyer | davecheney: It was great indeed.. very happy that I managed to meet Aram and rogpeppe in addition to the unrelated sprint | 22:22 |
| davecheney | niemeyer: didn't you meet roger at UDS ? | 22:22 |
| davecheney | :P | 22:22 |
| davecheney | btw, is Aram the Aram taht wrote doozer ? | 22:23 |
| niemeyer | davecheney: Yeah, but it was great even then.. we managed to cover some interesting ground there | 22:23 |
| niemeyer | davecheney: He's the Aram that patched doozer for persistence, yeah | 22:23 |
| niemeyer | ;) | 22:23 |
| davecheney | niemeyer: i had a great day with rog when we went to the computer history museum | 22:23 |
| niemeyer | davecheney: By the way, I've pushed a gozk branch that does the semantic change I mentioned | 22:23 |
| davecheney | 5 hours on SF public transport left ample time for getting to know each other | 22:23 |
| davecheney | niemeyer: sweet, i'll go get -u now | 22:24 |
| niemeyer | davecheney: I didn't submit yet, but if you LGTM it I'll push it on | 22:24 |
| davecheney | niemeyer: does the notification go to gophers ? | 22:24 |
| davecheney | i haven't seen an email | 22:24 |
| niemeyer | davecheney: I'd hope so | 22:24 |
| davecheney | i guess not | 22:24 |
| davecheney | let me double check | 22:25 |
| davecheney | https://codereview.appspot.com/6292044 | 22:25 |
| davecheney | but no email, i just foudn that from your reply to my handover note | 22:25 |
| niemeyer | Hmm | 22:28 |
| niemeyer | Weird | 22:28 |
| davecheney | am i a member of golang-gophers? or just the juju project ? | 22:28 |
| niemeyer | davecheney: I'm somewhat concerned with the lack of tests in the provisioner brancher.. there are tons of edge cases there that are being carefully considered but untested | 22:40 |
| niemeyer | davecheney: I'm sure you've tested by hand or at least put good thought on these edge cases | 22:41 |
| davecheney | niemeyer: yes, i wonder how I can decompose the functions to test them | 22:42 |
| niemeyer | davecheney: Have you used TDD before? | 22:42 |
| davecheney | nope | 22:42 |
| niemeyer | davecheney: Ok.. there are a couple of good books that might be useful.. I like the one from Kent Beck, but just last week I've received good feedback on Growing Object-Oriented Software, Guided by Tests by Steve Freeman | 22:43 |
| niemeyer | davecheney: I suspect Kent Beck might be more to-the-point in terms of TDD specifically, though | 22:43 |
| niemeyer | davecheney: Regarding decomposing the functions, that may not be necessary | 22:44 |
| niemeyer | davecheney: I'm more concerned about testing the real edge cases than the *implementation* | 22:44 |
| niemeyer | davecheney: A simple example: if a misconfigured environment somehow arrives, the new configuration is ignored, but a follow up configuration actually takes place | 22:45 |
| niemeyer | davecheney: We can integrate this branch before that, though.. I'm happy to have those increments nicely done on top of that, given that what we have in place was well debated over already | 22:46 |
| davecheney | niemeyer: cool | 22:46 |
| niemeyer | davecheney: Right now I'm just doing some minor commenting on trivial stuff like log messages and whatnot | 22:46 |
| davecheney | niemeyer: ... wondering how to observe a misconfigured environment being rejected | 22:47 |
| niemeyer | davecheney: What we care about is not that it is rejected, but that the provisioner itself stays up | 22:47 |
| niemeyer | davecheney: and that a follow up correct configuration is loaded | 22:48 |
| davecheney | niemeyer: gotcha | 22:48 |
| davecheney | understood | 22:48 |
| davecheney | making a list now | 22:49 |
| davecheney | * provisioning does not occur with broken env | 22:49 |
| davecheney | * provisioning does occur when env is valid, then broken | 22:49 |
| davecheney | * provisioning does occur when env is valid, then broken, then valid | 22:49 |
| davecheney | niemeyer: are you comfortable with me integrating those into the woodenman branch ? | 22:50 |
| niemeyer | davecheney: woodenman? Do we already have another branch up? Sorry, I'm still catching up | 22:50 |
| davecheney | yeah, strawman is the basic provisioning agent that just obseverves environment changes | 22:51 |
| davecheney | woodenman is the follow on that actually pokes the Environ when it needs to add machines, etc | 22:51 |
| niemeyer | davecheney: I'd prefer to fix these outstanding issues before we increase the functionality | 22:52 |
| niemeyer | davecheney: We have quite a few pending things there | 22:52 |
| davecheney | sure | 22:52 |
| niemeyer | davecheney: Tests, state issues within the loop (maybe?), state reconnection | 22:52 |
| davecheney | ok | 22:52 |
| niemeyer | davecheney: If we add more logic, we'll be increasing the debt | 22:53 |
| niemeyer | davecheney: It's not clear to me we need IsValid anymore, btw, assuming the gozk branch | 22:53 |
| niemeyer | davecheney: We'll have to think about it.. | 22:53 |
| niemeyer | davecheney: Well.. and maybe a test | 22:53 |
| davecheney | niemeyer: no, as long as the watchers close, then I can return to the Run function, tear everything down, and start again | 22:54 |
| davecheney | niemeyer: re: gozk, I guess that looks good | 22:56 |
| davecheney | i don't think i can comment authoratively | 22:57 |
| niemeyer | davecheney: Actually, we may still need a note from the state | 23:02 |
| davecheney | niemeyer: that is what I thought of from IsValid() | 23:02 |
| niemeyer | davecheney: Yeah, may be good to be explicit | 23:02 |
| davecheney | so the innerLoop becomes for state.IsValid() { .... } | 23:03 |
| niemeyer | davecheney: Yeah, or IsConnected | 23:03 |
| davecheney | niemeyer: yup, i'm not fussed about the name | 23:04 |
| niemeyer | davecheney: I know, just brainstorming as we go | 23:04 |
| davecheney | I had the idea that that would return false after some event that caused the state to be 'broken' | 23:04 |
| niemeyer | davecheney: We can watch the session channel for that | 23:05 |
| niemeyer | davecheney: Should be an easy walk | 23:05 |
| niemeyer | davecheney: Anything returning event.Ok() != true can set the flag to red | 23:05 |
| niemeyer | davecheney: Actually, now that I think of it.. I don't think the complexity of machinesChanges and environChanges makes sense anymore, with those decisions | 23:08 |
| niemeyer | davecheney: Just imagine in which circumstances these channels will break | 23:09 |
| davecheney | niemeyer: you mean making them recreate themselves | 23:09 |
| niemeyer | davecheney: Oh, duh.. please ignore me | 23:09 |
| niemeyer | davecheney: No, wait.. I'm confused | 23:09 |
| niemeyer | davecheney: These channels are internally reestablished.. | 23:10 |
| niemeyer | davecheney: Yeah, the initial idea seems to proceed | 23:10 |
| niemeyer | davecheney: We only need to recreate on !ok, right? | 23:10 |
| davecheney | niemeyer: they are only recreated if, a, they were nil to begin with (just started up) or b, their Stop() method was called, in response to them being closed | 23:13 |
| niemeyer | davecheney: Yeah, in that second scenario, it doesn't really make sense to bring them back | 23:14 |
| niemeyer | davecheney: If they're dead, the state itself must be dead too | 23:14 |
| davecheney | niemeyer: with your change, that is now true | 23:14 |
| niemeyer | davecheney: That said, it's a trivial simplification we can do in a second moment.. not hurting reliability, so let's move on | 23:15 |
| davecheney | they anre't recreated util you go through the loop again, so if the loop has a condition that is false if the state is closed | 23:15 |
| davecheney | they wont' be recreated | 23:15 |
| niemeyer | davecheney: It was always true.. | 23:15 |
| niemeyer | davecheney: We've changed !ok to happen more frequently, but it was always the case that with !ok the state is dead | 23:16 |
| davecheney | niemeyer: absolutely, just those channels never closed | 23:16 |
| niemeyer | davecheney: Exactly, and stop is only called when they closed | 23:16 |
| davecheney | so, we can use !ok as a proxy for state is closed | 23:16 |
| davecheney | ok, that makes things simpler | 23:16 |
| niemeyer | davecheney: Yeah, I think so too | 23:17 |
| davecheney | if a watcher closes, it's underlying state is closed, so collapse back to the top, cleaning up as we go, then start again | 23:17 |
| niemeyer | davecheney: Right | 23:17 |
| niemeyer | davecheney: I think we pretty much never have to retry in place | 23:17 |
| niemeyer | davecheney: These scenarios are edge cases anyway, and we need to handle harsher scenarios too | 23:17 |
| niemeyer | davecheney: So we can safely handle hiccups in conservative ways | 23:18 |
| davecheney | niemeyer: i also wanted to ask you about retring | 23:18 |
| davecheney | retrying | 23:18 |
| niemeyer | davecheney: and proceed as if they were major breakups | 23:18 |
| davecheney | roger suggested there was some retry logic in the ec2 code, but unless he's talking about goamz, I couldn't find it | 23:18 |
| niemeyer | davecheney: Hopefully we'll also be better off when the major break does happen | 23:18 |
| niemeyer | davecheney: No, there is indeed, inside environs/ec2 | 23:19 |
| davecheney | grep -i -r retry didn't help me, i'll try again | 23:19 |
| niemeyer | davecheney: grep for shortAttempt | 23:19 |
| davecheney | ok | 23:19 |
| niemeyer | davecheney: Should give you the seed | 23:19 |
| davecheney | i want to reuse that for the loop at the top of the PA, so we dont' jump straight back in | 23:19 |
| niemeyer | davecheney: Nice; feel free to refactor it off | 23:20 |
| davecheney | niemeyer: ok | 23:20 |
| davecheney | niemeyer: https://launchpad.net/~gophers << i'm still pending approval | 23:22 |
| davecheney | :( | 23:22 |
| niemeyer | davecheney: Hah, sorry | 23:22 |
| niemeyer | davecheney: That explains it | 23:22 |
| * davecheney feels slighted | 23:22 | |
| niemeyer | davecheney: Ok, review delivered | 23:22 |
| davecheney | thank you very much | 23:23 |
| davecheney | who is going to look at IsConnected ? | 23:23 |
| niemeyer | davecheney: I think the branch can go in with those changes, but I'm fine as well if you decide to refactor it further with some of these ideas we just discussed | 23:23 |
| davecheney | niemeyer: I would liek to land it today, if there are no objections, then follow up with the other things we have discussed | 23:24 |
| niemeyer | davecheney: Feel free to do it if you feel like it, or we can ask TheMue to have a look at it | 23:24 |
| davecheney | niemeyer: re IsConneced, i'll take a look today and hand over to TheMue at 4pm if I get stuck | 23:24 |
| niemeyer | davecheney: Okay, so let's move ahead.. fixing the comments/messages/etc, so we can get it in with a state we're both happy with | 23:25 |
| davecheney | ok | 23:25 |
| niemeyer | davecheney: Okay.. the idea is really simple: we have a loop with event, ok <- session in open.go already.. we just have to add a check to see if event.Ok() for each event received, and set a protected variable in case it's false | 23:25 |
| niemeyer | davecheney: You can find some Stop/Start tests within zk_test.go that may be useful | 23:26 |
| niemeyer | davecheney: Look for Reconnect in the test name | 23:26 |
| niemeyer | / .. but we take all session events that occur before a session is established | 23:27 |
| niemeyer | as errors. | 23:27 |
| davecheney | niemeyer: in state/open.go ? | 23:27 |
| niemeyer | davecheney: Hmm | 23:27 |
| niemeyer | davecheney: The comment seems clear.. all session event on non-session watches are fatal | 23:28 |
| davecheney | i guess i don't understand what a zk session is | 23:28 |
| niemeyer | davecheney: THere are two kinds of watches: a requested watch (e.g. GetW), and the session watch that is obtained via Dial | 23:29 |
| niemeyer | davecheney: Both of these receive events | 23:29 |
| davecheney | so a session watch observes everything, a getw is scoped to a path ? | 23:29 |
| niemeyer | davecheney: The session watch receive *only* connectivity state changes | 23:29 |
| davecheney | ahh, of course, events about this session, this connection | 23:30 |
| niemeyer | davecheney: The session watch receives both these and the actual requested watch | 23:30 |
| niemeyer | davecheney: Yeah.. I call them session events because they actually have a code of EVENT_SESSION | 23:30 |
| niemeyer | davecheney: It is confusing indeed, sorry | 23:31 |
| niemeyer | davecheney: Is there a way I can improve that comment in a more understandable way? | 23:31 |
| davecheney | niemeyer: no, it makes sense now. | 23:32 |
| davecheney | so if you're a watcher, and you receive an EVENT_SESSION, it's a signal that you should close | 23:33 |
| davecheney | your comment is correct | 23:33 |
| davecheney | afk 15 mins | 23:36 |
| niemeyer | davecheney: | 23:38 |
| niemeyer | // All session events on non-session watches will be delivered | 23:38 |
| niemeyer | // and cause the watch to be closed early. We purposefully do | 23:38 |
| niemeyer | // that to enforce a simpler model that takes hiccups as | 23:38 |
| niemeyer | // important events that cause code to reestablish the state | 23:38 |
| niemeyer | // from a pristine and well known good start. | 23:38 |
| niemeyer | if event.State == STATE_CONNECTED { | 23:38 |
| niemeyer | // That means the watch was established while we were still | 23:38 |
| niemeyer | // connecting to zk, but we're somewhat strict about only | 23:38 |
| niemeyer | // dealing with watches when in a well known good state. | 23:38 |
| niemeyer | event.State = STATE_CONNECTING | 23:38 |
| niemeyer | } | 23:39 |
| niemeyer | davecheney: Hope that's more understandable | 23:39 |
| davecheney | +1e9 | 23:45 |
| davecheney | niemeyer: my only change would be | 23:45 |
| davecheney | // important events which cause code to reestablish the state | 23:46 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!