| === mechanobot is now known as amithkk | ||
| jam | dang, just missed fwereade | 08:06 |
|---|---|---|
| rogpeppe1 | jam: surely he wasn't up all night? | 09:05 |
| rogpeppe1 | jam: morning, BTW! | 09:05 |
| jam | well he disconnected ~1.5hrs ago, just about 15 min before I went looking for him. | 09:05 |
| jam | hi fwereade | 09:05 |
| jam | hi rogpeppe1 | 09:05 |
| TheMue | btw, morning all | 09:06 |
| jam | apparently I keep scaring fwereade away :0 | 09:07 |
| mgz | :) | 09:07 |
| fwereade | jam, hey, sorry :) | 09:07 |
| fwereade | jam, didn't even realise I was disconnected | 09:07 |
| fwereade | jam, what can I do for you? | 09:07 |
| fwereade | TheMue, rogpeppe1: heyhey | 09:07 |
| rogpeppe1 | fwereade: heya | 09:08 |
| jam | fwereade: I was noticing you had a patch that landed with only 1 review. I'm guessing that was because you felt it was trivial. But I just wanted to check with you. | 09:08 |
| jam | (My personal feeling is that requiring 2 +1's is too much, but I'd rather other people feel the pain of it to understand why I feel that way :) | 09:08 |
| TheMue | rogpeppe1: your stack trace hint has been good, thank you. so i yesterday evening have been able to place those on the right locations and this morning i analyzed the output. and i found a pattern when the test fails and could fix it in the firewaller. | 09:09 |
| fwereade | jam, yeah, that's right, confirmed trivial in IRC | 09:09 |
| TheMue | rogpeppe1: now i'm doing some more tests, but it looks good. | 09:09 |
| rogpeppe1 | TheMue: glad to hear it | 09:09 |
| fwereade | jam, I though I said trivial in the CL description | 09:09 |
| TheMue | fwereade: heya | 09:09 |
| jam | fwereade: you did mention that. It wasn't immediately clear what that meant. (I haven't heard it discussed that trivial patches only need 1 review.) | 09:09 |
| fwereade | jam, I'm not sure it's written down anywhere but it's been part of the team culture since I joined, I think | 09:10 |
| rogpeppe1 | fwereade: tbh i think that particular CL was verging on the non-trivial (it's not usual to put substantial code blocks in export_test.go) but given where it was going, i don't mind. | 10:00 |
| fwereade | rogpeppe1, ah, sorry -- I'm working to remove the need for that even now | 10:01 |
| fwereade | rogpeppe1, although really I should be more release-focused, hmm | 10:01 |
| rogpeppe1 | fwereade: cool, thought so | 10:01 |
| TheMue | so, back, sorry for being afk. while we talked about the sleeping troubles with young children yesterday the troubles simetimes grow with the age. our are now almost 17 and 19 and have troubles with school and job. *sigh* | 10:39 |
| fwereade | rogpeppe1, https://codereview.appspot.com/7058061 addressed | 10:50 |
| rogpeppe1 | fwereade: reviewed | 10:56 |
| fwereade | rogpeppe1, thanks | 11:06 |
| fwereade | morning Aram | 11:10 |
| Aram | hey there. | 11:10 |
| rogpeppe1 | fwereade: trivial: https://codereview.appspot.com/7064064 | 11:21 |
| fwereade | rogpeppe1, LGTM, concur trivial | 11:24 |
| rogpeppe1 | fwereade: ta | 11:24 |
| rogpeppe1 | fwereade: Aram: could you register with gravatar? it would be nice to be able to see kanban card ownership at a glance. | 11:29 |
| Aram | rogpeppe1, but I did. | 11:29 |
| Aram | I don't know why it's now showing, | 11:30 |
| rogpeppe1 | Aram: with your canonical email addr, presumably | 11:30 |
| Aram | yes | 11:30 |
| rogpeppe1 | Aram: ah! i just needed to reload | 11:30 |
| rogpeppe1 | Aram: thanks | 11:30 |
| rogpeppe1 | fwereade: just you to go :-) | 11:31 |
| fwereade | rogpeppe1, blech, ok ;p | 11:31 |
| fwereade | rogpeppe1, I can't see myself yet but I think I've done it | 11:38 |
| rogpeppe1 | fwereade: i imagine there's some caching to time out before we see it | 11:39 |
| fwereade | rogpeppe1, so I would imagine | 11:39 |
| fwereade | rogpeppe1, TheMue, Aram: https://codereview.appspot.com/7062062 -- principal Uniter sets subordinates to Dying when the principal is Dying | 11:52 |
| TheMue | *click* | 11:52 |
| jam | mgz: poke for standup | 12:03 |
| mgz | did wonder at 11 if we were doing one :) | 12:03 |
| jam | mgz: yeah, I was on an hour ago, but we were meant to do it now. | 12:03 |
| rogpeppe1 | fwereade: reviewed | 12:04 |
| mgz | ah, yeah, had mentioned bumping but calendar was still old time | 12:04 |
| fwereade | rogpeppe1, TheMue: cool, thanks | 12:05 |
| * fwereade ->lunch | 12:10 | |
| jam | mgz: at one point you said that we don't need listings for public buckets, but the code we are trying to share with ec2 wants to list all the tools it has available, and 'pick the best', which is why we do need .rlistings | 12:32 |
| jam | for the public location. | 12:32 |
| rogpeppe1 | fwereade: initial stab at agent package: https://codereview.appspot.com/7064066 | 12:38 |
| rogpeppe1 | other reviews also appreciated, please | 12:38 |
| TheMue | rogpeppe1: done | 12:57 |
| rogpeppe1 | TheMue: thanks | 12:57 |
| TheMue | rogpeppe1: yw | 12:57 |
| fwereade | rogpeppe1, why json not yaml? I thought yaml was the project standard for data files, regardless of personal taste | 13:14 |
| fwereade | rogpeppe1, everything else looks good though | 13:14 |
| rogpeppe1 | fwereade: yaml doesn't cope well with []byte | 13:14 |
| rogpeppe1 | fwereade: i could change CACert to be string everywhere, i suppose | 13:15 |
| rogpeppe1 | fwereade: although that has its own issues (the standard crypto packages use []byte for certificates and keys) | 13:16 |
| fwereade | rogpeppe1, ok, that's reasonable but deserves a comment -- but also, remind me why we went with a single file? | 13:16 |
| TheMue | fwereade, rogpeppe1: a review with only two lines https://codereview.appspot.com/7068059/ (read the text, the firewaller is the change which alread had two LGTMs but then has been rejected by Dave. line 272 is the new one). | 13:16 |
| fwereade | rogpeppe1, I don't think you should change the cert type | 13:16 |
| rogpeppe1 | fwereade: it's much less code, easier to expand to include more options, and having a single "agent.conf" file seems to me to be a good thing for debugging and general non-clutteredness | 13:17 |
| rogpeppe1 | s/more options/more fields/ | 13:17 |
| TheMue | fwereade, rogpeppe1: and i would ask one of you to run the whole tests, i've got one language error because my system is germen and the compared text is expected tobe english. ;) | 13:18 |
| fwereade | rogpeppe1, ok, I guess tasks that need to change it can return ErrConfChangeReady, like we do with ErrUpgradeReady | 13:18 |
| fwereade | TheMue, can you not fix it with LC_LOCALE or something? | 13:18 |
| rogpeppe1 | TheMue: the compared text from what? | 13:19 |
| rogpeppe1 | fwereade: i'd imagined having an api that enables changing it in a safe way (with a mutex lock) | 13:19 |
| TheMue | rogpeppe1: one moment, will look for the name of test | 13:19 |
| fwereade | TheMue, os.Setenv("LC_ALL", "en_US") fixes language weirdness in the git tests, I believe | 13:19 |
| TheMue | fwereade: you, would be an idea | 13:19 |
| TheMue | s/you/yes | 13:20 |
| rogpeppe1 | fwereade: it's not difficult to do, and it's only owned by one running program, so there's no issue with concurrent changes from outside | 13:20 |
| TheMue | fwereade: the test is btw not mine, it's one where an external tool is called. | 13:20 |
| rogpeppe1 | TheMue: which tool? | 13:21 |
| fwereade | rogpeppe1, true -- feels like it subtly works against task independence but not enough to really bug me badly | 13:21 |
| rogpeppe1 | fwereade: i don't think it needs to | 13:21 |
| rogpeppe1 | fwereade: each task can still make independent changes | 13:21 |
| fwereade | rogpeppe1, will be able to, once api is in place | 13:21 |
| rogpeppe1 | fwereade: sure | 13:22 |
| rogpeppe1 | :-) | 13:22 |
| fwereade | rogpeppe1, but LGTM | 13:22 |
| rogpeppe1 | fwereade: thanks | 13:22 |
| rogpeppe1 | fwereade: will add a comment | 13:22 |
| fwereade | rogpeppe1, cheers | 13:22 |
| TheMue | rogpeppe1: it's bzr in lpad_test.go:16: StoreSuite.TestPublishCharmDistro | 13:22 |
| TheMue | rogpeppe1: nothing wild, the printed text in german matches to the expected english one. but the test fails. ;) | 13:23 |
| fwereade | TheMue, also LGTM assuming those are the only changes from before | 13:23 |
| TheMue | fwereade: yes, only addMachine() with instance id and treating ErrNoInstances correect. | 13:24 |
| TheMue | fwereade: there has been another race due to my usage of a vm, but that has been fixed by roger (thanks again) | 13:26 |
| fwereade | TheMue, was that the InjectMachine? I saw that, thanks | 13:36 |
| TheMue | fwereade: that has been after a hint of roger. but he also had a CL. | 13:36 |
| TheMue | fwereade: it's in since yesterday. | 13:37 |
| fwereade | TheMue, ok, thanks | 13:37 |
| * rogpeppe1 is away for a bit of lunch. back in 30 mins. | 13:50 | |
| mgz | jam: yup, discovered that when reviewing the branch, it's a change in the go version due to wanting to give out executables | 13:56 |
| === TheMue_ is now known as TheMue | ||
| rogpeppe1 | back | 14:24 |
| === rogpeppe1 is now known as rogpeppe | ||
| TheMue | rogpeppe: any chance to look into my review? | 14:52 |
| rogpeppe | TheMue: i will do in a little bit, just been in a meeting, and am looking at something else currently | 14:53 |
| TheMue | rogpeppe: ok, np | 14:53 |
| rogpeppe | TheMue: reviewed | 15:00 |
| TheMue | rogpeppe: just seen, thx | 15:00 |
| TheMue | rogpeppe: and the idea of the double proposal is good | 15:01 |
| rogpeppe | TheMue: kanban meeting? | 15:01 |
| TheMue | rogpeppe: afaik yes | 15:02 |
| rogpeppe | TheMue: https://plus.google.com/hangouts/_/539f4239bf2fd8f454b789d64cd7307166bc9083 | 15:03 |
| fwereade | rogpeppe, TheMue: https://codereview.appspot.com/7073056/ addresses the most critical remaining subordinates problem - I'm about to verify it live, but feel hopeful, and would appreciate reviews | 16:10 |
| TheMue | fwereade: *click* | 16:10 |
| TheMue | fwereade: oh, that is the one i already reviewed ;) | 16:11 |
| fwereade | TheMue, <3 | 16:11 |
| rogpeppe | fwereade: reviewed | 16:25 |
| fwereade | rogpeppe, tyvm | 16:27 |
| fwereade | rogpeppe, yeah, it works :) | 16:43 |
| === slank is now known as slank_away | ||
| rogpeppe | fwereade: yay! | 16:43 |
| rogpeppe | fwereade: that presumably increases the number of charms we're compatible with by a significant amount | 16:43 |
| fwereade | rogpeppe, almost better than expected -- a failed relation-broken hook in rsyslog-forwarder is keeping its principal unit alive | 16:43 |
| === slank_away is now known as slank | ||
| fwereade | I am suspicious about this: | 16:44 |
| fwereade | 2013/01/10 16:40:35 JUJU worker/uniter: HOOK /var/lib/juju/agents/unit-rsyslog-forwarder-0/charm/hooks/syslog-relation-broken: 5: /var/lib/juju/agents/unit-rsyslog-forwarder-0/charm/hooks/syslog-relation-broken: JUJU_REMOTE_UNIT: parameter not set | 16:44 |
| fwereade | rogpeppe, can you think why JUJU_REMOTE_UNIT would be expected or needed in a -broken hook? | 16:44 |
| rogpeppe | fwereade: no | 16:45 |
| rogpeppe | fwereade: perhaps it's expected to be the last unit that left the relation? | 16:45 |
| fwereade | rogpeppe, heh, maybe, that's pretty crackful if so | 16:46 |
| fwereade | rogpeppe, there's no guarantee there's been another unit in scope | 16:46 |
| fwereade | rogpeppe, but, yay, it all works -- resolving the error caused the subordinate to shut down smoothly, the principal recalled and removed it, the principal set itself to dead and the machine recalled and removed it | 16:48 |
| fwereade | rogpeppe, ...and when I destroyed the relation, the subordinate attached to the other unit shut itself down neatly too | 16:50 |
| rogpeppe | fwereade: why would you get a relation-broken if another unit hasn't been in scope? | 16:55 |
| rogpeppe | (sorry, someone came to the door, just back) | 16:55 |
| fwereade | rogpeppe, because relation-broken is the last thing that happens before a unit leaves a relation -- no more and no less | 16:57 |
| fwereade | rogpeppe, if we had a relation-forged hook that was guaranteed to run first, relation-broken would be its counterpart | 16:59 |
| rogpeppe | fwereade: ah, in which case JUJU_REMOTE_UNIT in that context is definitely crack | 16:59 |
| fwereade | rogpeppe, yeah, I just wonder why the charm uses it | 16:59 |
| rogpeppe | fwereade: have you had a look? | 16:59 |
| fwereade | rogpeppe, just did -- it's getting the service name from it :) | 17:00 |
| rogpeppe | fwereade: presumably there's an easier way of doing that... | 17:00 |
| fwereade | rogpeppe, or is it? actually I don;t know wtf it's doing | 17:00 |
| fwereade | rogpeppe, more to the point I want to find out why it would work in python | 17:01 |
| rogpeppe | fwereade: yeah | 17:01 |
| === slank is now known as slank_away | ||
| === slank_away is now known as slank | ||
| fwereade | rogpeppe, TheMue: almost trivial before I stop: https://codereview.appspot.com/7080043 | 17:29 |
| TheMue | fwereade: *click* | 17:29 |
| rogpeppe | fwereade: LGTM | 17:31 |
| TheMue | fwereade: ^ | 17:32 |
| fwereade | rogpeppe, TheMue: cheers guys | 17:32 |
| TheMue | fwereade: yw | 17:33 |
| === slank is now known as slank_away | ||
| rogpeppe | end of day hereg'night all. | 18:35 |
| rogpeppe | ha | 18:35 |
| rogpeppe | g'night all | 18:35 |
| === slank_away is now known as slank | ||
| === slank is now known as slank_away | ||
| === slank_away is now known as slank | ||
| === slank is now known as slank_away | ||
| === slank_away is now known as slank | ||
| === slank is now known as slank_away | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!