[03:26] shitballs, I deleted by ssh branch before pushing it [03:26] crap -- have to start again [06:36] davecheney: aw shit, i've managed to avoid doing that so far, but i've come close [06:37] davecheney: morning BTW! [06:37] fwereade: hiya [06:38] wrtp, heyhey [06:38] davecheney: i wanted to use that branch too [06:38] davecheney, morning [06:39] wrtp, sorry I haven't reviewed it properly yet, I'll do that right now [06:39] fwereade: np. that would be great, ta! [06:41] wrtp: s'ok, rewritten already [06:41] fwereade: is there are reason that state.Initialize doesn't take a *config.Config ? [06:41] s/are/a/ [06:41] wrtp: maybe that is my failt [06:41] fault [06:41] wrtp, I don't think so, no [06:41] wrtp: also, since I patched that issue in the TLS lib [06:41] wrtp, ie I think it probably should [06:41] not a single EOF from etc [06:42] fwereade: i was just looking at if len(c.EnvConfig) == 0 and thinking that it should probably be checking that the config was accepted by the config package [06:42] also, if you are following tip, cgo is broken again, http://codereview.appspot.com/6501101/ [06:42] ^ apply this for general satisfaction [06:44] davecheney: i haven't followed tip for a while now, as i haven't been working on go core stuff. there is something i've been meaning to do though, and i may have some time in the next few days as carmen's away for a bit. [06:45] wrtp: some awesome imrpovements to 6g coming [06:45] at least 15% improvement across the board [06:45] and substantial reduction in stack size [06:45] davecheney: which bit are you referring to? [06:45] davecheney: (i've been following golang-dev, but skimming it and missing quite a bit) [06:46] http://codereview.appspot.com/6501110/ [06:46] http://codereview.appspot.com/6494107/ [06:46] and so forth [06:46] filtering out all the LEAQ's [06:46] issue 1914 [06:47] davecheney: great. yeah, there's so much potential optimisation. [06:47] davecheney: i'm really looking forward to russ's Grand Optimisation branch whenever it might arrive [06:47] wrtp: this one is nice too http://codereview.appspot.com/6506089/ [06:49] the big improvements are in stack usage [06:49] reflect.Value.call consumes a kilobyte of stack on x64! [06:49] davecheney: that will help a lot [06:49] davecheney: woah [06:49] remy has shaved 1/4 off that [06:49] but it's stupid large [06:50] so any time that method comes near you, it's a stack split for sure [06:50] and an expensive one [06:50] davecheney: i'm not too surprised - it's a complex piece of code [06:50] nah, it's just long [06:50] davecheney: things will be better when the runtime knows what size of stack is likely to be necessary. [06:50] if it was split up [06:50] the stack could be reused because it works top to bottom [06:51] wrtp: i hope that is more than a pipe dream [06:51] davecheney: i don't see why it shouldn't work. a best-case stack size calculation should not be out of reach [06:51] wrtp: lp:~dave-cheney/juju-core/093-cmd-juju-ssh/ [06:53] davecheney: another optimisation i thought would be nice is to cache the function value from the itab when an interface value doesn't change. [06:53] wrtp: daniel morosing has been working on that one for a while [06:53] davecheney: that could save quite a bit in tight loops calling interfaces [06:54] wrtp: http://codereview.appspot.com/6351090/ ?maybe? [06:55] davecheney: i don't think so [06:55] davecheney: that's about interface type conversion; i'm just talking about interface calling [06:55] wrtp: does taht require runtime support ? [06:56] i'm guessing it does, but for some reason I can't think of where it is in the runtime [06:56] davecheney: i don't think so [06:56] davecheney: the compiler generates the code to call methods [06:57] which must take the interface type, and look it up in a table to get the func addr of the i impl [06:58] anyone heard from mark lately ? [06:59] wrtp, sorry, missed you; you may be right, I was trying to avoid getting sidetracked into fixing too much in one CL though :) [06:59] wrtp, reviewed https://codereview.appspot.com/6501106/ [06:59] davecheney: yeah, probably best in another CL [07:00] fwereade: thanks! [07:00] fwereade: i used VarDir because it was niemeyer's suggestion originally ("all directories are juju directories") [07:00] wrtp, you probably won't agree with it all [07:00] fwereade: but i'd personally be very happy to use JujuDir. [07:01] fwereade: in fact i agree it's a better name [07:01] wrtp, cool :) [07:01] wrtp, we'll have to see how it does then :)_ [07:01] wrtp, JujuRoot, perhaps? [07:02] fwereade: how about Machiner.simpleContainer ? [07:02] wrtp, (btw, sorry, I feel the review comments skewed rather negative, the "lots to like" is the overriding impression though) [07:02] fwereade: we can't stick with container.Simple as a global [07:03] fwereade: as we need to be able to pass in VarDir [07:03] wrtp, well, outside of testing, when do we need to do that? [07:03] fwereade: always [07:03] fwereade: there's no global VarDir any more [07:04] fwereade: and you can invoke commands with --juju-dir [07:04] fwereade: (which works now, BTW) [07:05] wrtp, hmm, I think I have an odd perspective on this, but I did a bunch of agent stuff over the w/e, and I'm (say) 90% sure that an Agent type is (1) a good thing and (2) the appropriate home for JujuDir [07:05] wrtp, so ISTM that it's not actually the container's responsibility (but that initDir and logDir are) [07:06] fwereade: i'm not sure what you mean by "the ... home for JujuDir" [07:06] fwereade: it's just a parameter [07:07] wrtp, it's a parameter used in several places, all of which IMO become nicer once an Agent type is added [07:08] fwereade: so an Agent would be a parameter to the Container? [07:08] wrtp, perhaps it would help if I propose -wip my current branch (which is very very unfinished but I hope instructive) [07:08] wrtp, I *think* so, yes, it comes out quite nice IMO [07:10] fwereade: i'll maybe give you a glance at another unfinished branch of mine to get your thoughts too [07:11] wrtp, cool -- I'm a little worried that we may be about to collide actually, a lot of the ugliness in https://codereview.appspot.com/6500095 will evaporate once your VarDir branch is merged [07:15] wrtp, the underlying insight that led me in this direction is that all the agents really *are* the same in that they should all be named, because they do in fact all correspond to a single state entity, and that messing around with --machine-id and --unit-name and not-having-one-for-provisioning is actually a side-effect of missing this insight [07:15] wrtp, ofc it may, as always, be crack [07:16] fwereade: i'm absorbing it [07:16] fwereade: i was thinking about something a little similar in some ways actually [07:16] fwereade: i wondered if we could have a single "jujud agent " command [07:17] wrtp, I've been keeping that out of mind [07:17] wrtp, because I think it is *probably* right but that we are not quite there yet [07:18] wrtp, once we have upgraders for everything, and (if it passes muster) agent.Agent, it will I think be a good time to move the upgrading task out of the Kind-specific tasks, and make it all happen at the Agent level [07:19] fwereade: the one thing i'm not sure about is having a single agent.Run for all the agents. [07:19] wrtp, ISTM that the list-of-tasks abstraction is an excellent one [07:19] fwereade: i think that's the kind of direction i was heading with my "runner" package and it was deemed crackful [07:19] wrtp, bah [07:20] wrtp, my feeling is that list-of-tasks is *exactly* the thing that differentiates multiple agents [07:20] fwereade: it depends whether we think that *all* agents *always* will be a simple set of concurrent tasks [07:21] wrtp, that STM to be the assumption underlying the worker package, and it seems to have served us well so far [07:21] fwereade: i'm not sure that's true actually [07:21] fwereade: the workers can be used in any way [07:22] fwereade: they *happen* to implement the same interface, but there's no requirement that they do so [07:24] fwereade: i do like the the factoring-out of the UpgradedError logic though. [07:24] fwereade: BTW why does this code deserve its own package? it seems like it would still live well in cmd/jujud [07:25] fwereade: ah! but you want to pass Agents around to other packages. [07:25] wrtp, yep, exactly [07:25] wrtp, like I say, might all be crack, but it seems like a small amount of code that is useful in several places [07:26] fwereade: i'm not sure. i think that simply passing JujuDir as a parameter work well for many of the methods [07:26] s/work/works/ [07:27] fwereade: and agent name when appropriate, i guess [07:27] wrtp, and agent kind... [07:27] wrtp, and frequently state info... [07:27] fwereade: when do we need agent kind? [07:28] wrtp, any time we want to get a tools dir (according to the agent-foo-123, provisioning-whatever naming scheme I thought we discussed on friday) [07:28] fwereade: that comes from the agent name, no? [07:28] wrtp, ah, ok, sorry, I misunderstood which name you were talking about [07:29] fwereade: i'm thinking that every agent has a unique name [07:29] er sorry s/agent-foo/unit-foo/ [07:30] fwereade: i think i'd be happier if the agent package had stuff for agent identification and location only, and all the *actual* agent logic lived elsewhere. [07:30] wrtp, I am also thinking that, but I've called it "badge" because ISTM that the *name* is the name of the attached state entity, and the agent itself is somewhat different [07:30] fwereade: when would you need "name" instead of "badge"? [07:32] wrtp, if one takes the Agent abstraction seriously it becomes useful to pass a *Agent into (eg) uniter, instead of the name/dir bits, and then the unit name is directly accessible from there [07:32] fwereade: but the uniter knows the *Unit? and the name comes from that, no? or perhaps the name is something else and i'm misunderstanding [07:33] wrtp, no, the Uniter has never been started with a *Unit [07:33] fwereade: why not, out of interest? [07:33] fwereade: it would seem logical [07:33] wrtp, there's no reason to? [07:34] wrtp, we have available a state and a name, and we need the state anyway [07:35] wrtp, it's just smearing the state setup across the uniter and its client to no apparent benefit [07:35] wrtp, s/it's/pregetting the unit is/ [07:36] fwereade: i think that when the uniter does upgrades, the caller of the uniter will need to get the unit anyway [07:36] fwereade: because it needs to be passed into the upgrader. [07:36] fwereade: same as the MA [07:36] wrtp, sure, but the MA takes the wrong params itself [07:36] wrtp, where's the state info? [07:37] wrtp, (another Agent field, you'll notice :)) [07:38] fwereade: i'm not sure. these are things that individual agents need, but it feels you're making them into universal parameters, and i'm not sure that's necessary. [07:38] fwereade: and, um, i think that's a red herring in fact. [07:39] fwereade: we'll still need to get the *Unit before calling the Uniter [07:39] wrtp, go on? [07:39] fwereade: so we may as well pass the *Unit into the uniter [07:40] wrtp, is it just that we want one to create the upgrader? [07:40] fwereade: yes. but if we've already got one, passing it into the Uniter seems like a fine thing. why pass in a name when you've already got the thing itself? [07:42] fwereade: BTW with your current arrangement, you *can't* pass a *agent.Agent into the Uniter [07:42] wrtp, ISTM like a nicer interface to pass a *State and a name than a *State and a *Unit [07:42] wrtp, you just talking about package dependencies? [07:42] fwereade: yeah [07:42] wrtp, yeah, doesn't feel insurmountable to me [07:43] fwereade: why a name nicer than the thing it's naming? [07:43] fwereade: i *think* it's inherent to this approach. the Agent calls the worker, which needs the Agent. cyclic dependency. [07:44] s/why a/why is a/ :-) [07:44] wrtp, surely it's just a matter of re-extracting an agent conf type and passing that around? [07:45] fwereade: so then we have *another* package, just to contain that type? [07:45] wrtp, which probably helps to address the run-mixed-with-info concerns [07:45] fwereade: why not factor out all the run stuff, and make the agent type *solely* concerned with agent storage? [07:45] fwereade: then there's no problem [07:46] wrtp, I think that's the type that holds most of the methods -- it's just Run and Stop that move elsewhere [07:46] fwereade: agreed [07:47] fwereade: in the end, i think you've got something like: type Agent {Name string; JujuDir string} [07:47] wrtp, what about kind and state info? [07:47] fwereade: (i *think* putting the StateInfo in there is mixing concerns) [07:47] wrtp, name me an agent that doesn't need a state info [07:48] fwereade: the firewaller :-) [07:48] fwereade: i know it's not its own agent, but it could be [07:49] wrtp, so is your contention that if 4 things use something, and a 5th doesn't, it is apropriate to write 5 separate code paths rather than suffer the shame and indignity of an unused param? ;p [07:49] fwereade: it's not really a matter of "which agent's don't need it" but "why is it living in this package, which is actually only to do with agent storage?" [07:49] fwereade: i don't see that this would require any extra code paths [07:49] wrtp, ah, hmm, I don;t think it is just concerned with agent storage, I think Conf is an imortant part [07:50] fwereade: Conf? [07:50] wrtp, Agent.Conf, which means we can write a Deploy wethod that works [07:50] wrtp, well, in concert with other things, it does [07:51] wrtp, rather than having the crazy doesn't-even-work-and-is-not-consistent-with-cloudinit Deploy we currently have [07:51] fwereade: an explicit StateInfo param to Deploy seems quite reasonable to me [07:51] fwereade: i'm already most of the way through fixing that [07:52] wrtp, and to the machine agent, and to the unit agent, and (I presume) to the provisioning agent as well? [07:52] fwereade: absolutely. [07:52] wrtp, sorry s/agent/worker/g [07:52] fwereade: i don't think an extra parameter is a problem [07:53] fwereade: especially as it makes it obvious that this worker connects to the state [07:53] fwereade: it would *not* be a parameter to the upgrader or to the firewaller [07:53] wrtp, what? that's not why we pass it at all [07:53] fwereade: no? [07:53] fwereade: why do we pass it? [07:53] wrtp, we pass it to the things that themselves need to set up new agents one way or another [07:54] fwereade: indeed - things that need to connect to the state [07:54] wrtp, the *State is handled outside and shared by many worker, and that really does need to be passed to everything [07:54] wrtp, an MA doesn't connect to the state [07:54] wrtp, sorry a Machiner [07:54] fwereade: it starts things that do [07:55] wrtp, yeah, exactly [07:55] fwereade: so, indirectly, yes, it does. [07:55] wrtp, I would like it if we used a definition of "connects to the state" that involved, y'know, opening a connection to the state ;p [07:55] fwereade: lol [07:56] wrtp, but yes, anyway, I see your perspective [07:56] fwereade: i think of it like a capability [07:57] wrtp, this is kinda by the by anyway [07:58] wrtp, I'm explicitly *not* proposing a common worker creation interface taking an agent, because I felt it would lead to derails ;) [07:58] fwereade: i'm just objecting to the fact we're stuffing StateInfo inside the agent package, when *nothing* in the agent package uses it. it's solely to avoid us passing an extra parameter. [07:58] wrtp, Conf uses it... [07:59] wrtp, and IMO it's a really nice thing to be able to take the exact same object and run it, or install it, or generate the scripts required to install it [07:59] wrtp, it feels like that's what an agent "is" [07:59] wrtp, you need exactly the same information to run one, and to generate a conf for it [08:00] fwereade: i'm not sure that Conf lives inside agent. [08:00] fwereade: i think it lives inside container. [08:00] fwereade: i've had some thought as to how container should work [08:01] wrtp, interesting [08:01] fwereade: that's what i was planning to work on this morning [08:02] fwereade: but your thought of "to be able to take the exact same object and run it, or install it, or generate the scripts required to install it" is exactly where i was coming from. [08:02] wrtp, if your position is that Container is a good place for this then I am very happy to stand back and let you get on with it [08:02] fwereade: i was planning to filch a pattern of yours from the unit agent testing, which i rather liked [08:03] wrtp, because my spidey-sense kept saying "use container in cloudinit", but I couldn't figure out how to [08:03] fwereade: i *think* it is [08:03] fwereade: that is my plan [08:03] wrtp, sweet [08:03] fwereade: the plan is for container to be able to generate shell scripts as well as running Deploy [08:03] wrtp, ok, I just need to figure out what I can do that doesn't conflict with you [08:04] fwereade: ok, sorry, i didn't realise you were so deep into this area [08:04] fwereade: the global VarDir was a prelude [08:04] wrtp, because, well, I really want to make the UA just run a Uniter, but the prospect of essentially writing more duped tests inside jujud bugged me enough to go looking for abstractions :) [08:05] wrtp, I think we have actually been for a couple of days, but I think we're coming at the problem from opposite ends so it hasn't been entirely apparent [08:05] fwereade: yeah, i know what you mean. but i think we can write shared tests without shoehorning them all into the same code. [08:06] wrtp, it is extremely reassuring to me that we seem to be in broad agreement about the general problems depsite our differing perspectives :) [08:06] fwereade: agreed [08:07] fwereade: like two ends of an inductive proof coming together... [08:07] wrtp, yeah :) [08:07] fwereade: now we just have to make the terms match [08:09] wrtp, the trouble is that getting a running Uniter is blocked on getting Container to run a unit agent [08:09] fwereade: ok, i'll try and get it out very soon [08:09] wrtp, I guess I can at least just write a really dumb unit agent + test, on top of your VarDir branch [08:10] wrtp, that should be pretty independent [08:10] fwereade: sounds like a reasonable way to go [08:10] wrtp, cool [08:11] wrtp, I look forward to seeing what you do with container [08:13] moin. [08:13] fwereade: here's the likely crackful branch i alluded to earlier. it gives us PA upgrading. but at what cost? what d'ya think? https://codereview.appspot.com/6493101/ [08:18] wrtp, haha, I'll take a look [08:24] wrtp, I think I'm -1 on that, it feels like too much special-casing in state [08:25] wrtp, I think it would be better to either charm the provisioner (which I don;t think is a good use of our time right now) or to build the provisioner-deploying directly into machiner [08:26] wrtp, I thought it was agreed a while ago that we'd be adding a provisioner field to state.machine somehow, and I was expecting to use that (in the absence of a nice charmy way to do it) [08:27] wrtp, (and *that* then makes me think that, hell, the MA itself should probably just run the PA tasks if it's configured to be a provisioner machine, and drop the whole idea of a separate agent [08:27] ) [08:28] fwereade: i wanted to go in that kind of direction, but niemeyer thinks that the PA should have its own entity in state [08:28] wrtp, foiled again :( [08:28] fwereade: which implies loads more mechanism [08:28] wrtp, indeed [08:29] fwereade: which i'm really reluctant to do, because it's actually *identical* to what Unit does [08:29] fwereade: except there's no charm for an agent [08:29] fwereade: hence my AgentService [08:30] wrtp, yeah, I understand, I just think it's basically inferior to a `provisioner bool` field in state.Machien [08:30] wrtp, or at least the underlying state if not that type [08:30] fwereade: except it's much more flexible than that [08:30] fwereade: it means PAs are independent of MAs [08:31] fwereade: ... well... [08:31] fwereade: an MA must start a PA, as with any unit [08:31] (non-subordinate unit) [08:31] wrtp, I'm not sure -- a separate provisioning state entity would be, but I'm suspicious that the AgentService thing feels unhelpfully cross-cutting [08:32] fwereade: you're probably right. [08:32] fwereade: the other thing it offers is the capability to add any new agent of our choice for free. [08:32] fwereade: *currently* we only have three agent types, but that may well change. [08:33] fwereade: i would be happy to just add a bool to a Machine for now. [08:33] wrtp, sure, I'm just not convinced that we can predict the circumstances that might lead us to change well enough to call it right [08:33] wrtp, that would be my choice, indeed, but I guess it's niemeyer's call [08:34] fwereade: i'm not sure i'll ever show him this branch [08:34] wrtp, but I would like it most if we were able to drop the conecpt of a PA entirely, I really don't see why the MA shouldn't run those tasks if so configured [08:35] fwereade: i agree. it seems fine for a machine-level task. [08:35] fwereade: ah, there is one issue [08:35] wrtp, ah bother, there's usually something ;p [08:36] fwereade: we won't give PA-like authority to all MAs [08:36] fwereade: but given that everything has all authority currently, i'm not sure it's a problem for the time being [08:37] wrtp, I don't *think* that's a serious problem... I presume the magic secure API layer will be able to grant/revoke appropriately [08:37] wrtp, yeah, exactly [08:37] fwereade: yeah [08:38] wrtp, ok, well, I think we know what we're doing, I wish you luck :) [08:38] fwereade: thanks! toi aussi [08:45] fwereade: i'm thinking along these kinds of lines for the container API: http://paste.ubuntu.com/1196267/ [08:46] wrtp, +-0, I'm not sure I know enough to judge yet [08:47] wrtp, I think I'll need to see some actual use :) [08:47] fwereade: ok. i think that gives enough for use by both cloudinit and agents, but we'll see. [09:47] wrtp, fwiw, it crosses my mind that agent commands should probably start their Run methods with `a.Conf.JujuDir = ctx.AbsPath(a.Conf.JujuDir)`, even if that's frequently a no-op [09:48] fwereade: interesting point. [09:48] fwereade: or should that happen where something passes the jujudir to something that might change directory? [09:49] wrtp, I *think* that we should never be depending on working directory internally anyway [09:50] wrtp, and I also think it's largely moot because we'll basically always be passing absolute paths anyway, but I think it will be more technically correct and mildly convenient for testing [09:51] wrtp, (it's feeling very hard to write this code because I forsee it changing significantly, but I'm still not quite sure in what direction :)) [09:51] fwereade: yeah, maybe. i don't really have a feel for how --juju-dir might be used in practice [09:52] wrtp, IMO it should just always be passed explicitly [09:52] fwereade: that sounds reasonable [09:52] fwereade: so we should make it a required flag? [09:52] fwereade: well, tbh defaulting to /var/lib/juju seems ok too [09:53] fwereade: we could just give an error if the path was *not* absolute. [09:53] wrtp, I'm inclined to just drop the defaults -- if anyone's ever running itby hand i think we want it to be explicit, and it doesn't cost us much to explicitly set it when generating upstart scripts [09:53] fwereade: but AbsPath seems like a reasonable thing to do too [09:53] fwereade: indeed [09:54] wrtp, all it means is that you *can* run it with a relative path and have it do the right thing in all situations [09:55] wrtp, although, maybe it won't be able to find the tools necessarily [09:55] fwereade: true. i'm just wondering when we'd ever actually want to run an agent from the command line [09:55] fwereade: it doesn't need to find the tools [09:55] wrtp, when things are weird and we're trying to figure out what is going on :) [09:55] wrtp, unit agent does [09:56] fwereade: good point. [09:56] fwereade: i think if things are weird, it's easy for us to pass an absolute pathname :-) [09:57] wrtp, yeah, maybe just requiring abs is the right way to go [09:58] wrtp, cheers [09:58] fwereade: np [10:32] Aram: morning [10:32] hello there. [11:30] fwereade: i'm looking at the upstart package and wondering whether it might be best folded into container [11:30] fwereade: it's juju specific, and the actual code is fairly trivial. [11:31] fwereade: and in doing so, i wondered: is there any time we actually care about Service.{Start,Remove} idempotency? [11:49] wrtp, hmmm, +0.9 to folding it into container [11:50] wrtp, not sure about start/remove but it's not like it's heavily used, so probably not [11:50] fwereade: thanks [12:17] wrtp, is there anything I should have done to induce a charm in a dummy env to be downloadable? [12:17] fwereade: i don't *think* so. it should just work, assuming it's pushing to storage. [12:18] wrtp, ah, why might it not push to storage? do I have to tell it to expect puts/gets? [12:18] fwereade: i don't think so [12:19] fwereade: without seeing what you're doing, i'm not sure i can help much [12:20] wrtp, I'm doing http://paste.ubuntu.com/1196552/ and expecting that a uniter will be able to download the result [12:20] wrtp, Get http://127.0.0.1:38234/dummyenv/private/local_3a_series_2f_dummy-1: dial tcp 127.0.0.1:38234: connection refused [12:21] wrtp, I can investigate myself, I'm just hoping for a shortcut to enlightenment, don't spend time on it :) [12:22] fwereade: i think it *should* work. [12:22] wrtp, cool, that is useful data :) [12:22] wrtp, cheers [12:23] fwereade: i *think* the juju deploy tests are testing this case [12:23] fwereade: perhaps you're doing a Reset by accident? [12:24] wrtp, hmm, I will poke around, that sounds quite plausible [12:24] wrtp, cheers [12:26] wrtp, bah, I trashed pkg and now it works [12:26] wrtp, well it fails differently actually but in a much more scrutable way :) [12:26] fwereade: good. i wonder what went on there [12:27] wrtp, no idea, but I have found "trash pkg" to be a useful troubleshooting step every so often [12:27] wrtp, couldn't remotely say what it's correlated with [12:27] fwereade: i almost never do that. [12:27] fwereade: i wonder how your setup differs [12:27] wrtp, it's probably related to my bloody-minded insistence on keeping separate source dirs and swapping them around [12:27] fwereade: lol [12:28] fwereade: i'm impressed you manage to do that [12:28] wrtp, meh, it fits my brain better and it costs my fingers little [12:28] fwereade: ah, yes, i understand why you have the problems now [12:29] wrtp, I presume something is checking for newer-than [12:29] fwereade: you're moving source directories, but none of the source files change mtime [12:29] wrtp, indeed [12:29] fwereade: yeah. [12:29] wrtp, the amazing thing honestly is that it works so well so much of the time ;p [12:29] fwereade: you should trash the pkg directory each time you change source dirs [12:29] fwereade: or touch all the .go files :-) [12:30] wrtp, yeah, it's really just that actual adverse consequences from failing to do so are surprisingly rare, and so I sometimes forget :/ [12:31] fwereade: fair enough. [12:34] wrtp, separate question: is a JujuConnSuite meant to be already bootstrapped? I thought it was but can't see where it's done [12:34] fwereade: it is, i believe [12:35] fwereade: search for Bootstrap on juju/testing/conn.go [12:35] s/on/in/ [12:36] * fwereade suspects he searched for Bots instead of Boots :/ [12:36] wrtp, thanks [12:36] fwereade: we don't have bots yet :-) [12:36] wrtp, for some reason "botstrap" seems to be one of my muscle-memory typos [12:48] wrtp: do you understand the purpose of this function? https://codereview.appspot.com/6503086/diff/7002/mstate/watcher/watcher.go#newcode168 [12:48] Aram: yeah [12:48] Aram: it's for using in tests [12:48] hmm. [12:49] Aram: so that we can have shorter timeouts when waiting for nothing to happen [12:49] ok, but then we don't need to export it publicly? [12:49] Aram: no, it's for any tests that use watchers [12:49] it can be in export_test.go [12:50] Aram: the idea is that you can change something in the state, call Sync, then you know that the watcher will have triggered any sends that might happen [12:50] Aram: no it can't [12:50] Aram: that would be ok if we only wanted to use in tests of the watcher package itself [12:58] Aram: i can't say i'm enormously keen on the idea, tbh. i'm not sure what particular advantage you get from having the done channels. [12:59] niemeyer: yo! [12:59] wrtp: interesting, I thought the trick of exporting something for a pkg_test package from a pkg package _test file worked for every package used in a test, not only for pkg_test. [12:59] Aram: no indeed not [12:59] Hello! [12:59] niemeyer, heyhey [12:59] hi. [13:01] * wrtp gets a bite of lunch [13:04] Aram: Just reproposed the branch [13:04] Aram: I think I've fixed the spurious error with Sync [13:07] Okay, I'm starting my morning by implementing a watcher, probably the EnvironConfig one, to get an idea if the infrastructure is working well for real in an end-to-end case, and will push that for review [13:07] After that I'm back in review mode, perhaps for the rest of the week [13:08] niemeyer: I'm running the test in a loop. [13:08] Aram: Cool [13:08] Aram: Any more errors? [13:08] nothing yet [13:08] Aram: Superb [13:09] Aram: It was a race.. the test is asserting very defined behavior, and with StartSync() we can actually move on without the watcher having done anything === wrtp is now known as rogpeppe [14:52] niemeyer, I am becoming fretful about how service configs change when charms are upgraded... is this something we've already thought about in detail, that I've missed? [14:54] fwereade: Hmm [14:54] fwereade: I'm afraid to not know the context [14:55] niemeyer, well, just that units running an old version will not necessarily be able to properly understand a new config, and vice versa [14:56] niemeyer, this may just be a matter of "write your service configs carefully" [14:56] fwereade: No, you're right, I don't think we have given the problem proper consideration yet [14:57]