| wallyworld | jam: 1 on 1? | 06:05 |
|---|---|---|
| jam | wallyworld: sure, guess I missed my alarm | 06:05 |
| jam | wallyworld: I just lost mumble connection, will try again | 07:04 |
| wallyworld | ok | 07:04 |
| jam | wallyworld: it seems dead from my view... Are you still on mumble.canonical.com? | 07:05 |
| wallyworld | yes, i'll retry also | 07:05 |
| TheMue | Morning | 07:05 |
| jam | wallyworld: it is possible the great firewall of the UAE has been updated to block mumble :). | 07:05 |
| jam | TheMue: hi | 07:05 |
| jam | clearly I still have some internet access. | 07:05 |
| wallyworld | jam: when i restart, it says host unreachble for me too | 07:05 |
| jam | k | 07:06 |
| jam | maybe just a dns issue? | 07:06 |
| wallyworld | so maybe mumble is down | 07:06 |
| jam | (migrating to a new host, and DNS updates being done.) | 07:06 |
| TheMue | jam: Hi, and yes, seems so. | 07:06 |
| jam | I got remote host closed it, connected now, but invalid password. | 07:06 |
| jam | maybe they are just restarting it or something | 07:06 |
| wallyworld | works now | 07:06 |
| davecheney | morning/evening | 07:07 |
| TheMue | davecheney: Hiya | 07:16 |
| jam | and mumble dies again... | 07:44 |
| * davecheney sad trombone | 07:44 | |
| jam | TheMue: How's stuff going? I saw that you're working on the firewalling code. Mark mentioned that you'll be starting to look into the MaaS code soon. Do you know when that might be? | 07:45 |
| jam | I think I also saw that you're going on vacation next week? | 07:45 |
| jam | davecheney: http://www.sadtrombone.com/ | 07:45 |
| jam | sorry: http://www.sadtrombone.com/?play=true | 07:45 |
| jam | the latter will autopaly | 07:45 |
| jam | play | 07:45 |
| * davecheney bookmarks | 07:46 | |
| jam | davecheney: though I wonder if you need a bit.ly link to it, so you can surprise people. | 07:46 |
| TheMue | jam: Yes, right now I'm fixing/fixed the firewaller. | 07:46 |
| davecheney | jam lmgtfy | 07:46 |
| TheMue | jam: I already took a first look into MAAS, but now I have to coordinate with Aram who started with it too. | 07:47 |
| Aram | good morning. | 07:49 |
| davecheney | morning | 07:50 |
| TheMue | Aram: Hiya | 07:50 |
| * Aram installed Debian because Unity doesn't work with NX. | 07:51 | |
| Aram | I forgot computers could be this fast. | 07:51 |
| davecheney | debian is quite lean | 07:51 |
| * davecheney has been playing with plan9 on a Pi | 07:51 | |
| Aram | I'm still waiting for my Pis. I somehow managed to order Pis in Australia, no wonder no Pi came. | 07:52 |
| Aram | perhaps I donated some Pis to someone this way. | 07:52 |
| davecheney | Aram: another canonical guy sold me his 512mb pi | 07:53 |
| davecheney | do you want my old 256mb one ? | 07:53 |
| davecheney | apart from running acme on your telly, it isn't all that useful | 07:53 |
| Aram | nah, thanks, I think I'll get one in a week or so. | 07:53 |
| davecheney | understood | 07:53 |
| Aram | yeah, I suppose/. | 07:53 |
| * davecheney is still waiting on the TTL 3.3v usb adapter | 07:54 | |
| * TheMue is playing with Smalltalk again and always wonders about the Power of such a small environment like Pharo. Funny how ST has been too large for most computers in its early days. | 07:54 | |
| === TheMue_ is now known as TheMue | ||
| davecheney | Aram: over the weekend some significant improvements were made on compiler memory usage | 08:03 |
| davecheney | there are CL's in review that effectively halve the amount of memory the compiler uses | 08:03 |
| Aram | fantastic. | 08:03 |
| davecheney | yup, pkg/net now takes ~ 43mb on 32bit platforms | 08:03 |
| davecheney | down from 108 | 08:04 |
| davecheney | 103 on 8g | 08:05 |
| davecheney | 5g is slightly larger because the Prog structure and Reg list is longer | 08:05 |
| rogpeppe | morning all! | 08:13 |
| Aram | hi. | 08:13 |
| TheMue | rogpeppe: Hi | 08:17 |
| rogpeppe | afk while the boiler man is fixing our boiler... hopefully. | 09:25 |
| TheMue | Anyone interest in reviewing the changed firewaller? https://codereview.appspot.com/6875053/ | 09:26 |
| TheMue | interested | 09:26 |
| TheMue | Aram: Do you want to take a look on the changed firewaller? https://codereview.appspot.com/6875053/ | 09:40 |
| TheMue | s/on/at/ | 09:40 |
| jam | mgz: /wave | 09:57 |
| TheMue | Aram: Having fun doing installations with following reboots? ;) | 09:57 |
| Aram | TheMue: just configuring NX. | 09:59 |
| TheMue | Aram: IC | 09:59 |
| rogpeppe | fwereade: ping | 10:25 |
| TheMue | rogpeppe: He's out this morning. | 10:31 |
| rogpeppe | TheMue: ah, thanks | 10:31 |
| TheMue | rogpeppe: yw | 10:31 |
| jam | rogpeppe: I've been meaning to ask. What does "CL" actually stand for? | 11:01 |
| rogpeppe | jam: change list | 11:02 |
| rogpeppe | jam: (i believe) | 11:02 |
| jam | it doesn't seem to be a very good fit, IMO, but as long as it is known. :) | 11:02 |
| rogpeppe | jam: yeah, i don't think it matters too much. | 11:02 |
| dimitern | rogpeppe: do you think having a single mutex for all maps and other internal structures will suffice to overcome possible race conditions, or it's better to have one mutex per member (I'm referring to my CL for nova double) | 11:05 |
| jam | dimitern: my suggesstion is to just put the mutex in ServeHTTP like the ec2 version | 11:06 |
| rogpeppe | jam: +1 | 11:06 |
| rogpeppe | jam: do you really need to export all those Nove functions? | 11:06 |
| rogpeppe | s/Nove/Nova/ | 11:06 |
| dimitern | jam, rogpeppe: but dave pointed out that since the API is public this might not be enough? | 11:06 |
| rogpeppe | s/jam/dimitern/ | 11:06 |
| rogpeppe | dimitern: that's my question: does this API need to be public? | 11:07 |
| rogpeppe | dimitern: who are you seeing as the clients of this API? | 11:07 |
| dimitern | rogpeppe: well, since they're used for tests only, unless we use them outside the package, they can be private | 11:07 |
| rogpeppe | dimitern: that's what i thought | 11:08 |
| rogpeppe | dimitern: i'd suggest making them private | 11:08 |
| jam | I'd slightly suggest that it is being a bit over prescriptive, but we can start there. | 11:08 |
| rogpeppe | dimitern: then we can ignore concurrency issues for the tests (because we know we're not calling them concurrently) and the ServeHTTP mutex will be sufficient. | 11:08 |
| rogpeppe | jam: if we don't do that, i think we'll probably end up needing a different entry point for each one of those calls, one for internal use (with lock held) and one for external use | 11:09 |
| dimitern | rogpeppe: ok, seems reasonable | 11:09 |
| rogpeppe | jam: given that almost all the functionality will be provided through the http interface, i'm not sure that's necessary. | 11:10 |
| rogpeppe | jam: but maybe you see some utility to the entry points that i don't? | 11:11 |
| jam | rogpeppe: I just think it is slightly overly prophylactic to say that you shouldn't have any public methods in case someone might use them incorrectly. But we can always go the 'its default private and we'll make it public when we need them' | 11:13 |
| jam | I'm also a bit more used to Python where everything that is "private" is still public. | 11:13 |
| jam | (consenting adults approach) | 11:14 |
| rogpeppe | jam: given that it's an http server, i'm not sure it's possible to use the methods correctly without second-guessing what the server is doing behind the scenes (which breaks modularity) | 11:15 |
| rogpeppe | jam: (because it's perfectly legitimate for the server to have something concurrent going on) | 11:16 |
| jam | rogpeppe: it is possible that in testing you would take out the HTTP section of the communication because it isn't always relevant for the tests themselves. (the fact that goose talks HTTP to openstack *should* be an implementation detail). | 11:17 |
| jam | ATM, we do, and we do for the forseeable future | 11:17 |
| jam | so we can go that route. | 11:17 |
| jam | but if HTTP overhead starts slowing down the tests, it would make sense to pull it out as uninteresting | 11:17 |
| rogpeppe | jam: it's one of those areas in Go that we tend to be very careful about, because unlocked concurrent access is a source of hard-to-find bugs. | 11:17 |
| rogpeppe | jam: if that happens to be the case (which i think is fairly unlikely), post-facto refactoring to make this possible would not be hard. | 11:19 |
| rogpeppe | jam: also, we have had times when the http section of the communication has contributed to test failures, so it's worth keeping it in the loop IMHO. | 11:21 |
| TheMue | Lunchtime, biab. | 11:28 |
| dimitern | rogpeppe: PTAL https://codereview.appspot.com/6877054 | 12:46 |
| rogpeppe | dimitern: looking | 12:47 |
| dimitern | jam: you too pls ^^ | 12:58 |
| rogpeppe | dimitern: replied | 13:02 |
| dimitern | rogpeppe: thanks! | 13:03 |
| dimitern | rogpeppe: was not sure *x.y when x is *T and y is *W will be the same as *x.*y | 13:05 |
| rogpeppe | dimitern: you can't do *x.*y :-) | 13:05 |
| dimitern | rogpeppe: yeah, but you got my point :) | 13:05 |
| rogpeppe | *x.y is always the same as *(x.y) | 13:05 |
| rogpeppe | dimitern: if x is a pointer, x.y is the same as (*x).y | 13:06 |
| dimitern | rogpeppe: I was confused, because before x was T and y was *W, hence the parens | 13:06 |
| dimitern | rogpeppe: I see, ok, good to know | 13:06 |
| rogpeppe | dimitern: so when both x and y are pointers, *x.y is the same as *((*x).y) | 13:06 |
| dimitern | rogpeppe: and no, flavors and servers must have links according to the OS API | 13:07 |
| rogpeppe | dimitern: so perhaps building links automatically if none are given is reasonable behaviour | 13:08 |
| rogpeppe | dimitern: but probably no need to distinguish []Link{} and nil there. | 13:08 |
| rogpeppe | dimitern: BTW for chapter and verse on selector expressions, see http://golang.org/ref/spec#Selectors | 13:09 |
| dimitern | rogpeppe: ok, so I can change it so it checks for len(links) = 0 and adds them | 13:09 |
| rogpeppe | dimitern: that seems reasonable to me, assuming it does to jam | 13:09 |
| dimitern | rogpeppe: but if Links []Link is nil, will len(Links) make sense? | 13:10 |
| rogpeppe | dimitern: absolutely | 13:10 |
| rogpeppe | dimitern: you can len([]T(nil)) and range over it too | 13:10 |
| dimitern | rogpeppe: good!\ | 13:11 |
| rogpeppe | dimitern: same with maps | 13:11 |
| rogpeppe | dimitern: and channels actually | 13:11 |
| rogpeppe | dimitern: it simplifies quite a bit of logic | 13:11 |
| dimitern | rogpeppe: yeah, for sure - learning new things about go every day :) | 13:12 |
| rogpeppe | dimitern: it's worth reading parts of the spec until they make sense | 13:13 |
| TheMue | Aram: How far has your digging into MAAS gone? Already found a suitable OAuth client for a MAAS client? | 13:21 |
| Aram | TheMue: bradfitz wrote an oauth library. | 13:24 |
| TheMue | Aram: OAuth 1 or 2? | 13:24 |
| TheMue | Aram: IMHO MAAS need OAuth 1. | 13:24 |
| dimitern | rogpeppe: but the problem is len([]T{}) == len([]T{nil}) == 0, so I cannot distinguish between those | 13:28 |
| rogpeppe | dimitern: yes, but do you want to? | 13:28 |
| dimitern | rogpeppe: well, originally my point was to create links only explicitly, so I can simplify my tests, otherwise I have to add the Links: []nova.Link{} to EACH server or flavor | 13:29 |
| rogpeppe | dimitern: i'm not sure i understand. why do you have to do that? | 13:29 |
| dimitern | rogpeppe: because if I don't DeepEquals will fail | 13:29 |
| dimitern | rogpeppe: I'll have to have server := nova.ServerDetail{Id: "x", Links: []nova.Link{}} instead of just {Id: "x"} | 13:30 |
| Aram | yes, DeepEquals distinguishes between []T{}, and the nil equivalent, that's a pity. | 13:31 |
| Aram | just wrap DeepEquals. | 13:31 |
| dimitern | rogpeppe: because in the latter case add will create the links, and what I passed and what got stored in the map will be different | 13:31 |
| rogpeppe | dimitern: so you're using the []nova.Link{} to suppress the link creation? | 13:31 |
| dimitern | rogpeppe: exactly | 13:31 |
| dimitern | Aram: how? | 13:32 |
| rogpeppe | dimitern: presumably you could put the expected links in there? | 13:32 |
| Aram | pseudocode: if len(a) == len(b) == 0 { return true} DeepEquals(a, b)... | 13:32 |
| dimitern | Aram: that's seems reasonable | 13:33 |
| rogpeppe | dimitern: but i think the "buildlinks" argument is a perfectly reasonable solution too | 13:33 |
| dimitern | rogpeppe: yes, but that will complicate a lot of cases, where I don't really need links at all | 13:33 |
| rogpeppe | Aram: that won't work in this case | 13:33 |
| Aram | rogpeppe: why not? | 13:33 |
| rogpeppe | Aram: because the slice is embedded in a struct | 13:34 |
| Aram | aah, ok. | 13:34 |
| rogpeppe | dimitern: here's another possibility: | 13:34 |
| Aram | well, you can write an Equals function for that struct. | 13:34 |
| rogpeppe | dimitern: you could have a method on FlavorDetail that builds the links for it | 13:35 |
| rogpeppe | dimitern: that would mean that addFlavor is a little less magic, and the link-building logic gets its own function, independent of addFlavor | 13:36 |
| rogpeppe | dimitern: then you could call flavor.BuildLinks() before addFlavor, and the DeepEquals would work ok | 13:37 |
| dimitern | rogpeppe: I was thinking of that actually, but the FlavorDetail is outside the package (in nova, not novatests) | 13:37 |
| rogpeppe | dimitern: then add a function instead | 13:37 |
| dimitern | rogpeppe: ok, that seems the least amount of overhead, I'll do it | 13:38 |
| rogpeppe | dimitern: perhaps AddLinks(f *FlavorDetail) ? | 13:46 |
| dimitern | rogpeppe: I was thinking of suite.buildLinksFlavor(f FlavorDetail) FlavorDetail and suite.buildLinksServer(s ServerDetail) ServerDetail | 13:47 |
| dimitern | rogpeppe: I need the suite to construct the URLs (the hostname, etc. are in the suite) | 13:48 |
| rogpeppe | dimitern: is it not functionality that you to be usable outside the test suite? | 13:48 |
| dimitern | rogpeppe: hmm, hold on - actually that won't work | 13:50 |
| rogpeppe | s/you to/you want to/ :-) | 13:50 |
| dimitern | rogpeppe: my original idea was to make the direct api complete in a sense that calling addServer is enough to have everything initialized in the state, so the HTTP API will just need to get the server and serialize the ServerDetail to json | 13:51 |
| dimitern | rogpeppe: but I suppose adding it to the suite won't be enough - i have to add it to the service itself and put a comment to call this always before add*() to produce a consistent result for the http api | 13:53 |
| rogpeppe | dimitern: that seems reasonable. | 13:53 |
| dimitern | rogpeppe: ok :) 10x for the walkthrough | 13:54 |
| rogpeppe | dimitern: np | 13:54 |
| dimitern | rogpeppe: ok, so now it should be fine https://codereview.appspot.com/6877054 | 14:06 |
| dimitern | rogpeppe: it you think it looks good I can go ahead and submit it | 14:07 |
| rogpeppe | dimitern: one thought (sorry for not catching it earlier) - you could just mutate the links field *ServerDetail in place, rather than passing it in and out by value. YMMV though. | 14:10 |
| dimitern | rogpeppe: I thought so, but I followed the same pattern append() uses for example | 14:11 |
| rogpeppe | dimitern: append is a bit of a special case | 14:11 |
| rogpeppe | dimitern: because it's designed to be called in many scenarios | 14:11 |
| rogpeppe | dimitern: anyway (and including if you choose not to change it), still LGTM | 14:12 |
| dimitern | rogpeppe: ok, I'll change it, 10x | 14:13 |
| TheMue | fwereade: ping | 14:36 |
| fwereade | TheMue, pong | 14:36 |
| fwereade | TheMue, I'm just looking at your CL right now :) | 14:36 |
| TheMue | fwereade: Aaaah, that's what I wanted to ask you to do. Thx. ;) | 14:36 |
| fwereade | TheMue, quick question: in initMachine, you appear only to start a unit watch when the service is exposed | 14:38 |
| fwereade | TheMue, AFAICT, in the rest of the FW, the unit watch is started regardless and the service exposure is only taken into account when refcounting? | 14:39 |
| TheMue | fwereade: Yes | 14:39 |
| fwereade | TheMue, did I misread somewhere? I'm still not familiar with the whole thing in detail :) | 14:39 |
| TheMue | fwereade: One moment, have to look, because it tries to act like later in "runtime mode". | 14:40 |
| fwereade | TheMue, ok, just to confirm: line 224, `if service.IsExposed() {` | 14:40 |
| fwereade | TheMue, vs line 276, `} else if unit != nil && unit.Life() != state.Dead && fw.machine ds[machineId] != nil {` | 14:41 |
| TheMue | fwereade: Yes, you're right. | 14:41 |
| fwereade | TheMue, so, definitely a bug? | 14:41 |
| TheMue | fwereade: Hmm, have to think what changes if the watch is started also for not yet exposed services. | 14:42 |
| dimitern | rogpeppe: while you're still fresh on the topic, PTAL https://codereview.appspot.com/6910055/ | 14:42 |
| TheMue | fwereade: Funnily all tests are green, even repeated multiple (150x) in a row. | 14:42 |
| dimitern | rogpeppe: this the follow up - removing interface and get prefix from methods | 14:42 |
| rogpeppe | dimitern: looking | 14:43 |
| rogpeppe | dimitern: just wondering: why NovaService rather than Nova? | 14:43 |
| rogpeppe | dimitern: (novaservice.NovaService seems perhaps unnecessarily stuttering) | 14:44 |
| dimitern | rogpeppe: well, as a convention - the other doubles are like that, and the package is novaservice to distinguish it from the nova package | 14:44 |
| fwereade | TheMue, sure, but it's quite possible they just don't have a test case with the right parameters to trigger a bug -- or maybe it comes out not to be a bug after all | 14:44 |
| TheMue | fwereade: Funny, behaves the same way. | 14:44 |
| dimitern | rogpeppe: you think it's too long? | 14:45 |
| fwereade | TheMue, but I am still having a bit of a hard time following all the different ways and times for creating the various datas | 14:45 |
| rogpeppe | dimitern: the package name is fine. if it's a convention i don't mind much, but, yeah i think it could be shorter without loss of clarity | 14:45 |
| fwereade | TheMue, ok, I think I have a fail case | 14:45 |
| fwereade | TheMue, let me check it's not covered by the tests | 14:46 |
| dimitern | rogpeppe: ok, fair enough I like shorter names too :) | 14:46 |
| rogpeppe | dimitern: (given that it's always going to be qualified with the package name) | 14:46 |
| rogpeppe | dimitern: generally the changes look great | 14:46 |
| rogpeppe | dimitern: LGTM with that thought about the name | 14:46 |
| dimitern | rogpeppe: thanks! I'll change the name and submit it then | 14:47 |
| fwereade | TheMue, ISTM that there are no test cases that check the behaviour when a FW is started with an unexposed service -- which is then exposed while the FW is running | 14:49 |
| TheMue | fwereade: OK, good hint, I'll add one for both modes. | 14:50 |
| fwereade | TheMue, ok, but wait a mo | 14:51 |
| fwereade | TheMue, there's something I think could be explored a bit further | 14:51 |
| fwereade | TheMue, if we look more at initMachineData and assume the service is exposed and everything is fine | 14:52 |
| fwereade | TheMue, sorry initMachine | 14:52 |
| fwereade | TheMue, just after the createUnitData call | 14:53 |
| fwereade | TheMue, when you get the OpenedPorts | 14:53 |
| fwereade | TheMue, (sorry, thinking out loud, crrect me when I go off the rails) | 14:54 |
| TheMue | fwereade: Go on. | 14:54 |
| fwereade | TheMue, you set them directly on the unitData and then start the ud's watch | 14:54 |
| TheMue | fwereade: The IsExposed() block has to be from before retrieving the OpenedPorts() and setting them in ud and md. | 15:03 |
| TheMue | fwereade: Additional tests are working fine. | 15:24 |
| fwereade | TheMue, hmm, ok | 15:24 |
| fwereade | TheMue, I think the thing I'm finding hard to follow is the creation chain now | 15:25 |
| fwereade | TheMue, you have several different funcs that create the same kinds of data in different ways | 15:25 |
| TheMue | fwereade: I find it now even more clear than before. | 15:25 |
| fwereade | TheMue, and I'm trying to think of a reason why any of those creation funcs need to be different in any way | 15:26 |
| TheMue | fwereade: Not at all, but maybe I can move it into own methods to avoid code doubling, yes. | 15:26 |
| fwereade | TheMue, ok; and yu now have two different styles of initialization too | 15:27 |
| TheMue | fwereade: No | 15:27 |
| fwereade | TheMue, I think that by using the new one troughout you will see a dramatic simplification | 15:27 |
| TheMue | fwereade: Before we had two, now there is only one left. | 15:27 |
| fwereade | TheMue, sometimes you fill in a machine's units and sometimes you don't, right? | 15:27 |
| TheMue | fwereade: Huh? | 15:27 |
| fwereade | TheMue, dammit, sorry | 15:29 |
| fwereade | TheMue, line 437 | 15:29 |
| TheMue | fwereade: *lol* | 15:29 |
| fwereade | TheMue, at what point does that machine's set of units contain valid data? | 15:29 |
| TheMue | fwereade: After it's unit watcher sent its initial event which is then handled by the firewaller, that sets the data. | 15:32 |
| TheMue | fwereade: This is the logic we had from beginning. | 15:32 |
| fwereade | TheMue, ok, how long does it take for the initial event to be handled? | 15:32 |
| fwereade | TheMue, or rather, at what point o yo know that all the initial events have been handled? | 15:33 |
| fwereade | TheMue, (does this line of questioning sound familiar? ;)) | 15:33 |
| TheMue | fwereade: Stop | 15:33 |
| TheMue | fwereade: This newMachineData() is only used after (!) the initial stuff is handled. | 15:33 |
| TheMue | fwereade: That's what changed. | 15:34 |
| fwereade | TheMue, yes | 15:34 |
| fwereade | TheMue, I know | 15:34 |
| TheMue | fwereade: In init() and in there in initMache(). Where explicitely the initial events of those initial watchers are handled before (!) the firewaller loop starts handling new changes. | 15:34 |
| fwereade | TheMue, initMachine is a step in the direction we need to take | 15:34 |
| fwereade | TheMue, but you do now have two ways of initializing a machine | 15:35 |
| fwereade | TheMue, in one of them, the machine has the correct unit data and can be safely used at any point onwards | 15:36 |
| fwereade | TheMue, in the other one, the machine starts off with empty unit data and has it added gradually after the fact | 15:36 |
| fwereade | TheMue, I think that the second approach *is* in fact adequate to the needs of a running FW | 15:37 |
| fwereade | TheMue, but I'm having difficulty figuring out why we have two separate initialization modes when we could have one that is correct in all circumstances | 15:37 |
| TheMue | fwereade: Yes, and port changing is done when the information has arived. The problem has been when the firewaller starts and there are already open ports that have to be closed or closed ports that have to be open. | 15:38 |
| TheMue | fwereade: But when those are handled all further initial and changing events handle the rest. | 15:38 |
| fwereade | TheMue, are you arguing that it is better to have two separate ways of initing the various *Data structs? | 15:40 |
| fwereade | TheMue, it really does make it hard for me to follow what's going on | 15:40 |
| fwereade | TheMue, it feels like you're drawing information from a whole bunch of different sources and asserting that you have enough information to cover every possible case | 15:41 |
| fwereade | TheMue, but every different data source is a potential race | 15:41 |
| fwereade | TheMue, and I'm not sure I'm equal to the task of analyzing them all | 15:43 |
| TheMue | fwereade: Avoiding code duplication for sure is a good goal. | 15:43 |
| TheMue | fwereade: Step by step the FW is totally rewritte. :/ | 15:43 |
| TheMue | rewritten | 15:44 |
| fwereade | TheMue, well, I was a little surprised by this direction -- I had thought we were clear on the advantages of doing *everything* based on watchers, instead of adding more random desynchronized data sources into the mix | 15:46 |
| TheMue | fwereade: The bad think is that you're right, but on the other hand the change grows and grows. :( | 15:46 |
| fwereade | TheMue, I *think* it's possible to make it shrink | 15:47 |
| fwereade | TheMue, you only need a single creation function for each *Data type | 15:47 |
| TheMue | fwereade: It is everythink done by watchers. But only in init() it's done on the initial event and later lazy. | 15:47 |
| fwereade | TheMue, initMachine uses IsExposed() and OpenedPorts(), completely divorced from any sort of watches on those things | 15:48 |
| fwereade | TheMue, you've moved the desynchronization up a level but not resolved it | 15:48 |
| fwereade | TheMue, so: in each *Data creation function, you must *consume the guaranteed first event* from your own watcher, and fill in your state based on the information received | 15:49 |
| TheMue | fwereade: Sorry, can't follow. | 15:49 |
| fwereade | TheMue, ok, any time you create a machine | 15:50 |
| fwereade | TheMue, all its unit and service data must be created and filled in before the watch lop is started and the creation function returns | 15:50 |
| fwereade | TheMue, that is all you need to do, and all these alternate ways of initialization just fall away | 15:51 |
| fwereade | TheMue, the *only* initialization you need is a `var reconciled bool` at the top of the FW loop | 15:51 |
| fwereade | TheMue, and then `if !reconciled { reconciled := true; err := fw.initWhateverMode() etc }` at the end of the <-machinew.Changes branch | 15:52 |
| TheMue | fwereade: ??? | 15:52 |
| fwereade | TheMue, ok | 15:52 |
| fwereade | TheMue, let's turn this around | 15:52 |
| fwereade | TheMue, why do you have more than one way of creating unit data, or machine data? | 15:52 |
| fwereade | TheMue, why are the serviceDatas for those units created in different ways? | 15:54 |
| TheMue | fwereade: That's already understood, the ??? has been for the reconciled mode. | 15:54 |
| fwereade | TheMue, it is a bool which is used to fire off some logic once and only once when the time is opportune | 15:55 |
| fwereade | TheMue, ie, once you have handled the first machines change | 15:55 |
| fwereade | TheMue, you then have complete state, and can run initGlobalMode() or whatever to reconcile with provider-supplied reality | 15:55 |
| fwereade | TheMue, then set the bool to true so we don;t hit the branch again | 15:56 |
| fwereade | TheMue, and just casually handle all other changes as they come | 15:56 |
| TheMue | fwereade: OK, just trying to bring it together. | 15:57 |
| dimitern | if I do x := y, when y is struct {..}, does this mean it's making a copy and you can change x independently? | 15:59 |
| dimitern | dimitern: answering to myself :) yep, it seems so - i checked | 16:03 |
| TheMue | fwereade: But you still would init the first machines wtacher event before the firewaller loop? | 16:03 |
| TheMue | fwereade: Eh, handle it. | 16:04 |
| fwereade | TheMue, no, why? | 16:04 |
| fwereade | TheMue, nothing is going to start sending on the other channels until the various *Datas have been set up | 16:05 |
| TheMue | fwereade: Ah, no, get it with the reconciled. | 16:05 |
| fwereade | TheMue, if you handle the whole chain in the first event that's all you need | 16:05 |
| fwereade | TheMue, (ok, you might get an env change before the machines change, but that's all to the good) | 16:05 |
| TheMue | fwereade: So the first event I get IS the initial machine watcher event, but in the loop. And there I use that flag to reconcile with the current real world. | 16:06 |
| TheMue | fwereade: That sound reasonable. | 16:07 |
| fwereade | TheMue, yeah, so, the first event is "all known machines" | 16:07 |
| fwereade | TheMue, you do the machineLifeChanged for each machine as before | 16:07 |
| fwereade | TheMue, the only difference is that you handle the initial event from the units watcher *before* starting the loop and returning the created machine | 16:08 |
| fwereade | TheMue, but, ofc, for each unit in the initial event for that machine, you have data to create and fill in before starting the watch and returning the unit data | 16:08 |
| fwereade | TheMue, this may even include starting a service watch, getting the initial event | 16:09 |
| fwereade | TheMue, etc | 16:09 |
| fwereade | TheMue, the idea is just to make sure that there's only one way to create each *Data kind, and that it always completely fills in its own data before it returns from the creation function (and has its watches based on diffs from that known state) | 16:10 |
| fwereade | TheMue, I *think* this hugely simplifies matters | 16:10 |
| fwereade | TheMue, am I making any sense? | 16:11 |
| TheMue | fwereade: Hmm, to make it clean I have to move those new…Data() to methods in firewaller, because the firewaller changes datas but not vice versa. | 16:11 |
| fwereade | TheMue, yeah, my gut was suggesting that would be a good way to go | 16:11 |
| fwereade | TheMue, I didn't want to be excessively prescriptive | 16:11 |
| fwereade | TheMue, what's your feeling about the relative simplicity of the above approach? I think it would be smaller and easier to follow | 16:12 |
| TheMue | fwereade: No, you aren't. But next time you've got such a redesign in mind just write down a little outline with comments. That's simpler than to do it in a dialog. | 16:13 |
| mramm | fwereade: TheMue: I agree that we would all benefit from a bit of design documentation around lifecycle stuff | 16:17 |
| TheMue | mramm: Hiya. | 16:17 |
| mramm | in general I think we often find out that there are disagreements about design later in the process than we should | 16:17 |
| mramm | some of that is inevitable | 16:18 |
| mramm | and I don't want to do Big Design Up Front | 16:18 |
| fwereade | mramm, yeah, I know what you mean | 16:18 |
| mramm | but a few quick notes about what is planed would be good to have more often | 16:18 |
| fwereade | yeah -- I think that perhaps some artifacts were preserved from lisbon, but I couldn't tell you *where* | 16:20 |
| fwereade | mramm, if you think it would still be a win at this stage I would be more than happy to document what I plan to be doing to them over the coming weeks | 16:21 |
| fwereade | mramm, what I plan to write does not differ in any significant way from what we agreed back then -- the fundamental dependencies have not changed | 16:21 |
| mramm | fwereade: I do think it would be a win | 16:21 |
| mramm | fwereade: I know, but we didn't really document what we agreed that well, so I think it would be good to do still | 16:22 |
| TheMue | So, AFK for a few minutes, playing daughters taxi. BRB | 16:24 |
| mramm | rogpeppe: I think it would also be good to document a bit about how we expect the API to work | 16:24 |
| mramm | TheMue: cool, see you soon. | 16:24 |
| rogpeppe | mramm: i agree | 16:24 |
| mramm | rogpeppe: and also a little bit about the plan for building it | 16:25 |
| rogpeppe | mramm: we some ideas but we haven't worked out what the message format will be like | 16:25 |
| mramm | rogpeppe: so that we can see if we are over-promising for 13.04 | 16:25 |
| rogpeppe | mramm: i've already got a preliminary branch submitted with some of the basics done | 16:25 |
| mramm | even writing down what we have now would be valuable I think, but of course the message format/content is the key | 16:26 |
| rogpeppe | mramm: i'm building the framework from the bottom up and leaving decisions about the actual protocol until later | 16:26 |
| mramm | ok, thats fine | 16:26 |
| mramm | let's just define what we know, and write todo items for the decisions that still need to be made | 16:27 |
| rogpeppe | mramm: i also did a preliminary top-down implementation, but it didn't go down that well | 16:27 |
| mramm | always good to give it time | 16:27 |
| TheMue | So, back again | 16:45 |
| rogpeppe | for anyone that would like something to look at, a small CL: https://codereview.appspot.com/6902070/ | 17:05 |
| === TheMue_ is now known as TheMue | ||
| rogpeppe | time for me to go. have a good rest-of-day, peeps | 18:33 |
| === robbiew is now known as robbiew-afk | ||
| === robbiew-afk is now known as robbiew | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!