[02:29] davecheney, if you are bored and listless at any time, you might enjoy taking a look at https://codereview.appspot.com/6460110 [02:29] davecheney, which has a real live uniter inside :) [02:31] fwereade_: will do [02:36] fwereade_: impressive [06:40] morning. [06:44] howdy [10:48] so [10:48] when is the release meeting? [10:49] in 70 minutes? [10:49] Aram: tomorrow at 9pm AEST I thought [10:50] mramm: is that correct ? [10:53] Aram, davechen1y: tomorrow after the regular weekly [10:53] I see. [10:54] Aram: Just accept the mail invitation and your calendar will show it. [10:54] too much trouble to regenerate a new password or two factor auth or whatever we use now. [10:54] * Aram hates this crap [10:56] Aram: It's a simple google account. Once setup and ok. I like it all networked, so I always see all our appointments and my private one in my desktop appl as well as on my phone. [10:57] I've setup it once, but for some reason it doesn't remember my password so I have to enter it all the time. [10:57] and because it's a random password I don't remember it. [10:59] Aram: That's why I don't have random passwords. [11:00] yeah, but for some reason I can only use the canonical provided password, couldn't change it. [11:00] Aram: The calendar is a google accout and it can be changed, once you switched to this account. [11:02] Aram: And random pwds are always hard to remember and don't interest brute force attacs. So I choose long ones with stupid sentences like TheQuickBrownFoxJumpsOverTheLazyDog. [11:02] Aram: But only more stupid. ;) [11:57] thats strange: when NOT closing a watcher channel, the according test for the closed watcher fails, so far ok. but when I add the closing a test for unexpected changes fails, that's executed BEFORE the watcher is stopped (and the channel closed). *dunno* [11:58] TheMue, sounds like a bug to me :) [12:00] fwereade_: yeah, but what kind of bug? it's in the SRW. your added tests like assertNoChange(), which mostly work fine. [12:00] fwereade_: funnily the fail happens after the w.Stop(), but the test is befor. [12:00] TheMue, ServiceRelationsWatcher? [12:00] oh, have to break for a moment, the technician for our heating is there. [12:00] fwereade_: yes [12:01] TheMue, I can take a look if yu give me a pointer to the branch [12:02] TheMue, but I guess what it *sounds* like is some actual error is killing the watcher at an unhelpful time... is that possible [12:15] fwereade_: I changed almost nothing in your test, only regarding Kill() and Die(). but the watcher now works differently. i'll paste it. [12:16] fwereade_: http://pastebin.ubuntu.com/1157107/ line 56 fails [12:16] fwereade_: without closing the change channel line 69 fails [12:18] TheMue, ok, so it's line 56 that gets an unexpectedly closed channel; does that not surely indicate that something has suffered some sort of error caused by the relation removal? [12:19] TheMue, try heavy logging in the watcher to figure out why it dies? [12:20] fwereade_: line 56 get's no closed channel, it receives an unexpected change of "nothing" [12:20] TheMue, if the watcher isn't dying I think there's definitely something weird going on [12:20] TheMue, then that just sounds like the watchers is sending something it shouldn't [12:21] fwereade_: i already logged around the sending (there's only one point), but it isn't explicity called [12:21] TheMue, ok, for us to receive, either we are sending or we are closing, right? [12:22] TheMue, if you have verified we are not sending, surely we are closing, which implies that something is killing the watcher [12:22] fwereade_: sorry, don't understand [12:22] fwereade_: ah, now it gets more clear [12:22] TheMue, ok, the problem is that the test receievs an unwanted event on the changes chan, yeah? [12:22] fwereade_: then i already have an idea, one moment [12:23] TheMue, defer a log of w.tomb.Err just before you start the loop maybe? [12:25] TheMue, fwiw I suspect something is manipulating a relation after it's removed [12:26] fwereade_: one moment please, test is running [12:28] fwereade_: hehe, it passes [12:30] fwereade_: it has indeed been a wrong handling of a dying relation life watcher. i signaled the SRWs tomb to kill itself [12:30] fwereade_: instead only doing so in case of an error [12:30] TheMue, ha, bad luck [12:31] fwereade_: so i can propose the new SRW now [12:31] fwereade_: it behaves a bit different than you wanted it to do [12:32] * fwereade_ raises an eyebrow [12:32] TheMue, would you hit the high points please? [12:32] fwereade_: as i get the events of alive/dying/dead by individual life watchers you don't get them bundled [12:33] TheMue, as long as it's the same event, but it just happens to have a single change at a time, that is just fine, so long as we still get a bundle for the first event [12:33] fwereade_: the SRW can't know when the last event will be received, so it just sent what it gets. [12:33] TheMue, I don't mind individual item events in any except the first event [12:34] TheMue, is the first event getting the proper state? [12:34] fwereade_: the life watcher sends a first proper event, but individual [12:35] fwereade_: so if the watcher starts and 5 relations are active you'll get 5 'alive' events [12:35] TheMue, so when I do <-s.WatchRelations().Changes(), I get something different to when I call s.Relations()? [12:35] TheMue, that can't work, sorry [12:35] fwereade_: see line 74ff [12:36] TheMue, I will think about it a bit, but offhand I'm not sure how I can make that work [12:36] TheMue, I would strongly favour something that worked like presence.ChildrenWatcher [12:38] fwereade_: the problem would have been the same in the SRW. i would have to store the startup state, ignore the alive events for all those of the startup state and then contine sending those which are not part of the startup [12:39] TheMue, that sounds sensible to me, what's the issue with it? [12:40] fwereade_: the SRW is so beautiful clean now, it would make it more complex. :D [12:41] TheMue, it doesn't matter how simple it is if it doesn't do what we need ;p [12:41] fwereade_: just define your requirement new *lol* [12:41] TheMue, I will think on how I can work with those events but I'm pretty strongly -1 on implementing just this watcher completely differently from all the others [12:42] fwereade_: sorry, it ISN'T completely different, absolutely not [12:42] TheMue, initial event? [12:42] TheMue, that seems like a pretty vast semantic change to me [12:43] fwereade_: a change yes, but the word completely is too strong [12:43] TheMue, well, all other watchers send an initial event that represents the state, and then send diffs [12:44] TheMue, this sends a sequence of initial events representing the state, and at someunknowable point starts sending diffs [12:44] fwereade_: This one also sends an initial even, only sadly per relation [12:44] TheMue, and how is a client meant to know when they've consumed that initial state? [12:45] fwereade_: but i understand your reason and will change it [12:45] * fwereade_ is relieved :) [12:45] TheMue, if you look at RelationUnitsWatcher you should see a similar model in place [12:47] TheMue, rather than what you described earlier, I would suggest: ignore initial state except to start a watcher for each; consume the initial events on each of those watchers, and collate those into the initial event; send the watches over to their own goroutines for further processing [12:48] TheMue, sorry, s/send the watches over to their own goroutines/start a goroutine for each watch/ [12:48] TheMue, sane? [12:48] fwereade_: that's already running, but the first part sound's good [12:48] fwereade_: consuming the initial events. [12:49] TheMue, have a look at RUW -- it's undeniably more complex but I think it still works out pretty clean [12:51] well, I went back to my room for a nap at 5pm after the pycon sprints before dinner [12:52] and just woke up with a fever, sore throat, sinus problems, etc [12:52] I thought I was just tired from the jetlag [12:52] * Aram is too hungry for the time of the day. [13:14] Morning! [13:14] niemeyer, heyhey [13:14] mramm, ouch :( [13:15] niemeyer: morning [13:15] fwereade_: yea I hate being sick while traveling [13:15] mramm, it sucks :( [13:17] I personally hate being sick. Period. :-) [13:22] niemeyer: morning [13:23] fwereade_: another question: think of service s1 having relations r1 and r2, both are alive [13:24] fwereade_: i get the SRW of s1, and now wait for the first events of r1 and r2. i get r1 as alive and now wait for r2. but during this time i get a r1 as dying [13:24] fwereade_: so after reciving alive from r2 i send r1 as dying and r2 as alive, ok? [13:25] TheMue, if you get a second event on one of the it should be happening on its own goroutine [13:26] fwereade_: yes, it will, and the srw collects it [13:26] TheMue, surely it shouldn't collect it until it has finished handling the initial state? [13:26] fwereade_: otherwise i would send you the dying as a single event before the inital event is out (while waiting for the last life watcher) [13:27] fwereade_: please rephrase [13:27] TheMue, surely you should use an intrnal channel to marshal the subgoroutine events onto the main goroutine [13:27] fwereade_: i do so, yes [13:28] TheMue, surely therefore you would not even pick them up for sending until you'd finished the initial event creation on the main goroutine [13:28] fwereade_: and here i talk about getting 2 or more events of r1 before r2 (which is also part of the initial state) sent its first life [13:29] TheMue, why is it relevant that you got more than one event? maybe you did, and the sub-goroutine is trying to send it to the main one, but the main one won't pick it up until it hits its main select again [13:29] fwereade_: i am in the main select loop [13:29] TheMue, have you looked at what RUW does? [13:30] TheMue, most of what I say is attempts to helpfully translate that into english :) [13:31] TheMue, but, ok, you've just got an initial event that indicates that N relations exist [13:31] fwereade_: may i describe how it works now (before i turn everything around)? [13:32] TheMue, surely you go through each of those relations, consuming event and starting goroutine, and send the collected initial state, all before you leave the current case? [13:32] TheMue, please do :) [13:32] TheMue, your knowledge will be better than my guesses :) [13:33] fwereade_: when the srw starts it watches the topology, as before, and retrieves the relations of the service. for each relation a lifewatcher in an own goroutine is started. [13:33] TheMue, ah, OK, to get what we need I think you need to start all the watchers on the main goroutine to consume the initial events [13:33] fwereade_: the all write their events back to the srw in one channel, where the srw collects then and sends the changes to the receiver of Changes() [13:34] TheMue, only once you have consumed the first event do you start a new goroutine to handle the subsequent ones [13:34] fwereade_: yes, that my question, please wait with writing [13:34] TheMue, sorry [13:35] fwereade_: now, as you wrote above, i want to consume the first events of those life watchers before sending it combined as the initial state to you [13:36] fwereade_: the question is now, that it may happen, that the state of r1 changes before r2 is received. [13:36] TheMue, yes, that can happen if you don't start the life watches on the main goroutine [13:36] fwereade_: but because i'm in the same loop i receive it. [13:37] TheMue, right, so... don't do it that way :) [13:38] fwereade_: ah, ok, ic, while setting the life watchers i directly wait blocking on their first event [13:38] TheMue, yeah, sorry unclear [13:40] fwereade_: eh, sorry, i wanted to say that, when setting up the life watcher for a relation I immediatelly wait for its first event. so after setting up the first set i've got all first life states. [13:41] fwereade_: that will be sent then and afterwards the single events of the life watchers [13:46] fwereade_, TheMue: I was following but didn't want to interrupt, but seems like we've reached agreement now [13:46] fwereade_, TheMue: I'm curious about what's the case there, given we'll be doing things slightly differently soon [13:47] niemeyer, sorry, I'm not sure what you're asking [13:48] niemeyer: we talked about the service relation watcher, which will handles the lifecycles of realations [13:48] fwereade_: I'm wondering what the underlying issue being debated above is about [13:48] niemeyer: and william and i discussed about how to get a propper initial state and then only diffs [13:49] fwereade_: Trying to load mind state since we have to do that stuff in mstate [13:49] niemeyer, just "should a watcher always send initial state as a single event" -- I feel that is part of the definition of a watcher [13:49] niemeyer: now it's more clear to me, thx to williams feedback [13:49] fwereade_: Okay [13:49] fwereade_: That's easy, thanks :) [13:49] niemeyer, cool [13:49] niemeyer, fwereade_ +1 [13:49] ;) [13:50] TheMue, cheers :) [13:50] niemeyer, btw, I don't know if you saw, there's a Uniter in review [13:51] fwereade_: Yeah, brilliant [13:51] fwereade_: How're things going there, btw? Are you happy with it? [13:51] fwereade_: Or with progress, in general [13:51] niemeyer, it's basically the whole unit agent state machine, excluding relations, because basically everything interacts with everything else and looks very different if you try to implement bits without other bits [13:52] niemeyer, honestly I think it's awesome, it does an awful lot in very few lines of code [13:52] fwereade_: Woot! [13:52] niemeyer, and the tests are pretty cool too [13:52] niemeyer, I fully expect to have missed something, but I was feeling pretty damn happy when I proposed it :) [13:52] niemeyer, uniter and modes are each under 250 lines [13:53] fwereade_: Sweeeet [13:53] niemeyer, I'm sad that I didn't et it proposed on friday, but happy that I think relation support will be easy to add [13:54] fwereade_: I get all excited just thinking that we're about to run our first charm [13:54] niemeyer, yeah :D [13:54] niemeyer, and, heh, the name Uniter feels like a surprisingly good joke, it really does pull almost everything together in one place [13:55] fwereade_: Hah.. hadn't thought of it from that perspective [14:04] fwereade_: Yes, Sir, Mr Reade, Sir. Initial state is up and running. Thx again. ;) [14:04] TheMue, awesome, tyvm [14:18] Aram: ping [14:18] pong [14:18] Aram: Yo [14:18] hi [14:18] Aram: I've just sent you a review on confignode [14:18] Aram: It's definitely awesome [14:18] looking [14:19] Aram: There are a couple of high-levels we should understand, just in terms of how we'll be porting these concepts [14:19] Aram: The logic itself is very cool [14:21] 1 min to answer IN the CL. [14:23] Aram: Okay, although I'd like to follow up here if that's fine with you [14:24] done [14:25] Aram: Okay, given those comments, LGTM on the branch going in as-is [14:25] Aram: We can tweak it further in follow ups [14:25] thanks :). [14:26] Aram: Re. the path, I'm wondering if we should be using context-specific collections [14:26] that might be a good kidea. [14:26] idea [14:27] Aram: On the version, once I finish the txn package (which is almost there, btw), this should become automatic, so it's cool to live as-is for now [14:27] s/live/leave [14:27] cool. [14:29] Aram: Spent a good while polishing it further over the weekend.. I haven't seen a single error with the current logic so far.. last night I've finally started on the handling of inserts/removes, and I *think* the approach is sane and easy to get right.. [14:29] yeah, I've seen your G+ post. so basically, after insert and delete it should be good to go? [14:29] Aram: It's not yet functional, but I suspect I can do that in a couple of hours tonight [14:29] Aram: Yep [14:37] Hmm.. I have to do some errands today, like banking and expense-reporting [14:39] * Aram needs to buy new backpack, laptop, and some small laptop carying thingy. [14:51] Aram: What's the plan for the new laptop? Anything interesting these days? [14:51] fwereade_: Nice one on the downloader refactoring [14:51] niemeyer, cheers, was rather pleasing [14:52] meh, I wanted to buy the new ThinkPad X1 carbon, it's a great machine, 14'' screen in 13'' chassis and weight, 1600x900 matte screen etc. [14:52] fwereade_: Isn't it awesome playing with lego when you have the right blocks? :-) [14:52] but at home I want to use two external monitors and this new thinkpad only has a stupid windows onlu usb (?!?) dock. [14:52] no drivers for linux at all. [14:52] niemeyer, yeah, it really is -- this last week or so has been a salutary lesson in the benefits of building bottom-up [14:53] niemeyer, not sure we could have done it like that starting from scratch, but that's maybe just me :) [14:53] If I didn't need to use two external monitors, I'd buy the new ThinkPad X1 carbon. [14:53] mainly because of screen. [14:54] Aram: Ouch [14:54] fwereade_: Yeah, we should definitely sit down for a couple of hours over the next sprint evaluating what we did [14:54] niemeyer, postmortems++ [14:55] fwereade_: I tend to agree that to do what we're doing right now, we need crystal-clear vision of where we're going [14:56] niemeyer, ...but when we *do*, it's awesome :) [15:24] fwereade_: +1000 x 0.001 [15:24] :-) [15:24] fwereade_: You have a LGTM! That branch is great [15:24] niemeyer, haha :) [15:24] niemeyer, sweet [15:24] niemeyer, thank you :D [15:57] fwereade_: Another review on uniter-support-tweaks [15:57] fwereade_: I'm afraid this one isn't so positive.. please let me know when you want to talk about it [15:58] niemeyer, you're probably right on most of it given a quick scan, and it's definitely worth a chat -- but are you about to have lunch? [15:59] niemeyer, because I am rather hungry myself, but will be back shortly [15:59] fwereade_: Indeed I am.. if there's anything quick you wanna talk before that, I'd be happy to unblock you [15:59] fwereade_: Otherwise I'll be back soon after lunch + bank [15:59] niemeyer, nah, I'm good, just need to work up the mental resolve to stop coding and go get some fod ;) [15:59] fwereade_: Superb, sounds like a plan :) === marrusl_ is now known as marrusl [16:55] robbiew: Sorry, I know you just mentioned it in the meeting, but I'm not sure: do we have the consistency call today? [16:57] lol [16:57] niemeyer: are you trolling me [16:57] no call [16:58] robbiew: Well, that's getting pretty consistent I guess :) [16:58] indeed [17:13] niemeyer, ping, I have a bit of pushback on HookContext.CmdGetter in the review [17:17] niemeyer, although honestly I could live without any or all of the things I propose -- they just make a few bits of the Uniter arguably neater [17:27] fwereade_: Agreed, I can also live without any code, although it wouldn't be so fun I believe :-) [17:28] fwereade_: That's not justification to get things in blindly, though.. CmdGetter feels like a pretty awkward piece at first sight [17:28] fwereade_: I'm happy to talk, though.. have been wrong a non-trivial number of times [17:29] niemeyer, I think there's an underlying point we haven't discussed: [17:29] niemeyer, IMO it's simplest right now to start a jujuc server for each hook and stop it when the hook's done [17:30] niemeyer, I know that this approach will probably not live forever, but I don't think it's better *now* to implement a persistent server which chooses contexts, without having any contexts to choose, than it is to defer implementing that until we know exactly what we want [17:31] niemeyer, assuming you're ok with this, then we can debate CmdGetter, although ...actually, I'm happy to concede it's unnecessary [17:32] niemeyer, if you're not ok with the above, we should probably talk more :) [17:32] fwereade_: I don't have a strong feeling either way, but I'd appreciate understanding what it means in practice [17:33] fwereade_: My view is that it has a context to choose, and we just haven't implemented the bit that makes it choose different contexts [17:34] fwereade_: If we already have that neatly in place, I'm curious about why we'd go backwards [17:38] fwereade_: Although, maybe we have the model entirely wrong [17:38] niemeyer, ok, but the actual mechanics of context choice become more complicated if there's ever more than one possibility: that is, if I have a server running all the time, I need to keep track of the current hook context in the Uniter, and to worry about what happens if someone happens to call into it when I'm not expecting it [17:39] fwereade_: Well, that's the very reason why we have a context id, right? [17:39] niemeyer, I'm not saying they're intractable, just that they're not something we need yet when we can just run single-context servers [17:39] fwereade_: Yeah, this is interesting.. let's explore that for a bit [17:39] niemeyer, sure, the concept of a context id has been flowed through from the start because we're confident we'll want to do something interesting with it one day [17:39] fwereade_: We had that conversation in the sprint where we agreed to do something entirely different from what we originally envisioned [17:40] fwereade_: Which can actually affect this in a good way [17:40] niemeyer, cool [17:40] fwereade_: We had that thing with.. how was it.. "juju-context foo.sh" or something? [17:41] niemeyer, that was the suggestion, I think, yes [17:41] fwereade_: Okay.. and IIRC we agreed that it was going to be serialized too [17:41] fwereade_: Or am I on crack? [17:41] niemeyer, although I'm thinking juju-run might actually be nicer [17:41] fwereade_: Yeah, that sounds better [17:42] niemeyer, no, I am entirely comfortable with this idea [17:42] fwereade_: Okay, so.. hmm [17:42] fwereade_: If it's all serialized, you're on the right track.. why do we have multiple contexts [17:43] fwereade_: There's still a role for the context id as originally incepted, though, I think [17:43] niemeyer, validation, primarily, I think -- commands will act slightly differently in a relation context, for example [17:43] fwereade_: Bingo! [17:43] fwereade_: What if we replaced it by a nonce? [17:44] niemeyer, expand please, not sure how that's superior [17:44] fwereade_: What we want is to prevent stuff from randomly accessing the juju server [17:44] fwereade_: When they've reportedly left the context in which they were running [17:45] fwereade_: If I juju-run my-stuff.sh, nothing from my-stuff.sh should be touching the server after juju-run returns [17:45] niemeyer, yes, exactly [17:45] fwereade_: A nonce is a good way to implement the mechanics for that, without separating into multiple contexts on the server side [17:46] fwereade_: When we start to run something, we provide a nonce to the environment, when we stop running it, we reset the nonce [17:46] fwereade_: Any requests that arrive with the wrong nonce are denied [17:47] niemeyer, ok, so, just as we have been doing, but I like this perspective, it doesn't have to be tied to the context [17:47] fwereade_: yeah, pretty much as we've been doing, except we lose the concept of the context id, because the client can't "select the context" [17:48] fwereade_: It's just able or unable to talk to "the one context" [17:48] niemeyer, we can keep the same context forever, and just take a little bit of care with concurrency [17:48] fwereade_: Right [17:49] niemeyer, ok, that sounds awesome [17:49] niemeyer, I have a plan, then: I drop evenything except the ExpandTo change, and rework the tests as you suggest [17:51] niemeyer, then you can take a look at the real-live-uniter branch and decide whether the command running is horrifying enough to demand that fix as a prereq, or whether you can live with it a a followup [17:51] niemeyer, sane? [17:52] fwereade_: +1 [17:52] fwereade_: Is CmdGetter there? [17:52] fwereade_: Or, will it remain there? [17:53] niemeyer, nah, I'll write the same tiny little closure inline, unless you're willing to put up with it as a temporary measure [17:53] fwereade_: I think it could go away entirely [17:53] fwereade_: but certainly willing to review for more context [17:54] fwereade_: This closure is doing if a != b { error } [17:54] niemeyer, yeah [18:04] niemeyer, oh, yes, we never did talk properly about charm.Status [18:05] niemeyer, from the questions you ask, I think I maybe got the name wrong [18:06] niemeyer, in a sense there are only 3 statuses -- missing, changing, installed -- but there are multiple reasons to be changing the charm, and I think the Uniter wants to keep track of them somehow [18:06] niemeyer, it seemed sensible to store them all in one type, for the convenience of the package's only client, which does care about those distinctions [18:07] fwereade_: I'm just wondering why [18:07] niemeyer, why the distinction between the two upgrade types? [18:07] niemeyer, because forced ones don't want to run hooks while already in a potential error state [18:08] fwereade_: Why the forced status is necessary [18:08] fwereade_: In the charm upgrading directory [18:08] fwereade_: If we have a charm directory upgrade in progress, it is in progress and we must complete it.. it doesn't matter how we got there [18:08] fwereade_: retry is unrelated to that.. force means "upgrade even if in error" [18:08] niemeyer, hum, I was not involved in the original implementation there, but AIUI it was precisely so that users *could* upgrade hte charm while in the middle of a hook error without otherwise perturbing the unit's state [18:09] niemeyer, ok, and given that the process could die N times in the course of the upgrade, when we come up in the middle of an operation, I think it is useful to know exactly what sort of operation it is [18:09] fwereade_: It's an upgrade operation., ? [18:10] niemeyer, right, but it wants to finish it idfferently depending on whether or not it's forced [18:10] niemeyer, python elevated the forcedness as the most important thing -- ie a forced upgrade would never run hooks [18:10] fwereade_: Exactly.. so why do we need both an "upgradeing" and an "upgrading forced" status in a charm directory? [18:11] niemeyer, because how the upgrade operation is completed differers, depending on that status [18:11] fwereade_: Why? [18:12] niemeyer, I was not privy to the original reasoning, but I would not be surprised if the word "twisted" didn't feature in the answer somewhere [18:12] fwereade_: Okay, so can we drop that status? [18:13] niemeyer, as long as you're happy to make a forced upgrade in started state act exactly like a normal upgrade [18:13] niemeyer, I would be very happy with that [18:14] fwereade_: I don't understand how it's any different, to be honest.. a "forced" upgrade indicates whether or not we're willing to start the upgrade even if in an error status [18:14] niemeyer, and then any upgrade operation will, when it completes, run hooks or not based purely on existing hook error [18:14] fwereade_: After that decision, we're running the upgrade.. it's already in progress and we can't stop it, no matter how we got to start running it [18:14] niemeyer, ok, sure, but when we finish I think we still do not want to run the upgrade-charm hook if another hook has an unresolved error [18:15] fwereade_: Once the upgrade completes, there's a decision to be made regarding running hooks or not, and that's not related to whether we said "force" or not, or is it? [18:16] niemeyer, the only reason it is related is because it is implemented like that in python [18:16] niemeyer, and I doubt anyone is depending heavily on the ability to upgrade charms without hooks in started status [18:16] fwereade_: Okay, I can't see the reasoning, but maybe we'll find out while trying to drop it.. if we don't, even better [18:16] niemeyer, so if you're ok with the behaviour change then I am ++happy to drop it [18:17] fwereade_: Okay, just to be clear, what's the behavior change again? [18:17] niemeyer, whether or not to run an upgrade-charm hook at the end of a forced charm upgrade in started status [18:17] niemeyer, I think the change is a good one [18:17] niemeyer, ie that forcedness should be entirely ignored in started mode [18:18] fwereade_: Huh.. that's what I expected it meant, so I guess the change from whatever was being done is a reasonable one [18:18] niemeyer, this also lets me -wip that branch before you take a look, the install/upgrade will get *much* nicer as a result [18:19] fwereade_: It's like "rm -f foo".. if foo would be removed without -f.. well.. that's fine :) [18:20] niemeyer, yeah, the meaning of force should not have been overloaded in the first place :) [18:22] niemeyer, ok, I think I'm going to rework my branches [18:23] niemeyer, the one we've been discussing will lose UpgradingForced, will get the ExpandTo test fixes, and will also get a reworking of the state unit/service charm API that I ended up needing in the followup [18:24] fwereade_: Cool [18:24] niemeyer, when that's ready I'll repropose and gtw on real-live-uniter with nicer charm changes [18:24] fwereade_: Superb, cheers [18:40] niemeyer, ok, now I'm looking at the state charm stuff on its own, I'm thinking some more [18:41] * niemeyer listens [18:41] niemeyer, do we have any reason to have CharmURL or SetCharmURL at all? I don't think we really do [18:42] fwereade_: Go on? [18:42] well, they're barely used, and they allow for inconsistency that setting and getting actual *state.Charms do not [18:42] fwereade_: I'm not yet following [18:43] fwereade_: I don't want to state the obvious, but they're the way we download charms [18:43] fwereade_: What am I missing? [18:43] niemeyer, ok, SetCharmURL(something-made-up) is possible; SetCharm(some-actual-charm) is not [18:44] niemeyer, ok, that made no sense [18:45] niemeyer, I'll try again: it should not be possible to set a unit's, or a service's, charm URL to a charm that is not in state [18:46] niemeyer, the two ways of accomplishing this are (1) check against state whenever we set a charm url or (2) allow setting of charm urls purely by setting actual *state.Charm values, which should be guaranteed to represent real charms [18:46] niemeyer, I favour (2), it feels cleaner [18:46] fwereade_: I don't understand how that's possible [18:47] fwereade_: Charm URLs are signed so that we can access a private bucket [18:47] niemeyer, no, charm urls -- *charm.URL -- not bundle urls [18:47] niemeyer, units and services identify their charm by charm URL [18:47] fwereade_: Oh, oka [18:47] y [18:48] niemeyer, when we come to download it, we get the bundle url from the state charm itself -- that's fine, I think it's immutable [18:48] fwereade_: So what's the thing on the left-hand side of SetCharmURL again? :-) [18:48] * niemeyer looks at the code [18:48] niemeyer, unit/service [18:50] fwereade_: Okay, sorry, I think I get it now [18:51] fwereade_: Are you suggesting we replace SetCharmURL and CharmURL by SetCharm and Charm? [18:51] niemeyer, yes [18:51] fwereade_: +1 [18:52] niemeyer, sweet -- and, while I'm there, I kinda want to change state.Unit.serviceName to state.unit.service -- I think we've always got a service available when we create a unit, and it then means that we can actually get to all the relevant state given just a *Unit [18:52] niemeyer, rather than having to store units and services together in a bunch of places [18:52] niemeyer, well, 2 places, anyway [18:53] niemeyer, (the point is that we can then have (*Unit)Service() *Service [18:53] ) [18:53] fwereade_: If it's cleaner, certainly +1 [18:53] fwereade_: Well, hmm [18:54] fwereade_: I guess that sounds sane.. I'm just wondering about how that will impact performance and the caching stuff [18:54] niemeyer, only positively, I *hope* [18:55] fwereade_: Yeah, probably [18:55] fwereade_: These changes should take place on mstate at the same time [18:55] fwereade_: I think we're already covering that part of the API there [18:55] Aram? [18:56] niemeyer, I think the Unit methods are missing, but that's just less code to delete :) [18:56] fwereade_: Yeah, and it'll probably be easier to add to mstate than state even, and it'll also give you a nice idea of how it maps [18:57] niemeyer, yeah, I have yet toget myhands dirty in there [18:58] niemeyer, sorry, more things to get agreement on ahead of time, actually [18:58] niemeyer, (1) units should start out without charms set [18:59] niemeyer, (2) the charm state file will actually want a charm URL *and* a status, because I won't be able to trust the charm URL I set in state (what if I'm restarted on a different machine?) [19:00] niemeyer, I think that's it [19:07] fwereade_: Hmm [19:07] fwereade_: (1) seems rather curious [19:09] niemeyer, well, given (2), we could ignore (1), but I guess the question is what exactly a unit's charm URL *means* [19:09] fwereade_: Is there any doubts about that? [19:09] fwereade_: It's the charm the unit is supposed to run? [19:09] niemeyer, if it's the charm the unit *should* have, it puts us in a position of potentially writing 10k nodes in order to upgrade the charm for a big service [19:10] niemeyer, if it's the charm the unit *does* have, IMO it should absolutely start out empty and only be set once the charm is installed [19:10] fwereade_: Yes, assuming we want to upgrade all at once [19:10] niemeyer, well, I guess, we do write those 10k *anyway* in order to actually tell them to upgrade [19:10] fwereade_: Yeah [19:10] niemeyer, if we're telling them what to upgrade to that's only twice as much work :) [19:10] fwereade_: It's also the easy part of the job, assuming we're upgrading 10k units :) [19:10] niemeyer, haha [19:11] fwereade_: Although.. I can easily see us doing better in that area [19:11] fwereade_: The thinking around all of that is to enable partial upgrades [19:12] fwereade_: Which I'm assuming *will* happen.. nobody will be upgrading 10k nodes at once to a new version if they have a real production system [19:12] fwereade_: Well, I should fix that.. nobody *should* do that.. they probably will :) [19:13]