=== nijaba_ is now known as nijaba [07:58] morning [08:13] heya TheMue [08:14] heya wrtp [08:14] fwereade_: moanin' [08:14] TheMue: hiya [08:14] fwereade_, wrtp: hi [09:22] hello. [09:28] Aram: hi [09:31] so, the MongoDB tailable cursors are a piece of shit. [09:44] Aram: tailable? [09:44] TheMue: http://www.mongodb.org/display/DOCS/Tailable+Cursors [09:45] the thing I wanted to use to avoid polling. [09:45] but it's a piece of crap. truth is MongoDB doesn't have this simple feature, and encapsulates server side polling inside a tailing API. [09:46] but it's not a real, fast, delivery mechanism, in fact it's slower then polling. [09:46] Aram: ic, yes, would be nice for queues. [09:46] if you starve the data from mondogb it will sleep on the server side for about 10 seconds. [09:46] it doesn't really block and wait for new data. [09:46] it just creates a thread that wakes after 10 seconds to do a new query. [09:48] Aram: *sigh* instead of really pushing it to an open connection. [09:48] I spent a day debugging why my watcher made tests take about one minute per test. [09:48] yeah. [09:58] Morning all! [10:04] niemeyer, heyhey [10:09] niemeyer: hiya [10:29] Reviews are clean.. I'll jump right into proposal implementation for presence.. got a nice design yesterday, which should scale to a reasonable level. [10:54] fwereade_: Do we have any place that checks for liveness in-place with Alive instead of AliveW? [10:54] fwereade_: Or, can you imagine why we'd do that? [10:54] niemeyer, hmmm, I'm not sure we do [10:55] niemeyer, Unit.AgentAlive [10:55] fwereade_: Cool, I'll try to go with a more conservative design [10:55] niemeyer, it's used in status [10:55] fwereade_: Ah, good point, cheers [10:56] niemeyer, (and Machine.AgentAlive too) [10:56] fwereade_: Yeah, for the same reason I expect [10:56] niemeyer, yeah [11:25] niemeyer: " [11:25] It'd be nice to avoid yet another place where the environment configuration is [11:25] manipulated magically. What's wrong with doing it when that configuration is [11:25] created? [11:25] " [11:25] niemeyer: i thought it was nice to ensure that the agent tools version was always correctly set [11:27] wrtp: It is nice, but that's not the agent tools version [11:27] niemeyer: i could easily change things so that jujud bootstrap-state (and dummy.Environ.Bootstrap) is responsible for setting the version, but that leaves a small window where if you do: {juju bootstrap; juju upgrade} the upgrade might not see the agent version, so must fail [11:27] niemeyer: proposed agent tools version? [11:28] wrtp: Exactly.. we have a setting for the current agent tools version, and it lives elsewhere [11:28] niemeyer: sorry, all this is talking about the proposed version, not the current version. [11:28] wrtp: Exactly [11:28] wrtp: That's why the suggested location seems awkward [11:29] wrtp: There's such window, actually [11:29] niemeyer: so what should upgrade-juju do if it sees that the version has not yet been set? my inclination is for it just to fail, as it's a marginal case. [11:29] wrtp: Why would the version not 'be set? [11:29] wrtp: That configuration that you're manipulating in initialization is coming from somewhere [11:30] niemeyer: hmm, of course [11:30] niemeyer: doh! [11:30] wrtp: :) [11:31] I'll get some coffee and bbiab [12:28] niemeyer: should be better now: https://codereview.appspot.com/6494057 [12:29] niemeyer: any good idea on how to invalidate the environment w/o writing direct to ZK? sadly JujuConnSuite embeds only ZkSuite to start ZK, but doesn't expose a ZK connection to use. [12:30] TheMue: can't you get the zk connection from the State? [12:31] wrtp: different package, the provisioner_test. [12:31] wrtp: Sorry, I think we're still not understanding each other.. [12:31] niemeyer: oh, darn [12:31] niemeyer: should agent-version be set when reading environments.yaml? [12:31] niemeyer: that seemed wrong to me [12:31] wrtp: Please check out environs/ec2/ec2.go:245 [12:32] niemeyer: in my branch? [12:32] wrtp: In trunk [12:33] niemeyer: i thought about that too, but it didn't seem quite right somehow [12:33] wrtp: See the tools there, two lines below it? [12:33] wrtp: It's all there [12:34] wrtp: What's wrong is that there's something generic being done in the provider [12:34] wrtp: But that's pending even despite this change [12:34] publicAttrs is really supposed to be generic [12:35] niemeyer: are you thinking of something like this: http://paste.ubuntu.com/1175776/ [12:35] ? [12:36] niemeyer: or do you think the agent-version should be in the Config that the Environ was created with? [12:36] wrtp: I'm thinking about something like environs.BootstrapConfig(e environ.Environ, tools *Tools) [12:37] wrtp: That is mainly publicAttrs, plus the new agent version logic [12:39] wrtp: The former, from your two options, except it'd be within publicAttrs, which is in fact BootstrapConfig [12:41] niemeyer: that seems like a reasonable thing to do. not entirely sure that it shouldn't be environs.BootstrapConfig(p EnvironProvider, cfg *config.Config, tools *Tools) [12:42] niemeyer: which gives the provider potential leave to manipulate the config before it goes out into the cloud. but... it probably doesn't matter much. [12:43] wrtp: e is what's calling BootstrapConfig.. [12:43] niemeyer: i realise that [12:43] wrtp: It means it manipulate the result.. [12:44] it can [12:44] niemeyer: but all BootstrapConfig will use e for is to get its Config, no? [12:44] niemeyer: and the provider [12:44] wrtp: e is the thing that "sends t'he config into the cloud" [12:44] wrtp: It has the result of BootstrapConfig in its hands [12:45] niemeyer: oh, hold on - will BootstrapConfig actually call Environ.Bootstrap? no, it couldn't. [12:45] niemeyer: environs.BootstrapConfig(e environ.Environ, tools *Tools) *config.Config [12:45] wrtp: No.. it will call e.Config() [12:45] presumably [12:45] wrtp: and e.SecretAttrs [12:46] e.Provider().SecretAttrs, actually [12:46] niemeyer: yes - SecretAttrs is a method on EnvironProvider [12:46] wrtp: Yep, and Config is in e [12:46] wrtp: It has all it needs [12:46] niemeyer: if all BootstrapConfig needs is the provider and the config, why not just pass those in explicitly [12:46] niemeyer: makes it easier to test, for one thing. [12:47] wrtp: Sure.. that works too [12:47] niemeyer: cool. [12:47] niemeyer: that type sig makes it more obvious what it's doing too. [12:52] YEAH! [12:53] Found my missing function that helps me. [13:01] niemeyer: a quick question: ec2.environProvider.publicAttrs uses UnknownAttrs not AllAttrs. i *think* that's a bug - do you concur? [13:02] wrtp: Looking [13:04] wrtp: yeah, looks like a bug. Good catch [13:04] niemeyer: it would be caught by tests soon enough :-) [13:04] niemeyer: i'm a bit surprised it wasn't caught already [13:05] wrtp: Yeah, you've probably saved Dave quite a bit of head-scratching [13:05] wrtp: I've filed a bug about updateSecrets yesterday [13:05] wrtp: It's still sending the full config ATM.. that's why the bug isn't visible yet [13:05] niemeyer: this is my initial stab at BootstrapConfig: http://paste.ubuntu.com/1175826/ [13:06] s/providerInstance/p/ obviously :-) [13:06] wrtp: Beautiful! [13:21] GAAAAH it's testing.HTTPServer that occasionally hangs the Uniter tests [13:21] * fwereade_ has a relieved [13:21] * fwereade_ is going for a giggie [13:21] ciggie [13:28] fwereade_: Woohay :) [13:54] * wrtp wants a cobzr branch feature that lists branches in time-last-modified order. [13:55] it's easy to create a script that does this. [13:56] Aram: comparing times isn't that easy in a script, but yeah, i could write a program to do it. [13:57] Aram: actually, the timestamps used are easy to compare lexicographically, so not too bad. [13:57] it would be great if it could print unix time or some other simple integer === mramm2 is now known as mramm [15:06] All going well.. review queue is empty, lots of things being merged.. [15:07] Presence is well in progress too [15:07] I'll step out for a slightly extended lunch (I expect ~1:30) to run some errands, and will continue on that [15:16] fwereade_, TheMue: do you think it should be an error if State.AddService is called twice with exactly the same arguments? [15:16] wrtp, hmm, that's a bit philosophical ;p [15:17] i.e. a service with the same name and charm being created twice? [15:17] yes. [15:17] wrtp, yeah, I *think* it should be [15:17] fwereade_: currently it seems to create a new service with an identical name. i don't think that's right. [15:17] wrtp, whoa, that definitely sounds screwed up [15:17] wrtp: I would say so. [15:17] in mstate it good [15:17] fwereade_: FirewallerSuite.TestNotExposedService does it. [15:17] it's [15:18] fwereade_: but i haven't actually checked what's happening - i'm just assuming from a brief look at the code [15:18] but the test passes, so it must be doing something [15:18] wrtp, heh, sorry, I'm not familiar with that bit [15:18] TheMue: any particular reason that that test calls AddService("wordpress", s.charm) twice in the same test? [15:19] wrtp, (the perspective that gives me pause is "well, at the end of the call there is a service named wordpress with the right charm, everyone should be happy") [15:19] wrtp, if there are *two* services with the same name, yeah, that's just wrong [15:19] fwereade_: ahem, yeah, but service names should be unique [15:19] wrtp: where exactly do you see it? i see it only once. [15:19] i think there must be [15:20] wrtp: line 74 [15:21] TheMue: oh drat! i'm just mistaking my own copy and pasted stuff for the original [15:21] * wrtp hangs head [15:21] sorry, false alarm! [15:21] *ROFL* [15:22] could've sworn i checked [15:22] haha, I'm pretty sure I've done worse :) [15:22] wrtp: btw, the environ config stuff is in [15:22] TheMue: brilliant, thanks! [15:54] TheMue: would appreciate if you could have a look at this. i hope that your tests are still fundamentally the same although i've shuffled things around a little. https://codereview.appspot.com/6499056 [15:55] *click* [16:01] wrtp: LGTM, only one smaller note. [16:01] TheMue: thanks [16:02] TheMue: tbh i'm not sure it's even worth keeping that check around - it's just checking that StartInstance works, which we test in other places. [16:02] TheMue: it looks to me like it was a debugging remnant [16:03] TheMue: i left it in in case it was important - what do you think? [16:06] wrtp: Yeah, I think it's by Dave and it's just to get sure that the instance is started. [16:06] TheMue: ok [16:06] TheMue: i'll delete it i think [17:13] * niemeyer waves [17:53] niemeyer: ping [17:53] wrtp: yo [17:53] niemeyer: one little wrinkle to our upgrade scheme change [17:53] wrtp: Uh oh :) [17:54] niemeyer: i think the dev flag has to be in the environ config [17:54] niemeyer: rather than an argument to upgrade-juju [17:54] wrtp: Why's that? [17:54] niemeyer: because the agents need to know if they can upgrade to a dev version or not [17:54] wrtp: Not that it sounds bad.. it actually sounds reasonable, but just curious [17:54] wrtp: Ah, interesting [17:54] niemeyer: and that's the best (i think) way of communicating that [17:55] niemeyer: it actually works out quite well i think [17:55] wrtp: Yeah, it sounds good [17:55] niemeyer: unfortunately all these changes mean i am unlikely to make tomorrow's deadline :-( [17:56] wrtp: Well, it'd be awesome if we have that well aligned by the end of the day tomorrow at least [17:56] niemeyer: to prepare for the other change, i'm changing FindTools to and BestTools to look like this: [17:57] niemeyer: http://paste.ubuntu.com/1176355/ [17:57] wrtp: That flag will likely be used for other stuff as well, btw [17:58] niemeyer: the "Highest" flag is useful because we don't want the version of the client to determine the highest version that can be deployed [17:58] niemeyer: and i'm happy to lose the bool tbh [17:59] wrtp: Why would we not want that? [18:00] niemeyer: why would we? if there's a version later than my current client in the cloud, i'd want it to be used when i bootstrap, i think [18:00] niemeyer: it's the same semantics we had previously [18:00] niemeyer: the <= semantics are only important when upgrading, i think [18:01] wrtp: Why? What's the logic? [18:01] niemeyer: if the logic *always* selects the highest version, then we can never downgrade. [18:02] wrtp: Uh.. now I'm even more lost :) [18:02] wrtp: Hmm [18:02] niemeyer: ok. so what we *did* have was that BestTools always selected the highest available version with the same major number. [18:03] niemeyer: now clients are using BestTools to choose a suitable set of tools when told to upgrade to a particular version number. [18:03] niemeyer: so we make sure (your idea) that they don't upgrade to a higher version number than requested. [18:04] wrtp: The idea was also to ensure compatibility with the client, but I guess it doesn't matter.. we have to preserve it either way [18:04] niemeyer: but the old semantics are still applicable IMHO when initially deploying [18:04] niemeyer: yes [18:05] wrtp: Okay.. can we please just tweak the flag names a bit.. Dev is too generic.. we need some kinds of prefix/suffix such as DevTools [18:05] niemeyer: sure, suggestions welcome. DevTools, HighestTools? [18:05] HighestVersion? [18:06] wrtp: You mean DevVerison as well? [18:07] niemeyer: i hadn't, but actually that could work well [18:07] wrtp: NewestVersion and NewestCompatVersion [18:07] ? [18:08] niemeyer: the flags are orthogonal [18:08] wrtp: Or VDev VNewest VNewestCompat [18:08] wrtp: So VNewest and VCompat [18:09] wrtp: (or NewestVersion and CompatVersion) [18:09] niemeyer: i'm not sure i like Compat [18:09] wrtp: Suggestions/ [18:09] ? [18:09] niemeyer: because dev versions *are* compatible with non-dev versions [18:09] niemeyer: (or should be) [18:09] niemeyer: we already have Version.IsDev [18:09] wrtp: Yep, the point of compat is that 4.0 is newer than 3.0 [18:10] niemeyer: ah, currently it *always* chooses a compatible version, so that's not an option [18:10] wrtp: Yes, but it is [18:10] niemeyer: i'd like to add that later, if i may [18:10] wrtp: Sure, but we don't have to wait until later to fix the flag name [18:10] niemeyer: sure, but DevVersion, NewestVersion, and CompatVersion would work ok [18:11] niemeyer: as orthogonal flags [18:11] niemeyer: CompatVersion to be added later [18:11] wrtp: Compat is the behavior you're introducing right onw [18:11] wrtp: The lack of Compat is what will come later [18:12] niemeyer: ah, so maybe we should make the flag opposite in meaning [18:12] niemeyer: AllowIncompatibleVersion :-) [18:12] wrtp: I don't see why.. the three flags above look nice and are orthogonal as you suggest [18:13] niemeyer: if i add the flag now, then i have to add loads more tests, and i'd prefer not to for the time being. there are so few calls to BestVersion and FindTools that it's trivial to add CompatVersion later [18:13] niemeyer: (like about 4 calls) [18:13] wrtp: Sorry, I don't get it.. you don't need any new tests in addition to what you'd have anyway [18:14] wrtp: I'm not suggesting you change the logic, I'm suggesting we have the real flags we want since there's zero cost in that [18:14] niemeyer: yes, i have to test what happens when that flag is *not* specified [18:14] wrtp: if flag not specific { panic("not yet") } [18:14] wrtp: !? [18:14] niemeyer: ok, i'll do that [18:16] wrtp: Cheers [18:36] niemeyer: https://codereview.appspot.com/6500052/ [18:36] niemeyer: i apologise for the size - i haven't had time to split it into several CLs. [18:37] niemeyer: i have to go now. see you tomorrow! [18:37] wrtp: np, I'll try to have it reviewed before the EOD [18:37] wrtp: Have a good evening [18:37] niemeyer: that would be marvellous! [19:29] sent an e-mail about the sprint [19:30] please book travel as soon as you can (and definitely before the weekend!!!) to save costs