[04:21] <thumper> mwhudson: testfix review ? http://pastebin.ubuntu.com/424977/
[04:21] <thumper> although I'd just rs it
[04:21] <mwhudson> thumper: r=me
[04:21] <thumper> ta
[04:21] <mwhudson> with added stabbing
[04:23] <mwhudson> thumper: lib/lp/code/doc/branch-karma.txt seems to have a similar issue btw
[04:23] <thumper> is it failing?
[04:24] <mwhudson> no
[04:24] <mwhudson> but it has a factory name in expected output
[04:24] <mwhudson> lib/lp/bugs/doc/bugnotification-sending.txt too
[04:24] <mwhudson> i guess i can make a quick branch for this
[04:24] <thumper> bzr pqm-submit -m "[testfix][r=mwhudson] Fix archive-deletion test - NEVER PRINT OUT THE NAME OF A FACTORY GENERATED ANONYMOUS PERSON (stabby-stabby)."
[04:25] <mwhudson> hah
[04:25] <thumper> you grepped for person-name ?
[04:26] <mwhudson> "PPA named nightly for Person-name2 owned by Person-name2" :(
[04:26] <mwhudson> thumper: grepping for '-name[0-9]
[04:26] <mwhudson> thumper: grepping for '-name[0-9]' finds some stuff
[04:28] <thumper> oh dear god!
[04:30]  * mwhudson changes the starting value for getUniqueInteger, and shoves that into ec2
[04:31] <thumper> :)
[06:08] <thumper> mwhudson: up for a review?
[06:08] <mwhudson> thumper: sure
[06:08] <mwhudson> four tests have failed in that ec2 test run so far btw
[06:11] <thumper> only four?
[06:11] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/more-careful-network-service-usage/+merge/24469
[06:11] <mwhudson> they've only been going for about 20 minutes :)
[06:12] <thumper> :)
[06:12] <thumper> oh
[06:12] <thumper> has it taken that long to set up the instance?
[06:12] <thumper> didn't you start it at 3:30?
[06:13] <mwhudson> um
[06:13] <mwhudson> i can't tell the progress of time any more clearly :-)
[06:13] <mwhudson> the first instance didn't start though
[06:14] <thumper> diff is up
[06:14] <mwhudson> thumper: do you think repr(self) would be a better default getOperationDescription ?
[06:14] <mwhudson> it's unfriendly, but maybe less useless
[06:14] <thumper> mwhudson: it is supposed to be in a nice email to a user
[06:15] <thumper> so a repr wouldn't be so good
[06:15] <thumper> saying: something went wrong doing xxx
[06:15] <thumper> roughly
[06:15] <mwhudson> thumper: 'unspecified operation' is not friendly!
[06:15] <thumper> better than oops
[06:15] <thumper> that line isn't necessary
[06:15] <thumper> but really a fall back
[06:16] <mwhudson> ok
[06:16] <mwhudson> self.logger.error(e) <- i think you want self.logger.exception there
[06:16] <thumper> oh
[06:16] <thumper> ok
[06:17] <thumper> line 162 of runner.py is where I copied it from
[06:17] <thumper> I should fix that too I guess
[06:17] <mwhudson> yeah, i reckon
[06:17] <thumper> done
[06:18] <mwhudson> tracebacks are good when things go ker-blooey
[06:18]  * thumper nods
[06:20] <mwhudson> thumper: you don't ned to pass the exception to exception(), did you know that?
[06:20] <mwhudson> i didn't
[06:20] <thumper> no
[06:20] <mwhudson> so it should be self.logger.exception("something went wrong")
[06:20] <thumper> I didn't know that
[06:20] <thumper> mwhudson: it checks the exec stack?
[06:21] <mwhudson> it calls sys.exc_info() i guess
[06:21] <mwhudson> seems a bit lame
[06:21]  * thumper shrugs
[06:21] <thumper> I guess I should add a message of some form then
[06:22] <thumper>                 self.logger.exception(
[06:22] <thumper>                     "Failed to notify users about a failure.")
[06:22] <thumper> ?
[06:22] <mwhudson> thumper: reviewed
[06:22] <mwhudson> +1
[06:23] <thumper> thanks
[06:24] <thumper> is BjornT our release manager?
[06:24] <mwhudson> i think so
[06:26]  * thumper whacks it into ec2 test while I await the RC
[06:32] <BjornT> thumper: yes, i am
[06:32] <thumper> BjornT: hi
[06:32] <thumper> BjornT: https://code.edge.launchpad.net/~thumper/launchpad/more-careful-network-service-usage/+merge/24469
[06:32] <thumper> BjornT: fallout from a screwed staging environment
[06:33] <thumper> we weren't handling some errors in a nice enough manner
[06:33] <thumper> especially since we can't send email right now
[06:33] <thumper> so our email about your failing job was failing causing more issues
[06:33] <thumper> this branch adds an extra except block that just logs
[06:35] <thumper> BjornT: also be aware that in a conflict resolution branch I disabled a soyuz test, so they should land a fix for that
[06:36]  * thumper fetches a drink
[06:36] <StevenK> thumper: Which test, and how does it fail?
[06:37]  * StevenK fetches the mail, while waiting for dogfood's ppa publisher to die again
[06:39] <BjornT> thumper: ok, in general it looks ok for rc. however, it seems like the new exception handling you added is untested, no?
[06:46] <thumper> BjornT: it is
[06:46] <thumper> BjornT: untested that is
[06:46] <thumper> BjornT: I'd have to cause a failure, then monkey patch the sending to fail again
[06:46] <thumper> all to add something to the log
[06:47] <thumper> is it worth it?
[06:47] <thumper> StevenK: https://bugs.edge.launchpad.net/soyuz/+bug/572005
[06:47] <mup> Bug #572005: Disabled test buildd-slavescanner.txt <Soyuz:Triaged> <https://launchpad.net/bugs/572005>
[06:52] <BjornT> thumper: it's not hard to test without monkey patching. all you have to do is to create a custom Job class that fails in the place you expect it to fail, and maybe sub-classing BaseJobRunner to not do anything in runJob(). giving it's so simple, i'd say it's worth it.
[06:53] <thumper> BjornT: ok, I'll take a look
[06:54] <BjornT> thumper: and you're not only logging, you also report an oops.
[06:54] <thumper> BjornT: do you think I should just log?
[06:54] <thumper> I'm pretty sure that generating an oops is guaranteed not to oops
[06:56] <BjornT> thumper: i do think you should log an oops, since that makes the error visible to us. so maybe two tests are needed. one where notifyUserError errors out, and one where notifyOops errors out, to show that you will still log an oops in that case.
[06:57] <thumper> ok
[06:57] <thumper> I'm failing to generate the enthusiasm needed to do it at 6pm on a Friday
[06:57] <BjornT> thumper: without such a tests, it's tempting for someone to refactor that outer oops reporting to use self._doOops()
[06:57] <thumper> ok
[06:58] <noodles775> Morgan
[07:03] <BjornT> thumper: i'd be willing to let you land that branch as it is, though, if you promise to file bugs about the tests, and write them on monday (so that you can more easily qa this on monday)
[07:06] <thumper> BjornT: sounds fair
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing:  || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[11:04] <noodles775> Hi adeuring, when you're up for it: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-enable-buildd-slavescanner.txt/+merge/24478
[11:05] <adeuring> noodles775: sure
=== adeuring changed the topic of #launchpad-reviews to:  On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== noodles785 is now known as noodles775
[11:20] <adeuring> noodles775: r=me
[11:20] <noodles775> Thanks adeuring
[11:22] <noodles775> Hi BjornT, can I get an RC to re-enable two soyuz tests (just to be sure that any later RC's can't break things tested in the two tests).
[11:22] <noodles775> https://code.edge.launchpad.net/~michael.nelson/launchpad/db-enable-buildd-slavescanner.txt/+merge/24478
[12:05] <BjornT> noodles775: of course
[12:44] <noodles775> Thanks BjornT
[13:25] <noodles775> adeuring: another one when you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/567922-binarypackagebuild-packagebuild-4/+merge/24356
[13:25] <noodles775> Sorry for the verbose MP.
[13:26] <adeuring> noodles775: nah, i like verbose MPs :)
=== adeuring changed the topic of #launchpad-reviews to:  On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:01] <allenap> adeuring: Fancy a couple of reviews?
[14:01] <adeuring> allenap: sure; I'm just finishing one for noodles775.
=== allenap changed the topic of #launchpad-reviews to:  On call: adeuring || reviewing: noodles775 || queue: [allenap, allenap] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:02] <allenap> adeuring: Thanks. https://code.edge.launchpad.net/~allenap/launchpad/storm-bulk-reload-bug-572211/+merge/24492 then https://code.edge.launchpad.net/~allenap/launchpad/checkwatches-bulk-reload-bug-572211/+merge/24493
=== noodles775 changed the topic of #launchpad-reviews to:  On call: adeuring || reviewing: noodles775 || queue: [allenap, allenap, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:05] <adeuring> noodles775: r=me
[14:05] <noodles775> Thanks adeuring.
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: allenap || queue: [allenap, noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:50] <adeuring> allenap: a very nice branch! just one suggestion: What about renaming "identity" to "object_is_key" so the intended usage is a bit more obvious
[14:58] <EdwinGrubbs> adeuring: I have a small branch that I'll want to try to get an RC for after it is reviewed. https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-567065-empty-branch/+merge/24497
=== EdwinGrubbs changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: allenap || queue: [allenap, noodles775, Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[14:58] <adeuring> EdwinGrubbs: add it to the queue.
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: allenap || queue: [noodles775, Edwin] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:02] <noodles775> adeuring: my branch isn't an RC, so pls give EdwinGrubbs's branch priority.
[15:03] <adeuring> noodles775: sure
[15:03] <adeuring> EdwinGrubbs: sorry, did not notice that it is an RC branch....
[15:06] <allenap> adeuring: Okay, good idea. Thanks!
[15:07] <adeuring> allenap: approved your other branch too
[15:07] <allenap> adeuring: Brilliant, thanks.
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: Edwin || queue: [noodles775] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:08] <adeuring> allenap: I'll miss things like I saw today when your on "bugs holidays"
[15:08] <adeuring> ...you are on...
[15:09] <allenap> adeuring: Hehe, I won't be gone long :)
[15:28] <adeuring> EdwinGrubbs: r=me
[15:28] <EdwinGrubbs> adeuring: thanks
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: noodles775 || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[15:39] <EdwinGrubbs> deryck: I assume you are the backup RM since you were the release manager last month. Can I get an RC for https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-567065-empty-branch/+merge/24497
[15:39] <adeuring> noodles775: has your branch 567922-binarypackagebuild-new-table-1 really a 1500 lines diff or should I use a diff against some other branch?
[15:40] <noodles775> adeuring: no it is not.
[15:40]  * noodles775 looks
[15:41] <deryck> EdwinGrubbs, sure, I can look at it.  Though I would feel more comfortable if flacoste looked, since he's probably already thought about release more than me.
[15:42] <EdwinGrubbs> deryck: ok, I'll ask him
[15:42] <EdwinGrubbs> flacoste: can I get an RC for https://code.edge.launchpad.net/~edwin-grubbs/launchpad/bug-567065-empty-branch/+merge/24497
[15:42] <deryck> EdwinGrubbs, if he is not able, then I can certainly.
[15:45] <noodles775> adeuring: here's the actual diff that I get locally...http://pastebin.ubuntu.com/425292/ I'm guessing I haven't pushed the previous branch.
[15:45] <adeuring> noodles775: thanks!
[15:58] <noodles775> adeuring: I'll need to merge a new db-devel on that branch and resolve all the conflicts :/
[15:58] <noodles775> It might be best to leave it for now.
[15:59] <adeuring> noodles775: ok, noproblem
=== gary_poster is now known as gary-lunch
=== adeuring changed the topic of #launchpad-reviews to: On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
[16:29] <EdwinGrubbs> deryck: I can't find flacoste, can you look at the branch?
[16:30] <flacoste> EdwinGrubbs: ???
[16:30] <flacoste> EdwinGrubbs: i approved it
[16:31] <deryck> no need for me then :-)
[16:36] <EdwinGrubbs> flacoste: oh, I'm still not used to that being part of the MP
=== EdwinGrubbs is now known as Edwin-lunch
=== adeuring changed the topic of #launchpad-reviews to: On call: - || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.edge.launchpad.net/launchpad/+activereviews
=== gary-lunch is now known as gary_poster
=== matsubara is now known as matsubara-lunch
=== deryck is now known as deryck[lunch]
=== matsubara-lunch is now known as matsubara
=== deryck[lunch] is now known as deryck
=== matsubara is now known as matsubara-afk