[00:57] axw: looking at 1.25 PR. interesting that in 2.x, we have leaseClientId as an attribute on State and hence set that as part of State.start() instead of during lease worker startup as your 1.25 change does [00:58] wallyworld: yeah probably should do it in 1.25 too, since it should be a terminal error. just made things a little simpler this way [00:59] axw: yeah, it may be what's proposed is good enough. i guess if we can't get the clientId stuff will still break, but in a different way [01:02] anastasiamac: hml: axw: forgot to remind - if there's anything you want to add to release notes for beta3, now is the time [01:03] wallyworld: nothing coming to mind [01:03] yeah, mostly bug fixes this time [01:03] i added a few key things [01:04] wallyworld: m all good but will confirm with the rest of the squad later on in the day [01:04] ty, may be too late then :-) [01:06] nah I don't think so [01:06] really? 4pm our time?.. [01:06] only thing was the mutex fix, and that's out :~( [01:06] wallyworld: most bug fixes anyway :) [01:07] indeed [01:07] anastasiamac: yeah veebers is on fire, i expect it wil be done by 4pm our time as that will be well into his evening and he will already be on the beer :-) [01:08] wallyworld: :D [01:11] wallyworld: hah more like cleaning up spit-up and baby wees off my hands, but hopefully there will be a beer there sometime ^_^ [01:11] the joys of parenthood [01:11] you'll be doing the same thing to yuorself when you hit 80 [01:11] hah ^_^ [01:12] s/when/if/ /me dies of parenthood related stress [01:32] wallyworld: https://github.com/juju/mutex/pull/5 [01:33] currently running the network-health test with the old code to repro locally, then will run with this to make sure it's the problem [01:33] it's definitely *a* problem though [01:34] axw: looking. good we have CI [01:34] indeed [01:39] axw: lgtm, just a comment typo [01:40] wallyworld: heh thanks [01:58] wallyworld: do you have rights to merge https://github.com/juju/mutex/pull/5 ? [01:58] yup, i think so [02:00] axw: added you with perms to merge [02:00] try it? [02:00] wallyworld: thanks [03:24] wallyworld: changes we discussed have been made, pr updated [03:47] axw: what was happening with the mutex? [03:47] axw: was it inheriting the lock, and only one closed it, so it was still "held" or something? [03:48] thumper: subprocess inherited the file descriptor, so flock was held by subprocess even though juju closed its FD [03:48] but that sub process would have finished as well, releasing it, right? [03:48] thumper: yeah, unless it spawned off something like "timeout" [03:55] axw: is that what the tests were doing? [03:55] so that explains why some hooks were fine and others not yes? [03:57] thumper: the network-health test starts a server from one of the hooks, and never stops it AFAICT [03:57] thumper: yeah, depends on the hook implementation for sure [03:58] ah.. [03:58] ok, I like to know why things fail :) [05:10] axw: the state we attach to the apiRoot object - the one facades get via ctx.GetState() - i can't see where that gets closed. do you know? [05:12] wallyworld: looking, have to remind myself [05:12] i also can't see where we Stop() resources [05:13] wallyworld: it's the same State that's attached to the Server [05:13] and we stop that one? let me look [05:13] wallyworld: which is closed by the agent [05:13] ok, ta [05:14] axw: trying to decide whether to add a "controller" Resource [05:14] rather that yet another attribute on server [05:14] as resources i think is the preferred way to add stuff to the facade context [05:15] wallyworld: I'd prefer it was part of the Server [05:15] there's a snaky comment from willian next to ctx.State() that it should be a resource [05:15] any reason for preferring to add to Server? [05:16] wallyworld: I don't know what that would buy us [05:16] wallyworld: because State and StatePool are there. and because it's not a per-connectio nresource [05:16] keeps facade Context interface stable [05:16] it's owned by the Server, not by the connection [05:17] and eventually it shouldn't even be owned by the Server, but passed in and owned by a manifold [05:18] hmmm, ok, let me think on it [05:18] wallyworld: perhaps we should start a new branch for this work? I have some things I'd like to land too, but have been holding off until after 2.3 [05:19] a feature branch? yeah maybe [05:19] wallyworld: though possibly my work will make fixing https://bugs.launchpad.net/juju/+bug/1642618 easier anyway [05:19] Bug #1642618: enable-ha using existing machines breaks agents [05:19] i think we can do a few things on develop [05:20] a lot of this stuff is shifting furnature around [05:21] wallyworld: sometimes the furniture is accidentally load bearing though :) [05:22] that is true [05:23] i'll do this work and see how it looks [05:23] can propose against a fb if it looks too messy [05:56] axw: sorry, another question. do you know why we pass in a State to startStateWorkers() but then provide a separate openState() method to the apiserverWorkerStarter? we barely use the result from openState(). but noentheless, why not just use the one we already have, especially when that one we alrready have comes from the manifold [05:57] openState() as it turns out provides a State used to get ControllerConfig(), which we could just pass in from the original st, and create an audit log sink, which we also could get from a manifold. [05:57] wallyworld: I thought there was a comment, but can't find it now. it's because the apiserver kills the state workers [05:58] wallyworld: so if hte apiserver restarts, it needs a new State [05:58] sas the old's watchers, lease manager, etc. will have been stopped [05:58] *as [05:58] wallyworld: this is one of the things I have fixes for in the pipeline [05:58] wallyworld: if it makes it easier for you to do what you want to do, I can look to land my work sooner rather than later [05:59] yeah, that would be good. i want to thread through a controller [05:59] but need a state [05:59] wallyworld: I have a branch that will move the apiserver to a manifold, too, but needs a bit more work [05:59] wallyworld: probably better to get that in, then you can use the controller manifold? [05:59] would be good to get that stuff landed [05:59] yes [05:59] maybe a fb then? [06:00] it's all very messy atm [06:00] would be good to get this cleaned up [06:00] wallyworld: I think that'd be prudent. then fold it into develop as soon as we branch [06:00] yup [06:00] wallyworld: first step is to land https://github.com/juju/juju/pull/8038 in a feature branch then [06:01] ok, and it's been approved [06:01] wallyworld: want to review for me? it has rog's +1, would be good to get a second opinion tho, as it's core [06:01] ok [06:01] i'll create the fb [06:01] as well [06:05] axw: https://github.com/juju/juju/tree/state-controller-refactor [06:05] looking at PR now [06:05] wallyworld: thanks [06:06] wallyworld: note there's two commits, might be easier to review them independently [06:06] one after the other, rather [06:06] ok [06:07] axw: i had a look earlier - we disucssed a bit already this week, i'll re-review [06:07] wallyworld: we did? I don't remember htat at all :o [06:07] * axw is losing the plot [06:08] i was asking about exposing context to api leayer, and clarifying operation on apiserver sid eetc [06:08] * wallyworld has already lost the plot [06:08] sounds vaguely familiar :p [06:19] axw: +1 to go into fb. i'd like to ensure we get some good CI runs (which we will) and do some more extensive manual testing with all the other changes we will be landing soon [06:20] wallyworld: ok. I think I broke something in the API package with a change I made about an hour ago, will fix that and land [06:20] sgtm [06:20] axw: and then you had a followup bit of work to go on top of this? [06:21] i'll look to pick up my stuff next week perhaps [06:21] wallyworld: yes, to move apiserver to the dependency engine [06:21] great, ok [06:21] then there'll be more followups on top of that I'm sure [06:21] yep [06:22] wallyworld: e.g. once we move to using a single StatePool, the introspection code can be cleaned up a bit [06:22] it's really awkward atm, the apiserver starter needs to create a new StatePool, then set that on the agent so the introspection worker can reference it === frankban|afk is now known as frankban [08:45] wpk@minnie:~/dev/src/gopkg.in/juju/names.v2$ grep -r IsValidRelation * [08:45] wpk@minnie:~/dev/src/gopkg.in/juju/names.v2$ grep -r IsValidRelation relation_test.go [08:45] c.Assert(names.IsValidRelation(key), gc.Equals, isValid) [08:45] magic [08:45] c.Assert(names.IsValidRelation(peerKey), gc.Equals, isValid) === frankban is now known as frankban|afk