| hazmat | jimbaker, you can do the encode/decode on the server | 00:57 |
|---|---|---|
| hazmat | jimbaker, the transport is irrelevant to maintaining the format | 00:57 |
| hazmat | ie transfer in yaml, encode/decode cycle in json | 00:58 |
| jimbaker | hazmat, i will see if i can make that work | 03:07 |
| jimbaker | hazmat, in principle this makes sense, it just is the approach i first tried. however, the code is much more refactored, so it's possible this is simpler to do | 03:07 |
| fwereade | morning davecheney | 06:06 |
| davecheney | fwereade: howdy | 06:15 |
| fwereade | davecheney, how's it going? | 06:16 |
| davecheney | fwereade: good; so we're back to lisbon in july i hear | 06:18 |
| fwereade | davecheney, yeah, that's my understanding | 06:18 |
| fwereade | davecheney, is that better or worse for you? | 06:18 |
| davecheney | fwereade: probably better, although I dunno if mark will like the prices, 2700 euro | 06:19 |
| davecheney | well, between 2000 and 2700 | 06:19 |
| fwereade | davecheney, cool -- and, well, the cheapest I saw for me to POA was >2000 euros | 06:19 |
| davecheney | that is what i decoded from the travel agent blather | 06:19 |
| davecheney | fwereade: it was more the 45 hours travel that didn't do it for me | 06:19 |
| * fwereade winces *hard* | 06:20 | |
| davecheney | i think it went SYD -> the frikken moon -> POA | 06:20 |
| fwereade | davecheney, haha | 06:20 |
| davecheney | then a return trip via saturn | 06:20 |
| davecheney | which is insane, because it's shorter to go SYD -> Perth -> POA, than SYD -> LAX | 06:20 |
| davecheney | geographically speaking | 06:20 |
| fwereade | davecheney, the longest I've had to deal with (without incorporating a bed, at least) is probably *just* over 24 hours door to door | 06:21 |
| fwereade | davecheney, and that was bad enough | 06:21 |
| fwereade | davecheney, if you have flights that long though it may well be worth checking with marianna if you can have an overnight break in an airport hotel | 06:21 |
| davecheney | fwereade: yeah, save my back | 06:22 |
| fwereade | davecheney, I wasn't expecting that, but my very first trip had what would otherwise have been 8 hours on the floor of an airport | 06:22 |
| davecheney | 16 hours LAX -> SYD | 06:22 |
| fwereade | davecheney, swapping that for 5 hours in an actualy bed was very much a good move | 06:22 |
| davecheney | was pretty sore by the end of it | 06:22 |
| fwereade | davecheney, I bet | 06:22 |
| davecheney | oh, and a return trip to POA was > 5,000 AUD, which was the 'piss off, we don't want to do it price' | 06:23 |
| fwereade | davecheney, crikey | 06:26 |
| fwereade | heya TheMue | 06:31 |
| TheMue | fwereade: Morning | 06:41 |
| fwereade | rog, TheMue: I would welcome opinions on https://codereview.appspot.com/6317043 (which, in hindsight, reads somewhat passive-aggressively, but I don't really know what to do about that :/) | 07:32 |
| TheMue | fwereade: Passive-aggressively? | 07:32 |
| fwereade | TheMue, http://en.wikipedia.org/wiki/Passive%E2%80%93aggressive_behavior | 07:33 |
| fwereade | TheMue, the problem is perhaps that I didn't express the issues I had with that approach in those terms during the discussion | 07:34 |
| fwereade | TheMue, in my mind, the assumed context was "it's obvious that this is a lot more work", but that may in fact not have been obvious to all participants | 07:35 |
| TheMue | fwereade: Understand | 07:36 |
| === TheRealMue is now known as TheMue | ||
| TheMue | fwereade: From a technological perspective LGTM. | 07:42 |
| TheMue | fwereade: But I need more background to understand the problem you want to solve. | 07:43 |
| TheMue | fwereade: Best in some simple words for a "newbie". ;) | 07:43 |
| rog | fwereade: i see what you mean :-) | 07:44 |
| rog | fwereade, TheMue: mornin' BTW | 07:45 |
| TheMue | rog: Morning | 07:58 |
| rog | hmm, my computer just crashed | 07:58 |
| rog | if anyone said anything after my "mornin' BTW", i missed it | 07:58 |
| TheMue | rog: Only my greeting after you re-entered the channel | 08:01 |
| rog | TheMue: cool | 08:01 |
| fwereade | TheMue, rog: rewritten; is it more helpful now? | 08:05 |
| fwereade | TheMue, btw, wrt what you said earlier, that CL is IMO entirely unnecessary, and that's the reason I was whining so extravagantly in the description | 08:07 |
| fwereade | rog, TheMue: btw, do you have a moment to discuss how I should test charm store access, from outside the charm package, without hitting the real charm store? | 08:08 |
| rog | fwereade: sure | 08:09 |
| fwereade | rog, ok... short of screwing up /etc/hosts, what *can* I do? :) | 08:10 |
| rog | fwereade: is the charm store address hard-coded into the charm package? | 08:11 |
| fwereade | rog, yeah | 08:11 |
| rog | fwereade: well, one possibility would be to make it an exported variable that could be changed by testds | 08:11 |
| fwereade | rog, sure; it just feels last-resorty | 08:12 |
| rog | fwereade: of course then you've got to run some kind of your own server | 08:12 |
| TheMue | fwereade: After reading the changed description it's more clear, thx. | 08:12 |
| fwereade | rog, AIUI niemeyer is rather against exposing that at all | 08:12 |
| fwereade | rog, as it is it's a const (because that was my understanding of niemeyer's perspective on the charm store) | 08:13 |
| TheMue | fwereade: If the address is returned by a function we could introduce a package local variable switch by a function SetTestMode(bool). | 08:14 |
| rog | fwereade: so what functionality are you needing to test exactly? | 08:14 |
| TheMue | fwereade: When setting it to true the function returns localhost. | 08:14 |
| fwereade | rog, that we can deploy a charm from the charm store | 08:14 |
| fwereade | rog, I think there *may* be a case to be made that InferRepository is properly tested, and both local repos and the charm store are properly tested | 08:15 |
| fwereade | rog, and that therefore hitting just one of the InferRepository code paths is actually adequate | 08:16 |
| fwereade | rog, but my go-testing-fu is not adequate to determine whether this is defensible | 08:16 |
| rog | fwereade: that sounds like it may be reasonable. | 08:16 |
| rog | fwereade: there's no point in testing the same functionality in two different places | 08:16 |
| fwereade | rog, yeah, that's what my heart says | 08:17 |
| fwereade | rog, but it was hard work forcing myself to test everything a million different times in python when I joined the project | 08:18 |
| fwereade | rog, and I haven't quite figured out how the prevailing rules have changed | 08:18 |
| fwereade | rog, ok, I'll propose it as-is with an explanation | 08:19 |
| rog | fwereade: seems reasonable. | 08:19 |
| TheMue | rog, fwereade: Different topic, did you already booked you flights? | 08:46 |
| rog | TheMue: yes, i've booked. arriving late sunday evening. | 08:46 |
| fwereade | TheMue, ah yeah, I crash-stopped the booking the other day and haven't redone it yet | 08:47 |
| fwereade | TheMue, ty for reminder | 08:47 |
| TheMue | fwereade: np ;) | 08:47 |
| TheMue | rog: Yes, I'll arrive at 21:45 and depart on Sat at 9:10. | 08:48 |
| * TheMue only hopes he's now right with location and date :D | 08:50 | |
| rog | fwereade: when you say "we discard the actual data and retain only the version." which code are you referring to? | 09:21 |
| rog | fwereade: or is this stuff as yet to be written? | 09:21 |
| fwereade | rog, the python, and the proposed code | 09:21 |
| rog | fwereade: so, in your view, it would be better to have a ConfigNodeWatcher or something like that? | 09:22 |
| rog | fwereade: that passes the settings along as they change | 09:22 |
| fwereade | rog, well, I don't think we need anything other than a map[string]interface{} | 09:22 |
| fwereade | rog, except hmm no | 09:23 |
| rog | fwereade: no? | 09:23 |
| fwereade | rog, blast, I haven't fully thought through the cross-relation settings-settings | 09:23 |
| rog | fwereade: that, i think, might have been part of gustavo's point | 09:24 |
| TheMue | So, added the flight data to the wiki page. | 09:24 |
| rog | fwereade: but explain, please. | 09:24 |
| fwereade | rog, I'm just mainly thinking that there is no reason whatsoever for settings of relations of *other* units to be writable at all | 09:24 |
| fwereade | rog, offering the opportunity to do do so can only lead to bugs and confusion | 09:24 |
| rog | fwereade: why does setting *writing* make any difference to what we're talking about, which is *reading* settings, no? | 09:24 |
| fwereade | rog, maybe it's just a slight discomfort with ConfigNodes used in read-only contexts | 09:25 |
| fwereade | rog, (and a ConfigNode can be refreshed, too, which doesn't fit my idea of what we should be doing) | 09:26 |
| rog | fwereade: you could return a ConfigNode with a nil zk :-) | 09:26 |
| fwereade | rog, heh, that could work :) | 09:26 |
| rog | fwereade: but tbh, why does it matter? we're not going to use it to set values. | 09:27 |
| TheMue | rog: Not nice, better an own type. | 09:27 |
| fwereade | rog, but actually, it's not an issue anyway -- you'll never see changes of your own unit's relation settings | 09:27 |
| rog | fwereade: a config node represents a node with settings. we can choose not to write to it. | 09:27 |
| fwereade | rog, so nothing we are ever watching ever has any reason to be anything other than read | 09:27 |
| rog | fwereade: i'd be happy if it was <-chan map[string]interface{} too. | 09:28 |
| rog | fwereade: is there a problem with that? | 09:29 |
| fwereade | rog, and IMO we shouldn't even offer the opportunity to write it or update it, for similar reasons to those people use to justify public/private distinctions in code | 09:29 |
| fwereade | rog, I'd like that | 09:29 |
| fwereade | rog, ok, people can still write to a map[string]interface{} but at least they have no way of screwing up Zk state by doing so | 09:29 |
| TheMue | fwereade: Otherwise just do a little type struct { data map[string]interface{} } with a Get(string) interface{}. So there's no chance. | 09:30 |
| fwereade | TheMue, indeed, that would work nicely | 09:31 |
| rog | TheMue: nah, let them use the map directly. | 09:31 |
| rog | TheMue: there's no harm in writing to it. | 09:31 |
| rog | TheMue: and then it's easy to range over, etc | 09:31 |
| fwereade | rog, in general my problem is simply with discarding well-sequenced information just so we can approximately reconstruct it later, with additional code to make sure the sequence breaks are less common than they might be | 09:32 |
| rog | i'd say either use a map, or a ConfigNode, but not something in between | 09:32 |
| TheMue | rog: No harm to ZK, yes. OK, and the range may be an argument. | 09:32 |
| rog | TheMue: an argument? | 09:33 |
| fwereade | rog, I'm not even saying this leads to perfect consistency (config-get is still out of sequence), but it's more consistent with less code and I just don't see the downside | 09:33 |
| TheMue | rog: For using the map directly. | 09:33 |
| rog | fwereade: i definitely don't see the point in a version watcher. | 09:33 |
| fwereade | rog, ok, there is a downside: we will store more data locally on the nodes | 09:33 |
| rog | fwereade: we've got to get that data anyway, right? | 09:34 |
| fwereade | rog, that doesn't seem to me to be a strong argument for all the extra complexity :) | 09:34 |
| fwereade | rog, indeed | 09:34 |
| TheMue | rog: The ssh.go in state is by you, isn't it? | 09:35 |
| rog | fwereade: and the version watcher is fetching all the content anyway. | 09:35 |
| fwereade | rog, yeah | 09:35 |
| rog | TheMue: no, that's me | 09:35 |
| fwereade | rog, this is rather my point :) | 09:35 |
| TheMue | rog: You *are* the ssh.go? ;) | 09:36 |
| * rog blushes | 09:36 | |
| fwereade | TheMue, ssh, we don't want reporters involved | 09:36 |
| TheMue | ;) | 09:36 |
| rog | TheMue: i think you meant to say "You are *the* ssh.go?" :-) | 09:37 |
| TheMue | rog: There are several fmt.Printf() in it, are they still needed? | 09:37 |
| rog | TheMue: erp | 09:37 |
| rog | TheMue: i don't see any fmt.Prints | 09:37 |
| rog | TheMue: i see log.Printfs | 09:38 |
| TheMue | rog: Oh, my mistake, just seen log. Sh**! | 09:38 |
| TheMue | rog: So here the prefix "state:" is also still ok. | 09:38 |
| rog | TheMue: they're probably a little excessive tbh, but i find them useful for the time being. perhaps they should be log.Debugf's | 09:38 |
| rog | TheMue: yeah. | 09:39 |
| TheMue | rog: I'm adding the errorContextf() so I found them and thought it would be debugging code. But in the log it makes sense. | 09:40 |
| * rog goes downstairs to get a bowl of cereal | 09:40 | |
| fwereade | rog, I was just wondering some more about MachineUnitsWatcher | 10:02 |
| rog | fwereade: uh huh? | 10:02 |
| fwereade | rog, as you point out, we can't make a *Unit from a deleted unit | 10:02 |
| fwereade | rog, but how are we keeping track of the things on the other end? might unit names be a better thing to pass around than units? | 10:03 |
| rog | fwereade: i suggested that originally, but gustavo didn't like it | 10:03 |
| fwereade | rog, isn't that how the client will be referring to the units in the first place? | 10:03 |
| rog | fwereade: and actually, he's right | 10:03 |
| fwereade | rog, bah, ok | 10:03 |
| fwereade | rog, go on | 10:03 |
| rog | fwereade: because if you look at the container API, it's phrased in terms of *Units | 10:04 |
| rog | fwereade: which seems nice | 10:04 |
| fwereade | rog, ah, ok, that does make sense | 10:04 |
| rog | fwereade: although actually it might only use 'em for their names | 10:04 |
| fwereade | rog, I kinda suspect it would ;p | 10:05 |
| fwereade | rog, I have an analogous situation I would welcome input on | 10:05 |
| fwereade | rog, ServiceRelationsWatcher | 10:05 |
| rog | fwereade: yup. but... it's entirely possible that it might use more info from the unit for deployment in the future. | 10:05 |
| fwereade | rog, yeah, could be | 10:06 |
| rog | fwereade: although then we've got the out-of-date info problem, potentially. | 10:06 |
| rog | fwereade: similar thing with ServiceRelationsWatcher, presumably? | 10:07 |
| fwereade | rog, yeah; specifically I suspect we'll have a map[something]*RelatedUnitsWatcher that I'll want to remove watchers from (and stop them) as the service leaves relations | 10:08 |
| rog | fwereade: so the ServiceRelationsWatcher is maintaining several watchers? | 10:09 |
| fwereade | rog, I don't *think* it makes sense to use actual *Relations as keys (I should probably reread the spec and find out exactly what it takes to be usable as a map key) | 10:09 |
| rog | fwereade: any pointer can be a map key | 10:10 |
| rog | fwereade: (anything that defines equality can be a map key) | 10:10 |
| fwereade | rog, no, the unit agent is maintaining several watchers which it starts/stops in response to the ServiceRelationsWatcher's changes | 10:10 |
| fwereade | rog, ok, so actually I should be doing exactly what you're doing | 10:10 |
| fwereade | rog, and define equality on Relations | 10:11 |
| rog | fwereade: equality is defined on any pointer | 10:11 |
| rog | fwereade: (the pointers are compared) | 10:11 |
| rog | fwereade: you can't "define equality" on something | 10:11 |
| fwereade | rog, ok, then that's surely not what I want | 10:11 |
| fwereade | rog, so I had thought, my misinterpretation of your comments gave me hope | 10:12 |
| rog | fwereade: what defines a relation in this context? | 10:12 |
| fwereade | rog, the endpoints | 10:12 |
| rog | fwereade: i.e. what information would you put in a key? | 10:12 |
| fwereade | rog, hold on a sec | 10:12 |
| fwereade | rog, what do you do in the machine agent when it comes up after an unexpected exit and wants to take down a container for a unit that no longer exists? | 10:13 |
| fwereade | rog, how do you create the *Unit in that case? | 10:13 |
| fwereade | rog, I have to solve the same problem in the unit agent with *Relations, I think | 10:14 |
| rog | fwereade: there's no *Unit in that case | 10:14 |
| fwereade | rog, how do you deal with it then? | 10:14 |
| rog | fwereade: i have to look at running containers | 10:14 |
| rog | fwereade: and see which of them do or don't correspond with current *Units | 10:14 |
| fwereade | rog, and somehow find out what *Units... hmm, ok | 10:14 |
| rog | fwereade: when we come up after an unexpected exit, we will never see units deleted. | 10:15 |
| fwereade | rog, and how do you determine correspondence? by name, I presume? | 10:15 |
| rog | fwereade: yeah, probably. | 10:15 |
| fwereade | rog, yeah, I'm talking about purely at the agent level, when it has to diff last-known state against what the watcher says is current | 10:15 |
| fwereade | rog, hold on | 10:16 |
| rog | fwereade: depends how you store last-known-state | 10:16 |
| fwereade | rog, can you give me an example of when we might want to know anything other than unit name to set up a container? | 10:16 |
| rog | fwereade: perhaps the unit's service might have settings that influence how the LXC container is allocated | 10:17 |
| fwereade | rog, isn't it a matter of setting up a named container, and poking in an upstart script to run `jujud unit foobar/17 ...`? | 10:18 |
| fwereade | rog, the unit agent itself should surely be responsible for everything else | 10:18 |
| rog | fwereade: i think things are a bit more involved when we're starting stuff within LXC containers | 10:18 |
| rog | fwereade: but yeah. | 10:18 |
| fwereade | rog, hmm, I fear I forsee another discussion-heavy afternoon :/ | 10:19 |
| rog | fwereade: for your ServiceRelationsWatcher, what would you use that corresponds to the use of unit names in MachineUnitsWatcher? | 10:20 |
| fwereade | rog, that's what I'm asking | 10:20 |
| rog | fwereade: for the record, i'm off on holiday at the weekend for two weeks. so good to get the discussions in now... | 10:20 |
| fwereade | rog, I'm inclined to tweak describeEndpoints to sort the endpoints before joining them with " " and using that as a key | 10:20 |
| fwereade | rog, very good to, I'm off thu/fri | 10:21 |
| rog | fwereade: ah, so this is last day we're online together before ages away... | 10:21 |
| fwereade | rog, one of those irritating situations where, really, I love to spend time with my family but right now feels rather inconvenient | 10:21 |
| rog | fwereade: yeah | 10:21 |
| fwereade | rog, I suspect I'll be sneaking away to work at night occasionally, but I probably won't coincide with you | 10:22 |
| rog | fwereade: we'll be in the camper van in cornwall. almost entirely offline. | 10:22 |
| fwereade | rog, cool, have fun :) | 10:23 |
| rog | fwereade: you can use a struct as a key BTW | 10:23 |
| rog | fwereade: e.g. map[struct{e1, e2 string}] *Relation | 10:23 |
| rog | fwereade: so no need to join with " " | 10:23 |
| fwereade | rog, number of endpoints is abritrary | 10:24 |
| fwereade | rog, and I'm pretty sure I can't use a slice as a key, right? | 10:24 |
| rog | fwereade: that's right | 10:24 |
| rog | fwereade: number of endpoints is arbitrary pending more relation kinds than provides/requires/peer | 10:25 |
| rog | ? | 10:25 |
| rog | fwereade: or is it not to do with that? | 10:25 |
| fwereade | rog, ATM only possibilities are 1 and 2 | 10:25 |
| fwereade | rog, that's exactly what it is | 10:25 |
| rog | fwereade: why can't you use the relation key as a key? | 10:26 |
| fwereade | rog, because I'm pretty certain maintaining the watchers is the unit agent's responsibility, and it doesn't feel like state is the right package | 10:27 |
| fwereade | rog, ahhhh hmmm maybe I could use the relation ident | 10:28 |
| rog | fwereade: we could add a Key method to Relation, i suppose. | 10:28 |
| fwereade | rog, we expose that to users, even though it's just a thinly-disguised key | 10:28 |
| rog | fwereade: relation ident? | 10:28 |
| fwereade | rog, I'd prefer not to go down that path unless we have to | 10:28 |
| fwereade | rog, for relation-0000000001, it's "relation-1" | 10:29 |
| rog | fwereade: that seems like a fine thing to use for a key | 10:29 |
| rog | fwereade: especially if it's exposed to users | 10:29 |
| fwereade | rog, we export JUJU_RELATION_IDENT in foo-relation-* hooks so the charm can distinguish between the different runtime relations using the foo charm relation | 10:30 |
| rog | fwereade: so maintaining a map of that to *Relation seems ideal to me | 10:30 |
| fwereade | rog, I was thinking more a map of that to RelatedUnitsWatcher | 10:31 |
| rog | fwereade: ok, sure. | 10:31 |
| fwereade | rog, which might hold a *Relation, but I'm not even sure that's needed past construction time | 10:31 |
| * fwereade slopes off for cig/think time | 10:32 | |
| Aram | hello everyone | 10:56 |
| fwereade | Aram, heyhey | 11:02 |
| rog | Aram: hiya | 11:10 |
| fwereade | rog, fwiw, I was wrong: it's JUJU_RELATION_ID, not IDENT, and it's not relation-1: it's relation-name-according-to-service-1 | 11:40 |
| rog | fwereade: well, maybe that's a reasonable form for the unit agent to key it on too. because it's not going to hold any relations for any other services, right? | 11:41 |
| fwereade | rog, yeah, agreed, I just wanted to corrcet the misconceptions that might right now be burrowing through your brain :) | 11:42 |
| rog | fwereade: rooted out! | 11:42 |
| fwereade | rog, btw, should I do a quick CL for the MustErr watcher stuff we discussed yesterday? | 12:08 |
| rog | fwereade: sounds good | 12:08 |
| fwereade | rog, or are you on that? | 12:08 |
| fwereade | rog, cool | 12:08 |
| rog | fwereade: nope, i'm on unit agent tests currently | 12:08 |
| fwereade | rog, ah cool, what sort of thing? | 12:09 |
| rog | fwereade: oops, i meant machine agent | 12:09 |
| rog | doh | 12:09 |
| fwereade | rog, ah rigt :) | 12:09 |
| rog | boring :-) | 12:09 |
| rog | fwereade: i'm just looking at cmd/juju/deploy.go BTW | 12:13 |
| rog | fwereade: i wondered if unpacking the charm in memory is a good idea | 12:13 |
| rog | fwereade: perhaps it would be better to store in a temp file instead | 12:13 |
| fwereade | rog, hmm, maybe so | 12:13 |
| rog | fwereade: and then for *charm.Bundle you could use the raw file as is | 12:14 |
| rog | fwereade: (no need to read twice, even) | 12:14 |
| rog | fwereade: 'cos charms could potentially be arbitrarily big | 12:15 |
| fwereade | rog, indeed so | 12:15 |
| rog | fwereade: i wonder if the charm.Charm interface should support BundleTo. | 12:18 |
| fwereade | rog, possibly, I'd have to think about it a bit :) | 12:19 |
| rog | fwereade: yeah, there may well be a good reason it's not there | 12:20 |
| rog | fwereade: at the very least, charm.Bundle could implement BundleTo :-) | 12:21 |
| rog | fwereade: although that kinda loses the point | 12:22 |
| Aram | rog: a new inferno fan: http://debu.gs/entries/inferno-part-2-let-s-make-a-cluster | 12:29 |
| rog | Aram: not new - this is the guy that did the post about my shell. | 12:30 |
| Aram | ah, yes. | 12:30 |
| rog | Aram: but it's really nice to see him get excited about it all | 12:30 |
| rog | Aram: it still excites me actually, but i've kinda left it behind | 12:30 |
| rog | Aram: it's always great to see people play with toys you've made yourself. | 12:32 |
| Aram | yes, indeed. | 12:32 |
| Aram | hi niemeyer. | 12:32 |
| niemeyer | Hello! | 12:33 |
| rog | niemeyer: yo! | 12:33 |
| fwereade | niemeyer, heyhey | 12:36 |
| fwereade | niemeyer, quick check: rick spencer is the head of my business unit, right? | 12:41 |
| niemeyer | fwereade: yep | 12:41 |
| fwereade | niemeyer, cool, tyvm | 12:42 |
| niemeyer | fwereade: That reminds me of a guy back at Conectiva | 12:45 |
| fwereade | niemeyer, oh yes? | 12:45 |
| niemeyer | fwereade: He ranted with someone online on an internal mailing list | 12:45 |
| niemeyer | fwereade: A couple of months after joining the company | 12:46 |
| niemeyer | fwereade: Then, right after sending his last email to the rant, he asks "Who's that silly guy!?" | 12:46 |
| niemeyer | fwereade: "It's your direct manager, dude.." | 12:46 |
| fwereade | niemeyer, haha | 12:46 |
| fwereade | niemeyer, in this case it's just because the travel agent wants to know | 12:47 |
| niemeyer | fwereade: Good thing you're not fighting with him yet! ;-D | 12:47 |
| fwereade | niemeyer, but I hope this guy called "sabdfl" I've been insulting isn't anyone in particular ;p | 12:47 |
| niemeyer | LOL | 12:47 |
| fwereade | niemeyer, btw, https://codereview.appspot.com/6298097 should be very quite, it's the mustErr stuff we discussed yesterday | 12:49 |
| fwereade | s/quite/quick/ | 12:49 |
| niemeyer | fwereade: Looking | 12:49 |
| niemeyer | I intend to to a good pass on everything now, btw | 12:49 |
| niemeyer | Aram: Specially on yours long standing | 12:50 |
| fwereade | niemeyer, cool, thanks :) | 12:50 |
| Aram | niemeyer: great, I have charms and units in the queue. | 12:51 |
| === TheRealMue is now known as TheMue | ||
| niemeyer | fwereade: Done | 12:59 |
| TheMue | Grrr, router had a problem. | 13:00 |
| fwereade | niemeyer, cool, thanks | 13:00 |
| rog | fwereade: did you run all tests before submitting https://codereview.appspot.com/6298097/ ? | 13:11 |
| rog | fwereade: i suspect it may have broken other packages | 13:12 |
| fwereade | rog, oh, crap, I may have just done state/... | 13:12 |
| rog | fwereade: yeah, i think it breaks cmd/jujud | 13:12 |
| rog | fwereade: though i haven't checked | 13:12 |
| fwereade | rog, I saw a ... in history, and... | 13:12 |
| * fwereade hangs head in shame | 13:12 | |
| fwereade | rog, you're absolutely right :( | 13:13 |
| * rog goes for lunch in the blessed sunshine | 13:15 | |
| fwereade | niemeyer, observe screwup above; now I've fixed that and really truly properly verified it's unbroken, what would happen if I were to just lbox propose/submit again in the same branch? | 13:17 |
| Aram | meh, how can flight search technology be so bad. | 13:18 |
| Aram | I mean, it's a trivial problem to solve. | 13:18 |
| niemeyer | fwereade: Won't work | 13:19 |
| niemeyer | fwereade: Just branch it over with "bzr checkout -b <original name>-fixup" | 13:19 |
| niemeyer | fwereade: and then "lbox propose; lbox submit" | 13:19 |
| fwereade | niemeyer, cheers, will do | 13:19 |
| Aram | hmm, lbox propose (without -cr) uses the previously created cr on rietveld if it exists? | 13:20 |
| fwereade | rog, it appears to me that there's something up with the container tests as well... | 13:31 |
| niemeyer | Aram: Yep | 13:31 |
| Aram | greta | 13:31 |
| Aram | great | 13:31 |
| niemeyer | Aram: The previous CL/merge proposal/whatever.. it updates whatever was done | 13:32 |
| fwereade | rog, http://paste.ubuntu.com/1050880/ | 13:32 |
| niemeyer | Aram: Well, almost.. it won't touch existing bugs | 13:32 |
| TheMue | fwereade: When I now start with ServiceRelation.AddUnit(), would I conflict with you? | 13:33 |
| fwereade | TheMue, yes, please don't touch ServiceRelation, it's gone | 13:33 |
| fwereade | TheMue, what are you planning? | 13:33 |
| TheMue | fwereade: I'm thinking of continuing with the relation methods, but I know you're doing some changes there. | 13:34 |
| TheMue | fwereade: Relation is gone, but ServiceRelation not yet. | 13:34 |
| fwereade | TheMue, I would very much prefer if it we didn't step on each other relation-wise | 13:35 |
| TheMue | fwereade: That's why I asked before. ;) | 13:35 |
| fwereade | TheMue, I have one CL out and a whole *bunch* of stuff waiting in the wings that wants only for agreement and go-ahead on the current bits | 13:35 |
| fwereade | TheMue, there's a missing thing for state.Service though... we don't have any way to set service config | 13:36 |
| fwereade | TheMue, and I think that then implies the whole format 2 business | 13:36 |
| TheMue | fwereade: OK, so I'll concentrate on other parts. | 13:36 |
| niemeyer | Aram: Review sent | 13:36 |
| Aram | thanks | 13:36 |
| niemeyer | Aram: Please let me know if you'd like to talk | 13:36 |
| TheMue | fwereade: OK, I'll take a look. | 13:36 |
| niemeyer | Aram: Most points should be straightforward, though | 13:37 |
| niemeyer | Aram: Glad to see this moving! | 13:37 |
| Aram | yes! | 13:37 |
| TheMue | fwereade: Todays code only has a get_config. What would you exactly expect from a SetConfig()? Isn't it normally done by retrieving the config, even if it's empty and then do changes and write it again? | 13:41 |
| fwereade | TheMue, huh, you're perfectly right | 13:42 |
| fwereade | TheMue, maybe a look at the format 2 stuff then if you're not too much averse to it? | 13:43 |
| fwereade | TheMue, I had it planned but I seem to have got rather caught up in these bits, and I think it'll take me a few days to burn it down again | 13:43 |
| TheMue | fwereade: No problem, but where do I find infos about the format 2 stuff? | 13:45 |
| fwereade | TheMue, just a mo; I'll collect what I can find for you | 13:46 |
| TheMue | fwereade: Thx. | 13:47 |
| Aram | niemeyer: right now in ZK we use incrementing integer id's for some thing, this has the property of making nice names (/machine-0, /unit-1-0, etc), but in MongoDB incrementing integers are nasty: http://www.mongodb.org/display/DOCS/How+to+Make+an+Auto+Incrementing+Field | 13:55 |
| Aram | so so we still want to go with incrementing integers in mongodb, or are random numbers fine? | 13:55 |
| rog | Aram: flight search is actually turing complete: http://research.swtch.com/ita | 14:00 |
| niemeyer | Aram: A machine id is an integer.. this isn't about MongoDB or ZooKeeper this is the API itself | 14:00 |
| Aram | niemeyer: I'm not debating whether it should be an integer or not, but whether should be an incrementing integer or a random one :)/ | 14:01 |
| niemeyer | Aram: It should remain an incrementing integer | 14:01 |
| niemeyer | Aram: 9879872198729831 won't be great :) | 14:01 |
| rog | fwereade: ah! i see the problem. i've got jujud in my path, but you haven't. | 14:03 |
| rog | fwereade: i think the test should probably create a fake jujud | 14:03 |
| rog | fwereade: and set the path to point to it | 14:03 |
| fwereade | rog, sounds sensible | 14:03 |
| Aram | rog: I know the problem is computationally hard, my issue is that nobody publicly exports the flight data so people could build better 3rd party software than the crap software flight agencies provide. | 14:04 |
| rog | Aram: true too | 14:04 |
| niemeyer | Aram: Btw, how is it nasty to increment an integer in MongoDB? | 14:04 |
| niemeyer | Aram: It's a trivial inc operation | 14:05 |
| niemeyer | Aram: I suggest adding a "sequence" collection, where the _id is the collection name | 14:06 |
| niemeyer | Aram: and having a sequence(name string) (int, error) helper in State | 14:06 |
| Aram | niemeyer: so in that link I sent you, you prefer option #1 to option #2? | 14:06 |
| Aram | that's perfectly fine by me. | 14:07 |
| niemeyer | Aram: I prefer to have a scheme as suggested above | 14:08 |
| niemeyer | Aram: So we can reuse this next time we need a sequence number | 14:08 |
| Aram | yes, it's great. | 14:09 |
| Aram | ok. | 14:09 |
| Aram | niemeyer: is it possible to create a new bzr branch, but use the same CR as an existing branch? | 14:18 |
| niemeyer | Aram: nope | 14:18 |
| Aram | bummer | 14:18 |
| niemeyer | Aram: Well, maybe yes | 14:18 |
| niemeyer | Aram: But why? | 14:18 |
| Aram | niemeyer: I did the mistake of using my trunk for the changes you reviewed, I ported them to a different branch, but that also created a new CR... | 14:19 |
| niemeyer | Aram: I think it can be made to work: | 14:20 |
| niemeyer | Well, you already created a new CL, right? | 14:20 |
| Aram | yes | 14:20 |
| niemeyer | Aram: Edit the description in the *new* merge proposal, and change the very last link of codereview to point to the proper place | 14:20 |
| Aram | interesting | 14:21 |
| niemeyer | Aram: Then, mark the last branch as Rejected | 14:21 |
| niemeyer | Aram: and run "lbox propose" again | 14:21 |
| niemeyer | Aram: It's not great to be doing this because the merge proposal will lose part of the history | 14:21 |
| Aram | ok. | 14:22 |
| niemeyer | TheMue: Reviewed https://codereview.appspot.com/6300098/ again | 14:30 |
| TheMue | niemeyer: Just seen, thx | 14:30 |
| niemeyer | TheMue: please ping me if you repush soon and I'll have an immediate look so we can integrate | 14:30 |
| niemeyer | Aram: Yeah, so we need to do something with this, since it's a copy of the last branch: https://codereview.appspot.com/6295103/ | 14:31 |
| TheMue | niemeyer: Should be only a few moments. | 14:31 |
| niemeyer | Aram: Are you doing the trick mentioned? | 14:31 |
| Aram | niemeyer: yes | 14:31 |
| niemeyer | Aram: Super | 14:31 |
| Aram | niemeyer: how do I mark a branch as rejected? | 14:33 |
| niemeyer | Aram: Sorry, not the branch, the merge proposal | 14:33 |
| Aram | same question, where do I do that? :) | 14:33 |
| niemeyer | Aram: I've marked it as Work In Progress | 14:34 |
| niemeyer | Aram: In the merge proposal page itself | 14:34 |
| * niemeyer finds | 14:34 | |
| niemeyer | "Sorry, there was a problem connecting to the Launchpad server." | 14:35 |
| Aram | heh | 14:35 |
| Aram | "Delete proposal to merge"? | 14:35 |
| Aram | this onw | 14:35 |
| Aram | one | 14:35 |
| Aram | ? | 14:35 |
| niemeyer | Aram: Done | 14:36 |
| niemeyer | Aram: No need to delete | 14:36 |
| niemeyer | Aram: It has some history there | 14:36 |
| niemeyer | Aram: I've marked it as Rejected | 14:36 |
| niemeyer | Aram: and added a note pointing out to the new MP | 14:36 |
| Aram | niemeyer: I think it worked, https://codereview.appspot.com/6304099 shows the correct branch. | 14:40 |
| Aram | niemeyer: but the issue is closed, is that expected? | 14:40 |
| niemeyer | Aram: No, this doesn't happen automatically.. have you closed it by hand? | 14:46 |
| niemeyer | Aram: lbox will only close on submit, which hasn't happened | 14:46 |
| Aram | niemeyer: ah, yes, sorry, my fault, forgot I closed it yesterday. | 14:46 |
| Aram | so I can open it again | 14:46 |
| Aram | done | 14:46 |
| Aram | great | 14:46 |
| niemeyer | Super | 14:46 |
| Aram | thanks again niemeyer | 14:47 |
| TheMue | niemeyer: So, had a text conflict to resolve first, but now it's in again. | 14:50 |
| hazmat | bcsaller, ping | 14:58 |
| niemeyer | Aram: np! | 15:15 |
| niemeyer | fwereade: you've got a review on https://codereview.appspot.com/6312048/ | 15:15 |
| niemeyer | fwereade: Let me know if you'd like to talk about it so we can get this in soonish | 15:15 |
| niemeyer | fwereade: I'll be stepping out for lunch in a moment | 15:15 |
| niemeyer | TheMue: Super, will look | 15:16 |
| fwereade | niemeyer, I'd be happy to, reading now | 15:17 |
| niemeyer | TheMue: LGTM | 15:18 |
| TheMue | niemeyer: Thx, will submit it now. | 15:18 |
| fwereade | niemeyer, ok, I think I agree with what you have there | 15:19 |
| fwereade | niemeyer, sorry about the Id, it seemed like a good idea at the time :/ | 15:19 |
| niemeyer | fwereade: Actually, LGTM with that in.. doesn't seem to be controversial in any way. | 15:19 |
| niemeyer | fwereade: No worries, I actually find the value of JUJU_RELATION_ID a good user-facing feature | 15:20 |
| niemeyer | fwereade: When sitting within the unit, that relation id makes perfect sense | 15:20 |
| niemeyer | fwereade: In state not so much | 15:20 |
| fwereade | niemeyer, well, I'd love to get this in... but I guess I'm not sure what in particular differentiates the review from a LGTM-with-these-fixes one | 15:22 |
| fwereade | niemeyer, most of your suggested changes seem sane and simple | 15:23 |
| fwereade | niemeyer, which ones are subtler than I think at first glance? | 15:23 |
| niemeyer | <niemeyer> fwereade: Actually, LGTM with that in.. doesn't seem to be controversial in any way. | 15:23 |
| niemeyer | fwereade: ? | 15:23 |
| fwereade | niemeyer, ah, ok, I see | 15:23 |
| fwereade | niemeyer, so, you're OK with the implementation of Id as it is? | 15:24 |
| niemeyer | fwereade: Nope, with the suggested change | 15:24 |
| fwereade | niemeyer, just the int then, cool | 15:24 |
| niemeyer | fwereade: func (r *Relation) Id() int | 15:24 |
| fwereade | niemeyer, great | 15:24 |
| niemeyer | fwereade: (no error even.. we have the key) | 15:24 |
| fwereade | niemeyer, indeed | 15:24 |
| fwereade | niemeyer, ok, I'll fix those and submit | 15:25 |
| fwereade | niemeyer, tyvm | 15:25 |
| niemeyer | fwereade: Thanks a lot | 15:25 |
| fwereade | niemeyer, a pleasure :) | 15:25 |
| niemeyer | fwereade: I'm afraid I don't have enough time to cover it before lunch, but maybe we should talk about the VersionWatcher stuff afterwards | 15:38 |
| niemeyer | fwereade: I'm missing the context | 15:39 |
| fwereade | niemeyer, I would be very keen to | 15:39 |
| niemeyer | fwereade: You seem unhappy with it, and I don't even know why we need it, which means we should talk :) | 15:39 |
| fwereade | niemeyer, definitely -- I was trying to talk about it yesterday, but in hindsight I was framing the discussion all wrong | 15:39 |
| fwereade | niemeyer, I'm not sure precisely when I'll be around this evening | 15:40 |
| niemeyer | fwereade: No worries | 15:40 |
| niemeyer | fwereade: We can talk whenever | 15:40 |
| niemeyer | fwereade: We have enough overlap for this to not be an issue | 15:41 |
| fwereade | niemeyer, and I'm off after that until the end of the week -- *but* this is a subject dear to my heart atm so I will definitely track you down at some point before the weekend | 15:41 |
| niemeyer | fwereade: Oh, I didn't know that, ok | 15:41 |
| fwereade | niemeyer, (it is a pleasure to see my family... but curse the timing) | 15:41 |
| niemeyer | fwereade: Take a good rest and enjoy their presence | 15:42 |
| fwereade | niemeyer, I *think* I mentioned it a while ago but I forgot to actually book it... | 15:42 |
| fwereade | niemeyer, cheers :) | 15:42 |
| fwereade | niemeyer, after that I think I'm saving my holiday for post 12.10 :) | 15:43 |
| niemeyer | fwereade: It will be quite a walk towards it :) | 15:43 |
| fwereade | niemeyer, it feels like we're doing ok so far :) | 15:43 |
| niemeyer | fwereade: Indeed, very happy with how we've been pushing things forward | 15:44 |
| niemeyer | fwereade: Real momentum | 15:44 |
| fwereade | niemeyer, absolutely, it makes even a short holiday feel quite the unwelcome intrusion ;) | 15:46 |
| niemeyer | fwereade: Hehe :) | 15:49 |
| niemeyer | Alright! | 15:49 |
| niemeyer | Time to find some nice lunch | 15:50 |
| rog | fwereade: is it possible for a service to be subordinate to more than one other service? | 16:07 |
| fwereade | rog, hmmm | 16:10 |
| fwereade | rog, I wouldn't swar you couldn't write a charm that implied such a relationship :/ | 16:10 |
| rog | fwereade: it certainly seems possible in the state API. | 16:10 |
| fwereade | rog, but I don't think such an arrangement makes any sense | 16:10 |
| fwereade | rog, how so? | 16:11 |
| fwereade | rog, ah, wait, reparsed: I think it makes perfect sense | 16:12 |
| fwereade | rog, deploy wordpress; deploy mysql; depoy logging; add-relation logging mysql; add-relation loggin wordpress | 16:13 |
| rog | fwereade: you can d1 = AddService("wordpress"); d2 = AddService("wordpress"); sub1 = AddService(subord); d1.AddUnit(); sub1.AddUnitSubo .... you got it | 16:13 |
| fwereade | rog, sorry, I have units on the brain at the moment, ahd totally wrong context | 16:13 |
| rog | fwereade: ok, so that's fine. good | 16:13 |
| rog | fwereade: thanks | 16:16 |
| Aram | niemeyer: did all the requested changes including monotonically increasing integer ids and state compatible errors. | 16:52 |
| rog | niemeyer, fwereade: machine agent implementation: https://codereview.appspot.com/6294096/ | 16:55 |
| fwereade | rog, cool | 17:00 |
| rog | fwereade: if you manage to give some feedback today, that would be marvellous, as it would be nice to polish it tomorrow & friday. | 17:01 |
| fwereade | rog, I'll do what I can :) | 17:01 |
| rog | fwereade: ta! :-) | 17:01 |
| imbrandon | where should this be filed http://15.185.100.228/out.charmload | 17:02 |
| rog | imbrandon: juju-core, i think | 17:03 |
| imbrandon | k | 17:03 |
| rog | niemeyer: looks like your schema checking isn't quite sufficient... | 17:05 |
| imbrandon | https://bugs.launchpad.net/juju-core/+bug/1015700 :) | 17:09 |
| fwereade | rog, initial thoughts posted | 17:11 |
| rog | fwereade: thanks. BTW the fact that the watcher returns subord units too was decided after fairly extensive debate with niemeyer. | 17:12 |
| rog | fwereade: that change was why i ended up adding the isPrincipal field to state.Unit | 17:12 |
| rog | fwereade: which makes the current behaviour more or less cost-free | 17:12 |
| fwereade | rog, cool, I'm always open to counterarguments/being overruled :) | 17:12 |
| fwereade | rog, in that narrow context it seems like a slightly odd decision, but I'm sure there's context I'm not seeing | 17:13 |
| rog | fwereade: and it's nice if status commands could potentially use Machine.Units to pull out stuff for a status, for example. | 17:13 |
| fwereade | rog, I'm ok with that, but less sure that Units and WatchPrincipalUnits aren't both sensible methods for their particulat uses | 17:14 |
| rog | fwereade: i don't see the extra four lines of code as particularly onerous | 17:14 |
| fwereade | rog, it's the *indentation*! the horror, the horror! ;p | 17:15 |
| rog | fwereade: oh well, if *that's* what you don't like, i'll do: if !u.IsPrincipal() {continue} :-) | 17:15 |
| fwereade | rog, haha :) | 17:15 |
| Aram | what the hell does bzr want from me this time: http://paste.ubuntu.com/1051236/ | 17:16 |
| Aram | I only did a bzr pull | 17:16 |
| Aram | bzr revert fixed it | 17:17 |
| Aram | but... what's the procedure with these things? | 17:18 |
| fwereade | Aram, was that a pull from a branch which included mate into a branch that had mstate but hadn't yet added it? | 17:18 |
| fwereade | Aram, but I'm not sure it's any nicer when it has been added | 17:18 |
| Aram | fwereade: I pulled from lp:juju-core into my mstate branch. | 17:19 |
| fwereade | Aram, why merge rather than pull? | 17:19 |
| fwereade | Aram, er vice versa | 17:19 |
| Aram | hm? I should have done bzr merge instead of bzr pull? | 17:19 |
| fwereade | Aram, I basically never pull, but I can't claim to have anything more than a vague intuitive understanding of bzr in the first place | 17:20 |
| Aram | hmm, this is strange. in hg/git you always pull, there's no other way. | 17:21 |
| Aram | so here pull does soemthing else... | 17:21 |
| fwereade | Aram, my workflow is almost always `bzr branch <prereq-or-trunk>`, followed by regular `bzr merge`s to keep the local branch up to date | 17:21 |
| Aram | ok, I'll try that. | 17:21 |
| Aram | fwereade: thanks, it worked. | 17:23 |
| fwereade | Aram, excellent :) | 17:23 |
| Aram | basically I never have to use pull. | 17:23 |
| Aram | strange. | 17:23 |
| fwereade | Aram, IME messing around with checkouts and pulls inevitably causes me pain | 17:23 |
| fwereade | Aram, branch, merge, and push do basically everything I need | 17:24 |
| fwereade | Aram, with ocasional judicious use of --remember on merge or push | 17:24 |
| rog | fwereade: do you think all watchers should implement Err()? it seems to me like calling Stop is enough. | 17:34 |
| Aram | "TOO LONG (>50)" change summaries are limited to 50 chars? | 17:41 |
| rog | fwereade: no more substantial comments? | 17:44 |
| rog | fwereade: actually, publish them on the CL if you have some. i've gotta go! | 17:44 |
| rog | fwereade: thanks a lot for the review BTW | 17:44 |
| rog | fwereade: have a great long weekend, and i'll see you in a little over 2 weeks' time | 17:45 |
| * rog is off. see y'all (william excepted) tomorrow! | 17:45 | |
| rog | fwereade: i think i fixed that container test bug BTW. it'd be good if you could test it on your machine. | 17:46 |
| rog | fwereade: https://codereview.appspot.com/6302103/ | 17:46 |
| Aram | rog: fwereade: TheMue: trivial one liner: https://codereview.appspot.com/6305121 | 17:49 |
| * niemeyer waves | 17:51 | |
| niemeyer | Aram: LGTM | 17:51 |
| niemeyer | Aram: Just in time ;) | 17:52 |
| niemeyer | rog: LGTM too | 17:53 |
| Aram | niemeyer: what do I do now, lbox submit? why does it want to update the description again? | 17:53 |
| TheMue | Aram: LGTM, I prefer this style of durations. | 17:54 |
| niemeyer | Aram: It's a chance for you to adjust it before committing, if necessary | 17:54 |
| Aram | niemeyer: ah, ok | 17:54 |
| Aram | niemeyer: I updated the mstate CR with everything you said | 17:55 |
| Aram | TheMue: yes, indeed. | 17:55 |
| niemeyer | Aram: Cool, will have a look now | 17:55 |
| niemeyer | Aram: Hmm.. your branch is not in the queue | 18:02 |
| niemeyer | Aram: Did you run lbox propose again? | 18:03 |
| Aram | yes | 18:03 |
| fwereade | niemeyer, I'm around for a bit; are you free for a quick talk about VersionWatcher and related affairs? say in 10 mins perhaps, if you're in midflow with Aram? | 18:04 |
| niemeyer | Aram: So something isn't right | 18:04 |
| Aram | niemeyer: maybe I should click on "resubmit proposal" on the lpad website? | 18:04 |
| niemeyer | Aram: Nope.. just "lbox propose" on the right branch should do it | 18:04 |
| niemeyer | fwereade: Yeah, sounds good | 18:04 |
| fwereade | Aram, the fantastic thing about lbox is you basically never need to visit launchpad ;) | 18:04 |
| fwereade | niemeyer, cool, off for a ciggie then, bbs, ping me when you're free | 18:05 |
| niemeyer | Aram: Change *summaries* are limited to 50 chars.. change descriptions are not | 18:05 |
| Aram | niemeyer: yes, I figured. | 18:05 |
| niemeyer | Aram: These summaries end up in emails etc | 18:05 |
| niemeyer | As in, subjects | 18:05 |
| Aram | niemeyer: what about now, is it in th queue? | 18:05 |
| Aram | I did lbox propose again | 18:06 |
| niemeyer | Aram: It is | 18:06 |
| niemeyer | Aram: Thanks | 18:06 |
| Aram | niemeyer: ah, I did -wip before. | 18:06 |
| niemeyer | Aram: Ah, that'd explain it | 18:06 |
| Aram | I didn't want to spam you all with emails while I was doing experiments. | 18:07 |
| Aram | personally I get enough email already. | 18:07 |
| niemeyer | Aram: Did you change the branch name a third time? | 18:07 |
| Aram | niemeyer: I did change it three times, yes, but all the hacketry we did today was with name it has now... | 18:08 |
| niemeyer | Aram: Okay, please don't keep changing branch names as it spreads references around | 18:08 |
| niemeyer | Aram: The note I added to the previous MP is broken, for example | 18:09 |
| Aram | I'll try to. | 18:09 |
| niemeyer | Aram: There's something wrong in your setup still | 18:10 |
| niemeyer | Aram: "Aram Hăvărneanu (aramh) wrote 4 minutes ago:" | 18:10 |
| niemeyer | https://code.launchpad.net/~aramh/juju-core/juju/+merge/110920 | 18:10 |
| niemeyer | Aram: This means a local branch is being pushed onto the old location still | 18:11 |
| Aram | hmm? how can I check/debug? | 18:11 |
| niemeyer | Aram: try "bzr info" on the branch itself | 18:13 |
| niemeyer | Aram: Well, maybe lbox is actually picking the wrong one | 18:13 |
| niemeyer | Aram: Let's just delete the old MP | 18:14 |
| niemeyer | Aram: Done.. please delete this branch too: https://code.launchpad.net/~aramh/juju-core/juju | 18:14 |
| Aram | done | 18:14 |
| Aram | niemeyer: now what? | 18:14 |
| niemeyer | Aram: Okay, now run "bzr info" on bzr.. let's make sure it's not referring to the old name | 18:14 |
| niemeyer | on the branch, I mean | 18:15 |
| Aram | niemeyer: http://paste.ubuntu.com/1051331/ | 18:15 |
| niemeyer | Aram: Okay, I suppose you're not using cobzr | 18:15 |
| Aram | I am | 18:15 |
| Aram | white:mstate$ cobzr branch | 18:16 |
| Aram | master | 18:16 |
| Aram | * mstate-machine-basic | 18:16 |
| Aram | store-mgo-timeconsts | 18:16 |
| niemeyer | Aram: Oh, sorry, I misread | 18:16 |
| niemeyer | Aram: What "bzr nick" tells you? | 18:16 |
| Aram | white:mstate$ bzr nick | 18:16 |
| Aram | mstate-machine-basic | 18:16 |
| niemeyer | Aram: Looks good | 18:17 |
| niemeyer | Aram: Try lbox propose again then, with -v please | 18:17 |
| Aram | ok | 18:17 |
| Aram | niemeyer: http://paste.ubuntu.com/1051334/ | 18:19 |
| niemeyer | Aram: Yay, looks good | 18:19 |
| niemeyer | Aram: Aha, I know what's wrong | 18:20 |
| Aram | niemeyer: what is? is it on my side? | 18:20 |
| niemeyer | Aram: The email of the Launchpad MP on the CL is outdated | 18:20 |
| niemeyer | Aram: Can you please edit it as you're the only one with permission? | 18:20 |
| niemeyer | Aram: It should say 111235 rather than 110920 | 18:21 |
| Aram | ok, let me try it | 18:21 |
| Aram | done | 18:22 |
| niemeyer | Aram: Outstanding.. should all be in sync now | 18:22 |
| niemeyer | fwereade: Do you wanna talk before I dive into the review? | 18:24 |
| fwereade | niemeyer, please | 18:24 |
| niemeyer | fwereade: Cool, so what's up.. tell me about a VersionWatcher :) | 18:24 |
| fwereade | niemeyer, so... it seemed to me that we discussed the python style of related-unit watching | 18:25 |
| fwereade | niemeyer, in which we track settings version changes | 18:25 |
| niemeyer | fwereade: Ok | 18:25 |
| fwereade | niemeyer, and then, when we come to execute hooks, we grab the latest settings for a given unit relation and then cache them for the duration of the hook | 18:26 |
| fwereade | niemeyer, this feels to me like unnecessary extra work -- we have the node data, we reduce it just to a version, and then later we grab the node data again and do extra work to make it appear constant | 18:26 |
| niemeyer | fwereade: Kind of.. I think you know it, but just to be super clear, it's cached at the time of first query | 18:26 |
| niemeyer | fwereade: Right? | 18:26 |
| fwereade | niemeyer, yes, first query per unit relation per hook, AIUI | 18:27 |
| niemeyer | fwereade: Ok | 18:27 |
| fwereade | niemeyer, it seems to me that this is an awful lot of work to go to when we could just grab the settings as they change | 18:27 |
| niemeyer | fwereade: Okay, can we please have a step back.. you seem to be assuming an implementation | 18:27 |
| niemeyer | fwereade: Where does that assumption comes from? | 18:28 |
| niemeyer | come | 18:28 |
| niemeyer | fwereade: Then, as a follow up question, what does "as they change" mean in the last sentence | 18:28 |
| fwereade | niemeyer, well, I was trying to suggest that that implementation, despite the fact that it's what we do in python, is an awful lot of work for little benefit, when we have all the data available anyway | 18:28 |
| niemeyer | fwereade: What does grab the settings as they change mean? | 18:29 |
| fwereade | niemeyer, it means we should, IMO, record the actual contents of the node (in one form or another... I kinda favour plain ol' map[string]interface{}, but that's by the by) | 18:30 |
| fwereade | niemeyer, rather than just the version of the node | 18:30 |
| niemeyer | fwereade: Okay, so.. | 18:30 |
| fwereade | niemeyer, I don;t think I'm assuming an implementation -- I'm comparing a proposed one with the existing python one | 18:30 |
| fwereade | niemeyer, and trying to get buy-in on the idea that it is worth pursuing | 18:31 |
| niemeyer | fwereade: I meant it assumes an approach already, when it wasn't clear what this was about | 18:31 |
| niemeyer | fwereade: I'm not judging, just trying to understand the background | 18:31 |
| fwereade | niemeyer, I understand, I clearly haven't communicated this especially well | 18:31 |
| niemeyer | fwereade: I think I have a better idea now | 18:31 |
| niemeyer | fwereade: As I understand it, there are two different problems, and they are being conflated in a single piece, which confuses the problem statement a bit more | 18:32 |
| fwereade | niemeyer, I definitely took the wrong approach yesterday -- "this leads to a more consistent hook execution environment" is fundamentally not interesting, but "this saves us a bunch of work" hopefully leads to a more interesting discussion | 18:32 |
| niemeyer | fwereade: The first problem is caching | 18:33 |
| fwereade | niemeyer, ok, yes; | 18:33 |
| niemeyer | fwereade: Caching is done so that we can implement more manageable data visibility for hooks | 18:33 |
| niemeyer | fwereade: So in effect, it's about *not showing changes* | 18:34 |
| niemeyer | fwereade: Slightly ironic, as the other problem is on the opposite end | 18:34 |
| niemeyer | fwereade: The second problem, is related to restarts | 18:34 |
| fwereade | niemeyer, yes: during the execution of a single hook, we don't want the environment to visibly change | 18:34 |
| niemeyer | fwereade: (at least the settings data) | 18:35 |
| niemeyer | fwereade: So, with the second problem, we have the issue that we may lose events | 18:35 |
| niemeyer | fwereade: Because there's of course zero guarantee that we'll be permanently connected to whatever the backend turns out to be | 18:35 |
| fwereade | niemeyer, indeed so | 18:36 |
| niemeyer | fwereade: So in that sense, we're interested in knowing whether things have changed since we last looked | 18:36 |
| niemeyer | fwereade: where "last looked" may be a process restart ago | 18:36 |
| niemeyer | fwereade: For that, we don't really care about what the content was | 18:37 |
| fwereade | niemeyer, AIUI, though, the losing-events problem is always potentially in play... ZK doesn't guarantee I'll see 2 changes to a node that changes twice, does it? | 18:37 |
| niemeyer | fwereade: Because we just wanna know if we're supposed to tell the charm implementation that the data has changed, via a hook execution, or not | 18:37 |
| fwereade | niemeyer, sure | 18:37 |
| niemeyer | fwereade: It doesn't, but that's fine. We offer at-least-once semantics | 18:37 |
| fwereade | niemeyer, but the actual data tells us that just as well as the version does | 18:38 |
| fwereade | niemeyer, in fact, it tells us better | 18:38 |
| niemeyer | fwereade: Without taking into consideration any implementation suggestions, | 18:38 |
| fwereade | niemeyer, if there's a change followed by a revert while we're offline, diffing the settings reveals that there are no changes we care about | 18:38 |
| fwereade | niemeyer, sorry, yes, go on | 18:38 |
| niemeyer | fwereade: it's a bit strange to cache a blob of, say, 64kb, when the text "42" would do | 18:39 |
| fwereade | niemeyer, ok, so this is an optimization? | 18:39 |
| niemeyer | fwereade: It's not an optimization.. I'd call it non-silliness, from that specific perspective | 18:39 |
| fwereade | niemeyer, ok, that is indeed a reasonable perspective | 18:40 |
| niemeyer | fwereade: But go on, you seem to have more detials | 18:40 |
| niemeyer | details | 18:40 |
| niemeyer | fwereade: This was just background | 18:40 |
| fwereade | niemeyer, I'm not certain that perspective is worth the extra complexity involved in throwing away data and then reconstructing and caching it, when caching from the get-go leads, in my mind, to a much simpler model | 18:41 |
| niemeyer | fwereade: Sorry, but you're again jumping on a bit | 18:42 |
| niemeyer | fwereade: Why do we need to reconstruct and cache anything? | 18:42 |
| niemeyer | fwereade: This data is old.. we don't use it anymore. | 18:42 |
| fwereade | niemeyer, well, distributed system, the data is always old | 18:42 |
| niemeyer | fwereade: No, that's not what I mean | 18:43 |
| fwereade | niemeyer, ok; sorry to be dense; please restate | 18:43 |
| niemeyer | fwereade: In the scenario I just described, we track the version so that, in case of restarts, we know whether we must run a hook or not | 18:43 |
| niemeyer | fwereade: If we do detect an out-of-date situation, we run the hook | 18:43 |
| niemeyer | fwereade: We don't care about what the *data* used to be.. that 64kb fromt the old version is trash | 18:44 |
| fwereade | niemeyer, ok, and that is a slightly eager but reasonable heuristic for rerunning hooks | 18:44 |
| niemeyer | fwereade: Eager? | 18:45 |
| fwereade | niemeyer, a newer version does not necessarily imply that the data we care about has changed | 18:45 |
| niemeyer | fwereade: | 18:45 |
| fwereade | niemeyer, but that's not a flaw worthy of the term flaw, let alone worth arguing over | 18:45 |
| niemeyer | fwereade: Does it generally change or not when we bump a version? | 18:45 |
| niemeyer | fwereade: We shouldn't be bumping the version of settings nodes without changes | 18:46 |
| niemeyer | fwereade: I'd consider that a bug | 18:46 |
| fwereade | niemeyer, sure, understood, but versions don't revert when the data does -- but regardless, can we drop this? it's a derail | 18:46 |
| niemeyer | fwereade: Okay, yes, agreed regarding reverts, and agreed regarding derail. | 18:47 |
| niemeyer | fwereade: Okay, now, should we talk about VersionWatcher in that context, or is there some background that is missing in that brainstorm? | 18:48 |
| fwereade | niemeyer, ok, that sounds interesting | 18:48 |
| fwereade | niemeyer, if there's some that's relevant it will no doubt become apparent | 18:49 |
| fwereade | niemeyer, my reading was that *if* we're tracking settings node versions, a VersionWatcher is just the ticket | 18:49 |
| niemeyer | fwereade: Well, I'm not sure, and that's what I'd appreciate gaining more insight into | 18:49 |
| fwereade | niemeyer, I'm hoping that you have some insight I've missed here ;) | 18:50 |
| niemeyer | fwereade: When the agent *is* running, we're interested not only in the version but also in the data, since as we've discussed it may be queried and cached | 18:51 |
| fwereade | niemeyer, ...ok, wait, I thought you were just arguing against caching the data outside of indivdual hook executions | 18:51 |
| niemeyer | fwereade: I'm arguing against saving it persistently | 18:52 |
| niemeyer | fwereade: and trying to understand whether we should be caching it in memory or not | 18:52 |
| niemeyer | fwereade: to solve problem *1* | 18:53 |
| fwereade | niemeyer, ah-HA, this is a distinction that had entirely escaped me | 18:53 |
| fwereade | niemeyer, ty for clarification | 18:53 |
| niemeyer | fwereade: Problem 2 doesn't require data.. problem 1 does | 18:53 |
| fwereade | niemeyer, this is true; I maintain that the same data *could* be used to solve problem 2 as well, but I agree that it is not necessary when one considers problem 2 in isolation | 18:56 |
| niemeyer | fwereade: It could. It's just misoptimization. :) | 18:57 |
| fwereade | niemeyer, that may well be so :) | 18:57 |
| fwereade | niemeyer, although what I was trying to optimize when I thought of it was simplicity of implementation, which is IMO a pretty good candidate for first-thing-to-optimize | 18:59 |
| niemeyer | fwereade: Comparing two integers counts highly in my simplicity-meter | 19:01 |
| niemeyer | :) | 19:01 |
| niemeyer | fwereade: You may have further information that reveals why it is simpler to do something else, though | 19:01 |
| fwereade | niemeyer, it's simply that we don't need to get and cache individual relation settings in a hook context if we already have them available | 19:02 |
| niemeyer | fwereade: I agree! | 19:03 |
| niemeyer | fwereade: That's about problem 1, though. | 19:03 |
| niemeyer | fwereade: And we get a version every time we get data out of ZooKeeper | 19:03 |
| niemeyer | fwereade: So there's a relationship between them | 19:04 |
| niemeyer | fwereade: We can solve both problems with a single watcher, cache the data for problem 1, and keep the version around for problem 2.' | 19:04 |
| fwereade | niemeyer, ok, I did think of just extending ContentWatcher to include the version | 19:05 |
| niemeyer | fwereade: +1 | 19:05 |
| fwereade | niemeyer, and let the clients discard what they will | 19:05 |
| fwereade | niemeyer, I think I have to go now; I think I'll want to talk about this some more over the next couple of days though | 19:06 |
| niemeyer | fwereade: Sounds good. Hopefully that was useful. | 19:07 |
| fwereade | niemeyer, thank you, I think we;re making progress :) | 19:07 |
| niemeyer | fwereade: Glad to hear it, and you're welcome | 19:07 |
| fwereade | niemeyer, I'm not quite there yet though ;) | 19:07 |
| fwereade | niemeyer, cheers | 19:07 |
| fwereade | niemeyer, happy evening :) | 19:07 |
| niemeyer | fwereade: Have a good one | 19:15 |
| niemeyer | Aram: So, back to your branch | 19:18 |
| Aram2 | right | 19:21 |
| === Aram2 is now known as Aram | ||
| niemeyer | Aram: Had another pass: https://codereview.appspot.com/6304099 | 19:32 |
| niemeyer | Aram: Probably the last one | 19:32 |
| niemeyer | Aram: It's looking great | 19:32 |
| Aram | niemeyer: thanks for the review. | 23:08 |
| niemeyer | Aram: My pleasure | 23:08 |
| davecheney | howdy, anyone seen mark | 23:14 |
| davecheney | https://twitter.com/campoy83/status/215255686592995331 | 23:19 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!