| fwereade_ | davecheney, if you are bored and listless at any time, you might enjoy taking a look at https://codereview.appspot.com/6460110 | 02:29 |
|---|---|---|
| fwereade_ | davecheney, which has a real live uniter inside :) | 02:29 |
| davecheney | fwereade_: will do | 02:31 |
| mramm | fwereade_: impressive | 02:36 |
| Aram | morning. | 06:40 |
| davecheney | howdy | 06:44 |
| Aram | so | 10:48 |
| Aram | when is the release meeting? | 10:48 |
| Aram | in 70 minutes? | 10:49 |
| davechen1y | Aram: tomorrow at 9pm AEST I thought | 10:49 |
| davechen1y | mramm: is that correct ? | 10:50 |
| TheMue | Aram, davechen1y: tomorrow after the regular weekly | 10:53 |
| Aram | I see. | 10:53 |
| TheMue | Aram: Just accept the mail invitation and your calendar will show it. | 10:54 |
| Aram | too much trouble to regenerate a new password or two factor auth or whatever we use now. | 10:54 |
| * Aram hates this crap | 10:54 | |
| TheMue | 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:56 |
| Aram | 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 |
| Aram | and because it's a random password I don't remember it. | 10:57 |
| TheMue | Aram: That's why I don't have random passwords. | 10:59 |
| Aram | yeah, but for some reason I can only use the canonical provided password, couldn't change it. | 11:00 |
| TheMue | Aram: The calendar is a google accout and it can be changed, once you switched to this account. | 11:00 |
| TheMue | 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 |
| TheMue | Aram: But only more stupid. ;) | 11:02 |
| TheMue | 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:57 |
| fwereade_ | TheMue, sounds like a bug to me :) | 11:58 |
| TheMue | fwereade_: yeah, but what kind of bug? it's in the SRW. your added tests like assertNoChange(), which mostly work fine. | 12:00 |
| TheMue | fwereade_: funnily the fail happens after the w.Stop(), but the test is befor. | 12:00 |
| fwereade_ | TheMue, ServiceRelationsWatcher? | 12:00 |
| TheMue | oh, have to break for a moment, the technician for our heating is there. | 12:00 |
| TheMue | fwereade_: yes | 12:00 |
| fwereade_ | TheMue, I can take a look if yu give me a pointer to the branch | 12:01 |
| fwereade_ | TheMue, but I guess what it *sounds* like is some actual error is killing the watcher at an unhelpful time... is that possible | 12:02 |
| TheMue | fwereade_: I changed almost nothing in your test, only regarding Kill() and Die(). but the watcher now works differently. i'll paste it. | 12:15 |
| TheMue | fwereade_: http://pastebin.ubuntu.com/1157107/ line 56 fails | 12:16 |
| TheMue | fwereade_: without closing the change channel line 69 fails | 12:16 |
| fwereade_ | 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:18 |
| fwereade_ | TheMue, try heavy logging in the watcher to figure out why it dies? | 12:19 |
| TheMue | fwereade_: line 56 get's no closed channel, it receives an unexpected change of "nothing" | 12:20 |
| fwereade_ | TheMue, if the watcher isn't dying I think there's definitely something weird going on | 12:20 |
| fwereade_ | TheMue, then that just sounds like the watchers is sending something it shouldn't | 12:20 |
| TheMue | fwereade_: i already logged around the sending (there's only one point), but it isn't explicity called | 12:21 |
| fwereade_ | TheMue, ok, for us to receive, either we are sending or we are closing, right? | 12:21 |
| fwereade_ | TheMue, if you have verified we are not sending, surely we are closing, which implies that something is killing the watcher | 12:22 |
| TheMue | fwereade_: sorry, don't understand | 12:22 |
| TheMue | fwereade_: ah, now it gets more clear | 12:22 |
| fwereade_ | TheMue, ok, the problem is that the test receievs an unwanted event on the changes chan, yeah? | 12:22 |
| TheMue | fwereade_: then i already have an idea, one moment | 12:22 |
| fwereade_ | TheMue, defer a log of w.tomb.Err just before you start the loop maybe? | 12:23 |
| fwereade_ | TheMue, fwiw I suspect something is manipulating a relation after it's removed | 12:25 |
| TheMue | fwereade_: one moment please, test is running | 12:26 |
| TheMue | fwereade_: hehe, it passes | 12:28 |
| TheMue | fwereade_: it has indeed been a wrong handling of a dying relation life watcher. i signaled the SRWs tomb to kill itself | 12:30 |
| TheMue | fwereade_: instead only doing so in case of an error | 12:30 |
| fwereade_ | TheMue, ha, bad luck | 12:30 |
| TheMue | fwereade_: so i can propose the new SRW now | 12:31 |
| TheMue | fwereade_: it behaves a bit different than you wanted it to do | 12:31 |
| * fwereade_ raises an eyebrow | 12:32 | |
| fwereade_ | TheMue, would you hit the high points please? | 12:32 |
| TheMue | fwereade_: as i get the events of alive/dying/dead by individual life watchers you don't get them bundled | 12:32 |
| fwereade_ | 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 |
| TheMue | fwereade_: the SRW can't know when the last event will be received, so it just sent what it gets. | 12:33 |
| fwereade_ | TheMue, I don't mind individual item events in any except the first event | 12:33 |
| fwereade_ | TheMue, is the first event getting the proper state? | 12:34 |
| TheMue | fwereade_: the life watcher sends a first proper event, but individual | 12:34 |
| TheMue | fwereade_: so if the watcher starts and 5 relations are active you'll get 5 'alive' events | 12:35 |
| fwereade_ | TheMue, so when I do <-s.WatchRelations().Changes(), I get something different to when I call s.Relations()? | 12:35 |
| fwereade_ | TheMue, that can't work, sorry | 12:35 |
| TheMue | fwereade_: see line 74ff | 12:35 |
| fwereade_ | TheMue, I will think about it a bit, but offhand I'm not sure how I can make that work | 12:36 |
| fwereade_ | TheMue, I would strongly favour something that worked like presence.ChildrenWatcher | 12:36 |
| TheMue | 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:38 |
| fwereade_ | TheMue, that sounds sensible to me, what's the issue with it? | 12:39 |
| TheMue | fwereade_: the SRW is so beautiful clean now, it would make it more complex. :D | 12:40 |
| fwereade_ | TheMue, it doesn't matter how simple it is if it doesn't do what we need ;p | 12:41 |
| TheMue | fwereade_: just define your requirement new *lol* | 12:41 |
| fwereade_ | 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:41 |
| TheMue | fwereade_: sorry, it ISN'T completely different, absolutely not | 12:42 |
| fwereade_ | TheMue, initial event? | 12:42 |
| fwereade_ | TheMue, that seems like a pretty vast semantic change to me | 12:42 |
| TheMue | fwereade_: a change yes, but the word completely is too strong | 12:43 |
| fwereade_ | TheMue, well, all other watchers send an initial event that represents the state, and then send diffs | 12:43 |
| fwereade_ | TheMue, this sends a sequence of initial events representing the state, and at someunknowable point starts sending diffs | 12:44 |
| TheMue | fwereade_: This one also sends an initial even, only sadly per relation | 12:44 |
| fwereade_ | TheMue, and how is a client meant to know when they've consumed that initial state? | 12:44 |
| TheMue | fwereade_: but i understand your reason and will change it | 12:45 |
| * fwereade_ is relieved :) | 12:45 | |
| fwereade_ | TheMue, if you look at RelationUnitsWatcher you should see a similar model in place | 12:45 |
| fwereade_ | 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:47 |
| fwereade_ | TheMue, sorry, s/send the watches over to their own goroutines/start a goroutine for each watch/ | 12:48 |
| fwereade_ | TheMue, sane? | 12:48 |
| TheMue | fwereade_: that's already running, but the first part sound's good | 12:48 |
| TheMue | fwereade_: consuming the initial events. | 12:48 |
| fwereade_ | TheMue, have a look at RUW -- it's undeniably more complex but I think it still works out pretty clean | 12:49 |
| mramm | well, I went back to my room for a nap at 5pm after the pycon sprints before dinner | 12:51 |
| mramm | and just woke up with a fever, sore throat, sinus problems, etc | 12:52 |
| mramm | I thought I was just tired from the jetlag | 12:52 |
| * Aram is too hungry for the time of the day. | 12:52 | |
| niemeyer | Morning! | 13:14 |
| fwereade_ | niemeyer, heyhey | 13:14 |
| fwereade_ | mramm, ouch :( | 13:14 |
| mramm | niemeyer: morning | 13:15 |
| mramm | fwereade_: yea I hate being sick while traveling | 13:15 |
| fwereade_ | mramm, it sucks :( | 13:15 |
| niemeyer | I personally hate being sick. Period. :-) | 13:17 |
| TheMue | niemeyer: morning | 13:22 |
| TheMue | fwereade_: another question: think of service s1 having relations r1 and r2, both are alive | 13:23 |
| TheMue | 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 |
| TheMue | fwereade_: so after reciving alive from r2 i send r1 as dying and r2 as alive, ok? | 13:24 |
| fwereade_ | TheMue, if you get a second event on one of the it should be happening on its own goroutine | 13:25 |
| TheMue | fwereade_: yes, it will, and the srw collects it | 13:26 |
| fwereade_ | TheMue, surely it shouldn't collect it until it has finished handling the initial state? | 13:26 |
| TheMue | 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:26 |
| TheMue | fwereade_: please rephrase | 13:27 |
| fwereade_ | TheMue, surely you should use an intrnal channel to marshal the subgoroutine events onto the main goroutine | 13:27 |
| TheMue | fwereade_: i do so, yes | 13:27 |
| fwereade_ | 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 |
| TheMue | 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:28 |
| fwereade_ | 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 |
| TheMue | fwereade_: i am in the main select loop | 13:29 |
| fwereade_ | TheMue, have you looked at what RUW does? | 13:29 |
| fwereade_ | TheMue, most of what I say is attempts to helpfully translate that into english :) | 13:30 |
| fwereade_ | TheMue, but, ok, you've just got an initial event that indicates that N relations exist | 13:31 |
| TheMue | fwereade_: may i describe how it works now (before i turn everything around)? | 13:31 |
| fwereade_ | 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 |
| fwereade_ | TheMue, please do :) | 13:32 |
| fwereade_ | TheMue, your knowledge will be better than my guesses :) | 13:32 |
| TheMue | 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 |
| fwereade_ | 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 |
| TheMue | 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:33 |
| fwereade_ | TheMue, only once you have consumed the first event do you start a new goroutine to handle the subsequent ones | 13:34 |
| TheMue | fwereade_: yes, that my question, please wait with writing | 13:34 |
| fwereade_ | TheMue, sorry | 13:34 |
| TheMue | 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:35 |
| TheMue | fwereade_: the question is now, that it may happen, that the state of r1 changes before r2 is received. | 13:36 |
| fwereade_ | TheMue, yes, that can happen if you don't start the life watches on the main goroutine | 13:36 |
| TheMue | fwereade_: but because i'm in the same loop i receive it. | 13:36 |
| fwereade_ | TheMue, right, so... don't do it that way :) | 13:37 |
| TheMue | fwereade_: ah, ok, ic, while setting the life watchers i directly wait blocking on their first event | 13:38 |
| fwereade_ | TheMue, yeah, sorry unclear | 13:38 |
| TheMue | 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:40 |
| TheMue | fwereade_: that will be sent then and afterwards the single events of the life watchers | 13:41 |
| niemeyer | fwereade_, TheMue: I was following but didn't want to interrupt, but seems like we've reached agreement now | 13:46 |
| niemeyer | fwereade_, TheMue: I'm curious about what's the case there, given we'll be doing things slightly differently soon | 13:46 |
| fwereade_ | niemeyer, sorry, I'm not sure what you're asking | 13:47 |
| TheMue | niemeyer: we talked about the service relation watcher, which will handles the lifecycles of realations | 13:48 |
| niemeyer | fwereade_: I'm wondering what the underlying issue being debated above is about | 13:48 |
| TheMue | niemeyer: and william and i discussed about how to get a propper initial state and then only diffs | 13:48 |
| niemeyer | fwereade_: Trying to load mind state since we have to do that stuff in mstate | 13:49 |
| fwereade_ | 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 |
| TheMue | niemeyer: now it's more clear to me, thx to williams feedback | 13:49 |
| niemeyer | fwereade_: Okay | 13:49 |
| niemeyer | fwereade_: That's easy, thanks :) | 13:49 |
| fwereade_ | niemeyer, cool | 13:49 |
| TheMue | niemeyer, fwereade_ +1 | 13:49 |
| TheMue | ;) | 13:49 |
| fwereade_ | TheMue, cheers :) | 13:50 |
| fwereade_ | niemeyer, btw, I don't know if you saw, there's a Uniter in review | 13:50 |
| niemeyer | fwereade_: Yeah, brilliant | 13:51 |
| niemeyer | fwereade_: How're things going there, btw? Are you happy with it? | 13:51 |
| niemeyer | fwereade_: Or with progress, in general | 13:51 |
| fwereade_ | 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:51 |
| fwereade_ | niemeyer, honestly I think it's awesome, it does an awful lot in very few lines of code | 13:52 |
| niemeyer | fwereade_: Woot! | 13:52 |
| fwereade_ | niemeyer, and the tests are pretty cool too | 13:52 |
| fwereade_ | niemeyer, I fully expect to have missed something, but I was feeling pretty damn happy when I proposed it :) | 13:52 |
| fwereade_ | niemeyer, uniter and modes are each under 250 lines | 13:52 |
| niemeyer | fwereade_: Sweeeet | 13:53 |
| fwereade_ | 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:53 |
| niemeyer | fwereade_: I get all excited just thinking that we're about to run our first charm | 13:54 |
| fwereade_ | niemeyer, yeah :D | 13:54 |
| fwereade_ | niemeyer, and, heh, the name Uniter feels like a surprisingly good joke, it really does pull almost everything together in one place | 13:54 |
| niemeyer | fwereade_: Hah.. hadn't thought of it from that perspective | 13:55 |
| TheMue | fwereade_: Yes, Sir, Mr Reade, Sir. Initial state is up and running. Thx again. ;) | 14:04 |
| fwereade_ | TheMue, awesome, tyvm | 14:04 |
| niemeyer | Aram: ping | 14:18 |
| Aram | pong | 14:18 |
| niemeyer | Aram: Yo | 14:18 |
| Aram | hi | 14:18 |
| niemeyer | Aram: I've just sent you a review on confignode | 14:18 |
| niemeyer | Aram: It's definitely awesome | 14:18 |
| Aram | looking | 14:18 |
| niemeyer | Aram: There are a couple of high-levels we should understand, just in terms of how we'll be porting these concepts | 14:19 |
| niemeyer | Aram: The logic itself is very cool | 14:19 |
| Aram | 1 min to answer IN the CL. | 14:21 |
| niemeyer | Aram: Okay, although I'd like to follow up here if that's fine with you | 14:23 |
| Aram | done | 14:24 |
| niemeyer | Aram: Okay, given those comments, LGTM on the branch going in as-is | 14:25 |
| niemeyer | Aram: We can tweak it further in follow ups | 14:25 |
| Aram | thanks :). | 14:25 |
| niemeyer | Aram: Re. the path, I'm wondering if we should be using context-specific collections | 14:26 |
| Aram | that might be a good kidea. | 14:26 |
| Aram | idea | 14:26 |
| niemeyer | 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 |
| niemeyer | s/live/leave | 14:27 |
| Aram | cool. | 14:27 |
| niemeyer | 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 |
| Aram | yeah, I've seen your G+ post. so basically, after insert and delete it should be good to go? | 14:29 |
| niemeyer | Aram: It's not yet functional, but I suspect I can do that in a couple of hours tonight | 14:29 |
| niemeyer | Aram: Yep | 14:29 |
| niemeyer | Hmm.. I have to do some errands today, like banking and expense-reporting | 14:37 |
| * Aram needs to buy new backpack, laptop, and some small laptop carying thingy. | 14:39 | |
| niemeyer | Aram: What's the plan for the new laptop? Anything interesting these days? | 14:51 |
| niemeyer | fwereade_: Nice one on the downloader refactoring | 14:51 |
| fwereade_ | niemeyer, cheers, was rather pleasing | 14:51 |
| Aram | 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 |
| niemeyer | fwereade_: Isn't it awesome playing with lego when you have the right blocks? :-) | 14:52 |
| Aram | but at home I want to use two external monitors and this new thinkpad only has a stupid windows onlu usb (?!?) dock. | 14:52 |
| Aram | no drivers for linux at all. | 14:52 |
| fwereade_ | niemeyer, yeah, it really is -- this last week or so has been a salutary lesson in the benefits of building bottom-up | 14:52 |
| fwereade_ | niemeyer, not sure we could have done it like that starting from scratch, but that's maybe just me :) | 14:53 |
| Aram | If I didn't need to use two external monitors, I'd buy the new ThinkPad X1 carbon. | 14:53 |
| Aram | mainly because of screen. | 14:53 |
| niemeyer | Aram: Ouch | 14:54 |
| niemeyer | fwereade_: Yeah, we should definitely sit down for a couple of hours over the next sprint evaluating what we did | 14:54 |
| fwereade_ | niemeyer, postmortems++ | 14:54 |
| niemeyer | 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:55 |
| fwereade_ | niemeyer, ...but when we *do*, it's awesome :) | 14:56 |
| niemeyer | fwereade_: +1000 x 0.001 | 15:24 |
| niemeyer | :-) | 15:24 |
| niemeyer | fwereade_: You have a LGTM! That branch is great | 15:24 |
| fwereade_ | niemeyer, haha :) | 15:24 |
| fwereade_ | niemeyer, sweet | 15:24 |
| fwereade_ | niemeyer, thank you :D | 15:24 |
| niemeyer | fwereade_: Another review on uniter-support-tweaks | 15:57 |
| niemeyer | fwereade_: I'm afraid this one isn't so positive.. please let me know when you want to talk about it | 15:57 |
| fwereade_ | 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:58 |
| fwereade_ | niemeyer, because I am rather hungry myself, but will be back shortly | 15:59 |
| niemeyer | fwereade_: Indeed I am.. if there's anything quick you wanna talk before that, I'd be happy to unblock you | 15:59 |
| niemeyer | fwereade_: Otherwise I'll be back soon after lunch + bank | 15:59 |
| fwereade_ | niemeyer, nah, I'm good, just need to work up the mental resolve to stop coding and go get some fod ;) | 15:59 |
| niemeyer | fwereade_: Superb, sounds like a plan :) | 15:59 |
| === marrusl_ is now known as marrusl | ||
| niemeyer | robbiew: Sorry, I know you just mentioned it in the meeting, but I'm not sure: do we have the consistency call today? | 16:55 |
| robbiew | lol | 16:57 |
| robbiew | niemeyer: are you trolling me | 16:57 |
| robbiew | no call | 16:57 |
| niemeyer | robbiew: Well, that's getting pretty consistent I guess :) | 16:58 |
| robbiew | indeed | 16:58 |
| fwereade_ | niemeyer, ping, I have a bit of pushback on HookContext.CmdGetter in the review | 17:13 |
| fwereade_ | 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:17 |
| niemeyer | fwereade_: Agreed, I can also live without any code, although it wouldn't be so fun I believe :-) | 17:27 |
| niemeyer | fwereade_: That's not justification to get things in blindly, though.. CmdGetter feels like a pretty awkward piece at first sight | 17:28 |
| niemeyer | fwereade_: I'm happy to talk, though.. have been wrong a non-trivial number of times | 17:28 |
| fwereade_ | niemeyer, I think there's an underlying point we haven't discussed: | 17:29 |
| fwereade_ | niemeyer, IMO it's simplest right now to start a jujuc server for each hook and stop it when the hook's done | 17:29 |
| fwereade_ | 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:30 |
| fwereade_ | niemeyer, assuming you're ok with this, then we can debate CmdGetter, although ...actually, I'm happy to concede it's unnecessary | 17:31 |
| fwereade_ | niemeyer, if you're not ok with the above, we should probably talk more :) | 17:32 |
| niemeyer | fwereade_: I don't have a strong feeling either way, but I'd appreciate understanding what it means in practice | 17:32 |
| niemeyer | 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:33 |
| niemeyer | fwereade_: If we already have that neatly in place, I'm curious about why we'd go backwards | 17:34 |
| niemeyer | fwereade_: Although, maybe we have the model entirely wrong | 17:38 |
| fwereade_ | 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:38 |
| niemeyer | fwereade_: Well, that's the very reason why we have a context id, right? | 17:39 |
| fwereade_ | 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 |
| niemeyer | fwereade_: Yeah, this is interesting.. let's explore that for a bit | 17:39 |
| fwereade_ | 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 |
| niemeyer | fwereade_: We had that conversation in the sprint where we agreed to do something entirely different from what we originally envisioned | 17:39 |
| niemeyer | fwereade_: Which can actually affect this in a good way | 17:40 |
| fwereade_ | niemeyer, cool | 17:40 |
| niemeyer | fwereade_: We had that thing with.. how was it.. "juju-context foo.sh" or something? | 17:40 |
| fwereade_ | niemeyer, that was the suggestion, I think, yes | 17:41 |
| niemeyer | fwereade_: Okay.. and IIRC we agreed that it was going to be serialized too | 17:41 |
| niemeyer | fwereade_: Or am I on crack? | 17:41 |
| fwereade_ | niemeyer, although I'm thinking juju-run might actually be nicer | 17:41 |
| niemeyer | fwereade_: Yeah, that sounds better | 17:41 |
| fwereade_ | niemeyer, no, I am entirely comfortable with this idea | 17:42 |
| niemeyer | fwereade_: Okay, so.. hmm | 17:42 |
| niemeyer | fwereade_: If it's all serialized, you're on the right track.. why do we have multiple contexts | 17:42 |
| niemeyer | fwereade_: There's still a role for the context id as originally incepted, though, I think | 17:43 |
| fwereade_ | niemeyer, validation, primarily, I think -- commands will act slightly differently in a relation context, for example | 17:43 |
| niemeyer | fwereade_: Bingo! | 17:43 |
| niemeyer | fwereade_: What if we replaced it by a nonce? | 17:43 |
| fwereade_ | niemeyer, expand please, not sure how that's superior | 17:44 |
| niemeyer | fwereade_: What we want is to prevent stuff from randomly accessing the juju server | 17:44 |
| niemeyer | fwereade_: When they've reportedly left the context in which they were running | 17:44 |
| niemeyer | fwereade_: If I juju-run my-stuff.sh, nothing from my-stuff.sh should be touching the server after juju-run returns | 17:45 |
| fwereade_ | niemeyer, yes, exactly | 17:45 |
| niemeyer | fwereade_: A nonce is a good way to implement the mechanics for that, without separating into multiple contexts on the server side | 17:45 |
| niemeyer | 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 |
| niemeyer | fwereade_: Any requests that arrive with the wrong nonce are denied | 17:46 |
| fwereade_ | 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 |
| niemeyer | 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:47 |
| niemeyer | fwereade_: It's just able or unable to talk to "the one context" | 17:48 |
| fwereade_ | niemeyer, we can keep the same context forever, and just take a little bit of care with concurrency | 17:48 |
| niemeyer | fwereade_: Right | 17:48 |
| fwereade_ | niemeyer, ok, that sounds awesome | 17:49 |
| fwereade_ | niemeyer, I have a plan, then: I drop evenything except the ExpandTo change, and rework the tests as you suggest | 17:49 |
| fwereade_ | 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 |
| fwereade_ | niemeyer, sane? | 17:51 |
| niemeyer | fwereade_: +1 | 17:52 |
| niemeyer | fwereade_: Is CmdGetter there? | 17:52 |
| niemeyer | fwereade_: Or, will it remain there? | 17:52 |
| fwereade_ | 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 |
| niemeyer | fwereade_: I think it could go away entirely | 17:53 |
| niemeyer | fwereade_: but certainly willing to review for more context | 17:53 |
| niemeyer | fwereade_: This closure is doing if a != b { error } | 17:54 |
| fwereade_ | niemeyer, yeah | 17:54 |
| fwereade_ | niemeyer, oh, yes, we never did talk properly about charm.Status | 18:04 |
| fwereade_ | niemeyer, from the questions you ask, I think I maybe got the name wrong | 18:05 |
| fwereade_ | 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 |
| fwereade_ | 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:06 |
| niemeyer | fwereade_: I'm just wondering why | 18:07 |
| fwereade_ | niemeyer, why the distinction between the two upgrade types? | 18:07 |
| fwereade_ | niemeyer, because forced ones don't want to run hooks while already in a potential error state | 18:07 |
| niemeyer | fwereade_: Why the forced status is necessary | 18:08 |
| niemeyer | fwereade_: In the charm upgrading directory | 18:08 |
| niemeyer | 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 |
| niemeyer | fwereade_: retry is unrelated to that.. force means "upgrade even if in error" | 18:08 |
| fwereade_ | 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:08 |
| fwereade_ | 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 |
| niemeyer | fwereade_: It's an upgrade operation., ? | 18:09 |
| fwereade_ | niemeyer, right, but it wants to finish it idfferently depending on whether or not it's forced | 18:10 |
| fwereade_ | niemeyer, python elevated the forcedness as the most important thing -- ie a forced upgrade would never run hooks | 18:10 |
| niemeyer | fwereade_: Exactly.. so why do we need both an "upgradeing" and an "upgrading forced" status in a charm directory? | 18:10 |
| fwereade_ | niemeyer, because how the upgrade operation is completed differers, depending on that status | 18:11 |
| niemeyer | fwereade_: Why? | 18:11 |
| fwereade_ | 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 |
| niemeyer | fwereade_: Okay, so can we drop that status? | 18:12 |
| fwereade_ | niemeyer, as long as you're happy to make a forced upgrade in started state act exactly like a normal upgrade | 18:13 |
| fwereade_ | niemeyer, I would be very happy with that | 18:13 |
| niemeyer | 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 |
| fwereade_ | niemeyer, and then any upgrade operation will, when it completes, run hooks or not based purely on existing hook error | 18:14 |
| niemeyer | 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 |
| fwereade_ | 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:14 |
| niemeyer | 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:15 |
| fwereade_ | niemeyer, the only reason it is related is because it is implemented like that in python | 18:16 |
| fwereade_ | niemeyer, and I doubt anyone is depending heavily on the ability to upgrade charms without hooks in started status | 18:16 |
| niemeyer | 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 |
| fwereade_ | niemeyer, so if you're ok with the behaviour change then I am ++happy to drop it | 18:16 |
| niemeyer | fwereade_: Okay, just to be clear, what's the behavior change again? | 18:17 |
| fwereade_ | niemeyer, whether or not to run an upgrade-charm hook at the end of a forced charm upgrade in started status | 18:17 |
| fwereade_ | niemeyer, I think the change is a good one | 18:17 |
| fwereade_ | niemeyer, ie that forcedness should be entirely ignored in started mode | 18:17 |
| niemeyer | 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 |
| fwereade_ | niemeyer, this also lets me -wip that branch before you take a look, the install/upgrade will get *much* nicer as a result | 18:18 |
| niemeyer | fwereade_: It's like "rm -f foo".. if foo would be removed without -f.. well.. that's fine :) | 18:19 |
| fwereade_ | niemeyer, yeah, the meaning of force should not have been overloaded in the first place :) | 18:20 |
| fwereade_ | niemeyer, ok, I think I'm going to rework my branches | 18:22 |
| fwereade_ | 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:23 |
| niemeyer | fwereade_: Cool | 18:24 |
| fwereade_ | niemeyer, when that's ready I'll repropose and gtw on real-live-uniter with nicer charm changes | 18:24 |
| niemeyer | fwereade_: Superb, cheers | 18:24 |
| fwereade_ | niemeyer, ok, now I'm looking at the state charm stuff on its own, I'm thinking some more | 18:40 |
| * niemeyer listens | 18:41 | |
| fwereade_ | niemeyer, do we have any reason to have CharmURL or SetCharmURL at all? I don't think we really do | 18:41 |
| niemeyer | fwereade_: Go on? | 18:42 |
| fwereade_ | well, they're barely used, and they allow for inconsistency that setting and getting actual *state.Charms do not | 18:42 |
| niemeyer | fwereade_: I'm not yet following | 18:42 |
| niemeyer | fwereade_: I don't want to state the obvious, but they're the way we download charms | 18:43 |
| niemeyer | fwereade_: What am I missing? | 18:43 |
| fwereade_ | niemeyer, ok, SetCharmURL(something-made-up) is possible; SetCharm(some-actual-charm) is not | 18:43 |
| fwereade_ | niemeyer, ok, that made no sense | 18:44 |
| fwereade_ | 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:45 |
| fwereade_ | 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 |
| fwereade_ | niemeyer, I favour (2), it feels cleaner | 18:46 |
| niemeyer | fwereade_: I don't understand how that's possible | 18:46 |
| niemeyer | fwereade_: Charm URLs are signed so that we can access a private bucket | 18:47 |
| fwereade_ | niemeyer, no, charm urls -- *charm.URL -- not bundle urls | 18:47 |
| fwereade_ | niemeyer, units and services identify their charm by charm URL | 18:47 |
| niemeyer | fwereade_: Oh, oka | 18:47 |
| niemeyer | y | 18:47 |
| fwereade_ | 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 |
| niemeyer | fwereade_: So what's the thing on the left-hand side of SetCharmURL again? :-) | 18:48 |
| * niemeyer looks at the code | 18:48 | |
| fwereade_ | niemeyer, unit/service | 18:48 |
| niemeyer | fwereade_: Okay, sorry, I think I get it now | 18:50 |
| niemeyer | fwereade_: Are you suggesting we replace SetCharmURL and CharmURL by SetCharm and Charm? | 18:51 |
| fwereade_ | niemeyer, yes | 18:51 |
| niemeyer | fwereade_: +1 | 18:51 |
| fwereade_ | 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 |
| fwereade_ | niemeyer, rather than having to store units and services together in a bunch of places | 18:52 |
| fwereade_ | niemeyer, well, 2 places, anyway | 18:52 |
| fwereade_ | niemeyer, (the point is that we can then have (*Unit)Service() *Service | 18:53 |
| fwereade_ | ) | 18:53 |
| niemeyer | fwereade_: If it's cleaner, certainly +1 | 18:53 |
| niemeyer | fwereade_: Well, hmm | 18:53 |
| niemeyer | fwereade_: I guess that sounds sane.. I'm just wondering about how that will impact performance and the caching stuff | 18:54 |
| fwereade_ | niemeyer, only positively, I *hope* | 18:54 |
| niemeyer | fwereade_: Yeah, probably | 18:55 |
| niemeyer | fwereade_: These changes should take place on mstate at the same time | 18:55 |
| niemeyer | fwereade_: I think we're already covering that part of the API there | 18:55 |
| niemeyer | Aram? | 18:55 |
| fwereade_ | niemeyer, I think the Unit methods are missing, but that's just less code to delete :) | 18:56 |
| niemeyer | 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:56 |
| fwereade_ | niemeyer, yeah, I have yet toget myhands dirty in there | 18:57 |
| fwereade_ | niemeyer, sorry, more things to get agreement on ahead of time, actually | 18:58 |
| fwereade_ | niemeyer, (1) units should start out without charms set | 18:58 |
| fwereade_ | 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?) | 18:59 |
| fwereade_ | niemeyer, I think that's it | 19:00 |
| niemeyer | fwereade_: Hmm | 19:07 |
| niemeyer | fwereade_: (1) seems rather curious | 19:07 |
| fwereade_ | niemeyer, well, given (2), we could ignore (1), but I guess the question is what exactly a unit's charm URL *means* | 19:09 |
| niemeyer | fwereade_: Is there any doubts about that? | 19:09 |
| niemeyer | fwereade_: It's the charm the unit is supposed to run? | 19:09 |
| fwereade_ | 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:09 |
| fwereade_ | 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 |
| niemeyer | fwereade_: Yes, assuming we want to upgrade all at once | 19:10 |
| fwereade_ | niemeyer, well, I guess, we do write those 10k *anyway* in order to actually tell them to upgrade | 19:10 |
| niemeyer | fwereade_: Yeah | 19:10 |
| fwereade_ | niemeyer, if we're telling them what to upgrade to that's only twice as much work :) | 19:10 |
| niemeyer | fwereade_: It's also the easy part of the job, assuming we're upgrading 10k units :) | 19:10 |
| fwereade_ | niemeyer, haha | 19:10 |
| niemeyer | fwereade_: Although.. I can easily see us doing better in that area | 19:11 |
| niemeyer | fwereade_: The thinking around all of that is to enable partial upgrades | 19:11 |
| niemeyer | 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 |
| niemeyer | fwereade_: Well, I should fix that.. nobody *should* do that.. they probably will :) | 19:12 |
| fwereade_ | niemeyer, ha | 19:13 |
| fwereade_ | niemeyer, well, ok, this is again a behaviour change, about which I'm a little less certain, because I don't know whether I will be implementing upgrade-charm, and this all needs to tie in nicely there | 19:14 |
| fwereade_ | niemeyer, ATM python depends on (1) the unit's upgrade flag and (2) the service's charm | 19:14 |
| fwereade_ | niemeyer, and it uses the unit's charm in state to determine that it doesn't need an upgrade | 19:14 |
| niemeyer | fwereade_: I quite like what we did in the agent stuff | 19:15 |
| niemeyer | fwereade_: We could mimic it | 19:15 |
| fwereade_ | niemeyer, that last part is crack anyway, really, it should be using local state to know what's already installed | 19:15 |
| fwereade_ | niemeyer, that has crossed my mind | 19:15 |
| niemeyer | fwereade_: We have proposed and current | 19:15 |
| niemeyer | fwereade_: Which means we support both ends | 19:16 |
| niemeyer | fwereade_: That doesn't change what you just said, though | 19:16 |
| niemeyer | fwereade_: Current is set via local state | 19:16 |
| niemeyer | fwereade_: This also means no behavioral change is necessary | 19:16 |
| fwereade_ | niemeyer, ok, yep, it's not an external behaviour change, wrong term -- what I mean is that it's a change that has ramifications beyond my current narrow focus | 19:17 |
| fwereade_ | niemeyer, ie in the implementation of juju upgrade-charm itself | 19:17 |
| fwereade_ | niemeyer, which I will capture in the bug, so np there :) | 19:18 |
| fwereade_ | niemeyer, so: service just has Charm/SetCharm, and the service's charm controls the proposed-charm that new units are created with | 19:18 |
| fwereade_ | niemeyer, unit has proposed-charm and current-charm, which starts out empty | 19:19 |
| fwereade_ | niemeyer, whenever the UA finishes unpacking a charm to disk, it sets current-charm | 19:19 |
| niemeyer | fwereade_: +1 | 19:20 |
| niemeyer | fwereade_: Well, I assume proposed-charm should start out filled | 19:20 |
| fwereade_ | niemeyer, so unit gets Charm/SetCharm, and ProposedCharm, and maybe something else | 19:20 |
| fwereade_ | niemeyer, SetProposedCharm and SetNeedsUpgrade sound like they could profitably be combined | 19:21 |
| niemeyer | fwereade_: As it doesn't make sense to fire a unit if we don't know what it is | 19:21 |
| niemeyer | fwereade_: Agreed | 19:21 |
| fwereade_ | niemeyer, ok, I'm starting to feel that the ripple effects may be pretty significant, because a diff between current and proposed should be enough information to determine that an upgrade should happen | 19:23 |
| niemeyer | fwereade_: Exactly | 19:23 |
| niemeyer | fwereade_: Alternatively, we may dump that half-baked idea | 19:24 |
| niemeyer | fwereade_: Since we're paying cost for nothing so far | 19:24 |
| fwereade_ | niemeyer, I therefore feel like we should stick with the existing upgrade flag approach, because that does contain one valuable bit (force) along with the less valuable bit (upgrade) | 19:24 |
| niemeyer | fwereade_: And say that proposed-charm lives in the service, while current-charm lives in the unit | 19:25 |
| fwereade_ | niemeyer, hey, nice | 19:25 |
| fwereade_ | niemeyer, and that then does mean that the unit starts out empty | 19:25 |
| niemeyer | fwereade_: Right | 19:26 |
| fwereade_ | niemeyer, then we keep the upgrade flag exactly as is | 19:26 |
| fwereade_ | niemeyer, and then in fact we just keep the existing API, but use Charm instead of CharmURL | 19:26 |
| fwereade_ | niemeyer, because really unit.CurrentCharm/service.ProposedCharm don't make a huge amount of sense, while .Charm/.Charm can be sensibly documented | 19:27 |
| niemeyer | fwereade_: I think NeedsUpgrade can go | 19:27 |
| niemeyer | fwereade_: Agreed | 19:27 |
| fwereade_ | niemeyer, just the API method? yeah, nobody needs to read that without watching, I don;t think | 19:28 |
| niemeyer | fwereade_: The whole idea of the flag in the unit, I mean | 19:29 |
| niemeyer | fwereade_: needing upgrade == service.Charm != unit.Charm | 19:29 |
| fwereade_ | niemeyer, hmm... just a moment ago you convinced me that it would be a good idea to keep the possibility of upgrading parts of a service | 19:30 |
| fwereade_ | niemeyer, and we still need somewhere for the force flag to live -- I guess it could be the service, but see above, partial upgrades | 19:30 |
| niemeyer | fwereade_: Agreed, but I've been changing my mind since then.. we're putting brand new logic in place that doesn't scale, for a benefit we don't yet have | 19:32 |
| niemeyer | fwereade_: DOuble loss | 19:32 |
| fwereade_ | niemeyer, well, ok, cool, but also ouch, more infrastructure rework :) | 19:32 |
| niemeyer | fwereade_: Well, you mean the method in Unit? That sounds trivial.. I'd be worry if we had logic in the uniter already | 19:33 |
| niemeyer | worried | 19:33 |
| fwereade_ | niemeyer, I'm talking about the watcher really | 19:34 |
| fwereade_ | niemeyer, I expect it will be trivial, it's just that for some reason it flips my perception of my blockers from "meh, I'll probably finish it tonight" to "omg this will take forever" | 19:35 |
| fwereade_ | niemeyer, which is self-evidently insane, I do not need to be told ;) | 19:35 |
| niemeyer | fwereade_: Ah, yeah, you mean we need a watcher for the service which I suppose we don't yet have | 19:36 |
| fwereade_ | niemeyer, yeah, that's all really | 19:36 |
| niemeyer | fwereade_: A lot of blocks indeed | 19:37 |
| fwereade_ | niemeyer, but still, I'll see what I can do :) | 19:37 |
| niemeyer | fwereade_: I wonder about the force upgrade flag SetCharm(charm, force)? | 19:39 |
| niemeyer | fwereade_: service.Charm() (charm, force)? | 19:39 |
| niemeyer | Well | 19:39 |
| niemeyer | fwereade_: service.Charm() (charm, force, error)? | 19:39 |
| fwereade_ | niemeyer, yeah, I *think* so, but I'm still following it through | 19:40 |
| fwereade_ | niemeyer, hmmm, how do we know when the force flag can be cleared without scanning all units? | 19:42 |
| niemeyer | fwereade_: I don't think we have to clear it | 19:42 |
| niemeyer | fwereade_: (!) | 19:42 |
| fwereade_ | niemeyer, ah, yeah, next upgrade we just set a new charm and force it or not | 19:43 |
| niemeyer | fwereade_: Yep | 19:43 |
| niemeyer | fwereade_: It's awesome that the right path generally shows itself obviously good by the number of things we *don't* have to do | 19:43 |
| fwereade_ | niemeyer, hmm, is it meaningful to unforce the upgrade to a particular charm? | 19:44 |
| fwereade_ | niemeyer, IIRC we have some logic that says you can't unforce an upgrade, but I forget why | 19:44 |
| niemeyer | fwereade_: I actually commented in the review that this was weird, but accepted not to argue until later | 19:44 |
| fwereade_ | niemeyer, ha :) | 19:45 |
| niemeyer | fwereade_: The upgrade operation as "I want to use this charm, with forcing enabled/disabled" feels as good as it gets | 19:45 |
| fwereade_ | niemeyer, sgtm, can we explore the idea a bit more in the context of compatibility? | 19:46 |
| fwereade_ | niemeyer, Service.SetCharm gives us a great point at which to enforce interface compatibility... maybe that's all we need | 19:47 |
| fwereade_ | niemeyer, trying to figure out how much damage a zombie unit can do | 19:48 |
| fwereade_ | niemeyer, I guess the answer is "none" as long as it's operating well enough that it kept its relations alive | 19:48 |
| niemeyer | fwereade_: Hm | 19:49 |
| fwereade_ | niemeyer, because those relations, had they had that unit as a member, would not have dies, and would therefore themselves have blocked the upgrade in the first place | 19:49 |
| niemeyer | fwereade_: I'm afraid I'm a bit out of context | 19:51 |
| niemeyer | fwereade_: I agree re. SetCharm being a nice point | 19:51 |
| fwereade_ | niemeyer, sorry, it feels like it's connected to, er, 1 sec | 19:51 |
| niemeyer | fwereade_: It's actually brilliant, in the sense that we can have a single point efficiently checking for all units | 19:51 |
| niemeyer | fwereade_: I don't know what a zombie unit is, though :) | 19:51 |
| fwereade_ | niemeyer, I mean a unit that comes back up after a long break, which is using a hugely outdated charm, and may even be dying | 19:52 |
| niemeyer | fwereade_: Ok, so what's the issue with it again? | 19:53 |
| fwereade_ | niemeyer, I'm just feeling like we should consider https://bugs.launchpad.net/juju-core/+bug/1032557 in this context before we go too far | 19:53 |
| fwereade_ | niemeyer, but on a reread I don;t think it's relevant | 19:54 |
| fwereade_ | niemeyer, it's just another chunk of ugliness for the uniter, wwhich I'll hopefully be able to keep under control :) | 19:54 |
| niemeyer | fwereade_: The bug sounds a bit awkward in its wording | 19:56 |
| niemeyer | fwereade_: Oh, wait | 19:56 |
| niemeyer | fwereade_: upgrade-charm as in "juju upgrade-charm"? | 19:56 |
| niemeyer | fwereade_: I had the hook name in my mind for whatever reason | 19:56 |
| fwereade_ | niemeyer, ah, sorry :) | 19:57 |
| niemeyer | fwereade_: No, it's my bad | 19:57 |
| fwereade_ | niemeyer, but anyway, I don't think it actually does hurt us here | 19:57 |
| niemeyer | fwereade_: I think we should start by just being conservative.. no changes in established relations | 19:57 |
| niemeyer | fwereade_: We can improve from there, always staying on the safe side | 19:58 |
| fwereade_ | niemeyer, ok, sure, there's still a chunk of ugliness to deal with in the uniter | 19:58 |
| niemeyer | fwereade_: How so? | 19:58 |
| fwereade_ | niemeyer, toactually stay on the safe side we need to make sure we don't join relations that don't exist in the current version of the charm | 19:58 |
| fwereade_ | niemeyer, otherwise we'll get a bunch of hooks "executed" despite their not actually existing | 19:59 |
| niemeyer | fwereade_: Wow.. my mind just spinned for a second.. how's that again? :-) | 19:59 |
| fwereade_ | niemeyer, unit has cs:series/dummy-0, service has cs:series/dummy-1, upgrade is unforced, uniter is in error state | 20:00 |
| fwereade_ | niemeyer, relation is added using an interface that exists in -1, but not in -0 | 20:00 |
| niemeyer | fwereade_: Great, I'm with you now | 20:00 |
| niemeyer | fwereade_: Aha, thanks, gotcha | 20:01 |
| fwereade_ | niemeyer, when the uniter goes into the started state, it... well, really, it just needs to check the upgrade flag before it starts messing with relations | 20:01 |
| fwereade_ | niemeyer, ok, it's easier than I thought :) | 20:01 |
| fwereade_ | niemeyer, yay! | 20:01 |
| niemeyer | fwereade_: Just when I was starting to get worried!? LOL | 20:01 |
| fwereade_ | niemeyer, haha, sorry :) | 20:02 |
| niemeyer | fwereade_: The fact we disallow "correct multi-versioning" actually seems like a great simplification there | 20:03 |
| fwereade_ | niemeyer, yes indeed | 20:03 |
| niemeyer | fwereade_: The unit should either follow along, or be broken | 20:04 |
| niemeyer | fwereade_: Which is not only easy to implement, but also easy to understand | 20:04 |
| fwereade_ | niemeyer, yep, and as soon as it's unbroken it catches up | 20:04 |
| fwereade_ | niemeyer, hum, I have a new scenario | 20:05 |
| niemeyer | fwereade_: Uh oh | 20:05 |
| niemeyer | :-) | 20:05 |
| fwereade_ | niemeyer, unit with v-0 is running a hook which takes a long time; someone upgrades the service and adds a new v-1 relation, such that both watches are ready when the unit comes out | 20:06 |
| fwereade_ | niemeyer, again it's easy, do a quick check of the upgrade flag before going back into the main select | 20:07 |
| niemeyer | fwereade_: Welllll | 20:07 |
| fwereade_ | niemeyer, but the fiddliness factor is growing | 20:07 |
| niemeyer | fwereade_: I think we can be more deterministic than that | 20:08 |
| niemeyer | fwereade_: We know what charm the unit is running.. we know which relations it takes | 20:08 |
| fwereade_ | niemeyer, hey, wait, it's even easier | 20:09 |
| fwereade_ | niemeyer, just *totally ignore* all relations that don;t make sense yet | 20:09 |
| fwereade_ | niemeyer, we'll start new relation watches when pick ourselves up from the upgrade | 20:09 |
| fwereade_ | niemeyer, and it will all shake out then | 20:09 |
| fwereade_ | niemeyer, niiice | 20:09 |
| niemeyer | fwereade_: It doesn't feel so straightforward | 20:10 |
| fwereade_ | niemeyer, oh :( | 20:10 |
| niemeyer | fwereade_: I mean, it is, but you're trying to order two events that happen independently and chaotically | 20:10 |
| niemeyer | fwereade_: Doing: 1) do I have upgrades; 2) do I have new relations | 20:11 |
| niemeyer | fwereade_: Isn't safe | 20:11 |
| niemeyer | fwereade_: Because between 1 and 2, I can very quickly upgrade the charm and add a new relation | 20:11 |
| fwereade_ | niemeyer, agreed, which is why the simpler solution is also *better* -- the only question I need to ask is "does this relation look sane?" and if it doesn't I can just ignore it | 20:12 |
| fwereade_ | niemeyer, trusting that the rest of the system has enough integrity that sooner or later I'll pick up the upgrade that will make that relation make sense | 20:12 |
| niemeyer | fwereade_: Right, and by sane I suppose you mean verifying against the charm that the unit is running ATM | 20:12 |
| fwereade_ | niemeyer, yeah, exactly | 20:12 |
| niemeyer | fwereade_: Cool | 20:12 |
| niemeyer | fwereade_: The only detail then is that we have to "re-watch" after upgrading | 20:13 |
| niemeyer | fwereade_: Since we may have lost events that were responsible for the creation of such relations | 20:13 |
| fwereade_ | niemeyer, yeah, the idea is that upgrading is a spearate mode from starting, which is what actually watches for relations | 20:13 |
| fwereade_ | niemeyer, s/starting/started/ | 20:14 |
| niemeyer | fwereade_: Cool, I haven't yet run through the uniter in detail, so I'll just trust you on that.. it definitely sounds great :-) | 20:14 |
| fwereade_ | niemeyer, once we're in started, we start a whole bunch of watches that only last for the duration of that mode, and relations is one of them | 20:14 |
| niemeyer | fwereade_: Neat | 20:14 |
| fwereade_ | niemeyer, that said, I should -wip it, and I haven't yet | 20:15 |
| fwereade_ | niemeyer, these underlying changes are only going to have good effects at the uniter level :) | 20:16 |
| niemeyer | fwereade_: Yeah, I have a great feeling about those conversations | 20:16 |
| niemeyer | fwereade_: It sounds like we're actually understanding the problems we've been dealing with, and are making good progress towards a fully working system | 20:17 |
| fwereade_ | niemeyer, I am approaching a state of mystical oneness with the Uniter :) | 20:17 |
| fwereade_ | niemeyer, I guess I should go write a spot more code, though, not sure how much the branches will need to be split up | 20:18 |
| niemeyer | fwereade_: That sounds so great.. LOL | 20:19 |
| niemeyer | fwereade_: Yeah, I felt tempted to suggest splitting things up in a few occasions too | 20:19 |
| fwereade_ | niemeyer, I *think* it'll come out surprisingly clean | 20:19 |
| niemeyer | fwereade_: +1 | 20:20 |
| fwereade_ | niemeyer, fixing up the bundle stuff is trivial, though, I'll getthat one out of the way in the first branch before anything else | 20:21 |
| niemeyer | fwereade_: Sounds good.. happy to review in-promptu as well | 20:21 |
| fwereade_ | niemeyer, cool, I ballsed up my sleep patterns over the weekend so I'll be around for a bit, I'll shout if I'm unsure of anything | 20:22 |
| niemeyer | fwereade_: I know how that goes.. :-) I'll have to step out for a moment soon, but I'm also going to be working on the evening, so we can stay in sync | 20:23 |
| fwereade_ | niemeyer, great | 20:23 |
| niemeyer | Okay, biab | 20:54 |
| fwereade_ | niemeyer, ping | 22:18 |
| fwereade_ | niemeyer, I was wondering about the motivation behind the (1) continue-on-error and (2) write-revision-last behaviours of the current charm.ExpandTo | 22:19 |
| niemeyer | fwereade_: Hmm, let me check | 22:20 |
| niemeyer | fwereade_: Can't tell why the heck it continues on error | 22:23 |
| niemeyer | fwereade_: revision-last is an artifact of the revision bumping | 22:24 |
| fwereade_ | niemeyer, ahhh, ofc -- and we actually use that on bundles? | 22:24 |
| fwereade_ | niemeyer, it doesn't look like we do | 22:26 |
| niemeyer | fwereade_: Probably not.. it was part of the Charm interface at some point, which is how it ended up there | 22:26 |
| fwereade_ | niemeyer, and it feels like there's somehow something odd about being allowed to, actually | 22:26 |
| fwereade_ | niemeyer, can I drop it please? | 22:26 |
| niemeyer | fwereade_: Sure | 22:27 |
| fwereade_ | niemeyer, sweet, thanks | 22:27 |
| fwereade_ | niemeyer, also, just to check, IMO files and symlinks should fail to replace directories; sane? | 22:29 |
| niemeyer | fwereade_: Why? | 22:30 |
| niemeyer | fwereade_: It really depends on the intended semantics | 22:30 |
| fwereade_ | niemeyer, hmm, just because it feel a bit drastic to my personal gut | 22:30 |
| fwereade_ | niemeyer, it's equally perfectly fine by me to say Bundle.ExpandTo will overwrite with prejudice | 22:31 |
| niemeyer | fwereade_: I think we have to pick something sane from the upgrading perspective | 22:31 |
| niemeyer | fwereade_: If that's what we're doing | 22:31 |
| niemeyer | fwereade_: You know what we could do.. | 22:32 |
| fwereade_ | niemeyer, well, as long as people don't store data in dirs that then get turned into files/symlinks by future versions of the charm, we're fine | 22:32 |
| niemeyer | fwereade_: We could use bzr | 22:32 |
| fwereade_ | niemeyer, how so? | 22:33 |
| niemeyer | fwereade_: Well, "as long as people don't do something that feels ok doing" is somewhat of a dead end :) | 22:33 |
| niemeyer | fwereade_: bzr has all the logic for merging state against working trees | 22:34 |
| niemeyer | fwereade_: We'll spend weeks trying to get close to it, and won't | 22:34 |
| fwereade_ | niemeyer, I'm not sure it feels like an entirely ok thing to do to me, honestly | 22:35 |
| niemeyer | fwereade_: You're thinking too rationally about it, IMO | 22:35 |
| fwereade_ | niemeyer, haha | 22:35 |
| niemeyer | fwereade_: People will delete the directory.. hack some more.. write the file.. deploy | 22:35 |
| niemeyer | fwereade_: This isn't particularly bad looking | 22:36 |
| fwereade_ | niemeyer, is it really rational of them to think that they can have two entirely different things coexisting with the same name, though? | 22:37 |
| niemeyer | fwereade_: Again, they won't sit in a terminal thinking "I'm replacing a directory by a file!" | 22:37 |
| niemeyer | fwereade_: It will simply happen, and it will blow up after the fact | 22:37 |
| fwereade_ | niemeyer, and how will bzr deal with that situation? just that it'll tell us there's a problem? | 22:38 |
| fwereade_ | niemeyer, in that case I favour just falling over directly when we try to delete a dir | 22:39 |
| fwereade_ | niemeyer, do the charm tests have an upgrade-from-previous-version bit? | 22:39 |
| niemeyer | fwereade_: http://pastebin.ubuntu.com/1157995/ | 22:40 |
| fwereade_ | niemeyer, http://pastebin.ubuntu.com/1158000/ | 22:44 |
| fwereade_ | niemeyer, I think that is the situation we have to worry about | 22:44 |
| fwereade_ | niemeyer, with the added bonus that there is nothing that stops people from creating their state directories only after the charm has been deployed | 22:45 |
| fwereade_ | davecheney, heyhey | 22:46 |
| niemeyer | fwereade_: Right, and bzr seems to have done the right thing in both cases | 22:50 |
| fwereade_ | niemeyer, hmm, ok, it would also have some other nice advantages like being able to delete files as well | 22:54 |
| niemeyer | fwereade_: Right, and only if they changed | 22:55 |
| niemeyer | erm | 22:55 |
| niemeyer | fwereade_: Right, and only if they changed NOT | 22:55 |
| niemeyer | :-) | 22:55 |
| niemeyer | fwereade_: bzr import -h, btw | 22:55 |
| niemeyer | fwereade_: We'd manage the importing into a side repo, and then pull onto the production side | 22:57 |
| niemeyer | fwereade_: Will all the nice benefits of that (don't screw up unless it all works, etc) | 22:57 |
| niemeyer | fwereade_: Rollbacks (!) | 22:57 |
| niemeyer | fwereade_: For the moment, though, bzr import + bzr pull sounds like an awesome low-hanging fruit | 22:59 |
| fwereade_ | niemeyer, well, goodness me: so we just create a little standalone tree with the first expanded content and branch that into the charm dir; and subsequently import direct from the bundle into that side repo, then pull from that into the charm dir? | 22:59 |
| niemeyer | fwereade_: Exactly | 22:59 |
| niemeyer | fwereade_: We can share the repo between them, for added goodness | 23:00 |
| fwereade_ | niemeyer, sorry, between who? | 23:00 |
| niemeyer | fwereade_: The two working trees | 23:00 |
| niemeyer | fwereade_: Although I'm not sure if bzr import could make our day | 23:01 |
| * niemeyer tries | 23:01 | |
| fwereade_ | fwereade suspects this is the point at which he should actually figure out bzr rather than just using it in a spirit of optimism ;) | 23:02 |
| niemeyer | LOL | 23:04 |
| niemeyer | BOOOM | 23:06 |
| niemeyer | [niemeyer@gopher ~/foo]% bzr import ../foo3.zip | 23:06 |
| niemeyer | bzr: ERROR: bzrlib.errors.MalformedTransform: Tree transform is malformed [('overwrite', 'new-3', u'dir')] | 23:06 |
| niemeyer | fwereade_: Yeah, we need the side-repo :-) | 23:06 |
| fwereade_ | niemeyer, aww | 23:06 |
| niemeyer | fwereade_: Well, side branch | 23:06 |
| niemeyer | fwereade_: So a side branch where we import stuff | 23:06 |
| niemeyer | fwereade_: bzr branch import-branch final-charm-location | 23:07 |
| fwereade_ | niemeyer, ok, yep, and the actual charm dir is just a branch of that | 23:07 |
| niemeyer | fwereade_: run unit on final-charm-location | 23:07 |
| fwereade_ | niemeyer, ok, great | 23:07 |
| fwereade_ | niemeyer, we'll also want a final pass to fix up file modes, I think, which is a bit annoying | 23:08 |
| niemeyer | fwereade_: On upgrade, 1) bzr import charm.bundle import-branch (I think that works out of the box); 2) bzr commit final-charm-location -m "Preparing for upgrade."; 3) bzr merge import-branch final-charm-location; 4) bzr commit final-charm-location | 23:08 |
| niemeyer | fwereade_: Hmm, why? | 23:09 |
| fwereade_ | niemeyer, there are tests for modes like 750 that apparently can't be expressed by bzr | 23:10 |
| niemeyer | fwereade_: I'd wait until that is a real problem | 23:10 |
| niemeyer | fwereade_: Getting sane upgrades would already be outstanding | 23:11 |
| fwereade_ | niemeyer, ok, then I'll just drop the unwanted features of ExpandTo and implement this upgrade logic separately | 23:12 |
| niemeyer | fwereade_: May I suggest ignoring ExpandTo for the moment? | 23:12 |
| fwereade_ | niemeyer, yeah, why not :) | 23:13 |
| niemeyer | fwereade_: Well, unless you're already half-way through it | 23:13 |
| niemeyer | fwereade_: Otherwise, it feels like it won't help or get in the way | 23:13 |
| fwereade_ | niemeyer, yeah, it's totally orthogonal really, I'll leave the branch around and finish it off in my Copious Free Time | 23:14 |
| * fwereade_ has a *new* plan forming | 23:14 | |
| niemeyer | fwereade_: Awesome | 23:14 |
| fwereade_ | niemeyer, now embarrassingly trivial: https://codereview.appspot.com/6454169 | 23:24 |
| niemeyer | fwereade_: Looking | 23:24 |
| niemeyer | fwereade_: Love embarrassingly trivial branches.. LGTM | 23:25 |
| fwereade_ | niemeyer, cheers | 23:25 |
| niemeyer | fwereade_: Please just tweak the message before submitting | 23:25 |
| fwereade_ | niemeyer, thanks for the reminder :) | 23:25 |
| niemeyer | fwereade_: np :) | 23:25 |
| fwereade_ | niemeyer, ok, more ramifications trickle into my mind: this now means we have another potential "error" state which is not hook-related but which is susceptible to human intervention | 23:39 |
| fwereade_ | niemeyer, so this is good, it even fits sanely with --retry (thank goodness we didn't call it retry-hooks ;)) | 23:41 |
| fwereade_ | niemeyer, it would be possible for us to revert and continue, but I don't think it's actually helpful to do so | 23:42 |
| niemeyer | fwereade_: Hmm.. I'm on the fence on it | 23:46 |
| niemeyer | fwereade_: Either way, it feels like a plus we don't have to worry about for now | 23:46 |
| fwereade_ | niemeyer, ok, I'll go with a pause then; retry can revert and try again with (hopefully, presumably) the new version of the charm they've set on the service, which doesn't have this problem -- while no-retry implies that they have completed the change themselves | 23:48 |
| fwereade_ | niemeyer, if we auto-revert they don't get much of an opportunity to fix it themselves unless we also add a pause-unit feature or something | 23:49 |
| niemeyer | fwereade_: Good point | 23:50 |
| niemeyer | fwereade_: Hmm.. what does "completing themselves" mean? | 23:50 |
| fwereade_ | niemeyer, fixed the conflicts | 23:50 |
| niemeyer | fwereade_: It's not clear --retry makes sense for upgrading, besides for the hook itself | 23:50 |
| fwereade_ | niemeyer, I think it offers a nice path back to a working unit by settings a new charm that's ok -- or even the old one again | 23:52 |
| niemeyer | fwereade_: After "juju resolved", it seems like we could run "bzr resolved --all', and in case that succeeds, commit the merge | 23:53 |
| fwereade_ | niemeyer, yeah, that would be what do do on non-retry | 23:54 |
| niemeyer | fwereade_: How's that different from yes-retry? | 23:54 |
| fwereade_ | niemeyer, yes-retry, I think, means that the user has set a different charm on the service and wants us to try an upgrade from scratch | 23:55 |
| niemeyer | fwereade_: We don't have to ask the user whether he wants that or not | 23:56 |
| niemeyer | fwereade_: We just need metadata to tell us what is in the directory | 23:56 |
| niemeyer | fwereade_: Same trick we use with the agent | 23:57 |
| niemeyer | fwereade_: if proposed != current { upgrade } | 23:57 |
| fwereade_ | niemeyer, hmm... ok, so we also store what charm we were trying to upgrade to; and if that is different from the current one on the service, we revert and try again with the new charm? | 23:58 |
| fwereade_ | niemeyer, or if the service's new charm is the same as the unit's old one, I guess we just revert :) | 23:59 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!