[09:14] <adeuring> noodles775: for which branch do you need a review?
[09:15] <noodles775> adeuring: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-get-or-create-from-identity/+merge/28885
[09:15] <noodles775> Thanks!
[09:16]  * adeuring is looking
[09:16] <noodles775> I asked salgado to take a brief look yesterday (as it's his area), and he's commented, but it needs a proper review.
[10:06] <noodles775> adeuring: also, i've got another branch that's not ready for review, but I need some help (related to private XMLRPC authentication - I'm a little unfamiliar) if you've got any input: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059
[10:09] <adeuring> noodles775: so, you want to add another celebrity?
[10:09] <noodles775> adeuring: that's already done (and landed by StevenK). At the moment I'm just trying to write a test that logs in as admin and passes.
[10:11] <noodles775> The xmlrpc call works fine (I can switch to zope.Public and the tests work), but I can't see how to assign launchpad.Admin to the xmlrpc view (or if I even should be going down that direction - I can't see any other examples in the code)
[10:12] <noodles775> Hrm, maybe the test should be authenticating differently, or the logged in user needs ssh keys etc.
[10:12] <adeuring> noodles775: as I understand it, you just need to ensure in checkAuthenitcated that the current user is your celebrity. Or am I missing something?
[10:13] <noodles775> adeuring: that's what I would have thought, but that breakpoint is never being called (and yet the tests return 401 Unauthorized)
[10:14] <adeuring> noodles775: seems that some other authentication check is kicking in then?
[10:14] <noodles775> Yep... I'll check if it's related to ssh keys on the test users.
[10:14] <adeuring> is there any auth class for the same permission and interface class?
[10:15] <noodles775> Nope - it's a new class in that branch.
[10:21] <adeuring> noodles775: after looking closer, I must ammit that I have no clue why this doesn't work...
[10:22] <noodles775> adeuring: ok, thanks for looking :D
[10:33] <noodles775> adeuring: found it... I needed to supply the credentials when creating the xmlrpc server proxy... obvious now.
[10:33] <adeuring> noodles775: uuhhh, yes ;)
[10:34] <noodles775> But I still need to find out how I can ensure other users have launchpad.Admin on the view...
[10:34]  * noodles775 keeps looking.
[11:04] <adeuring> noodles775: overall, your code looks good. Just one question: SHouldn't the "with MasterDBPolicy():" be moved from processPositiveAssrtion() to getOrCreateByOpenidIdentifier()?
[11:06] <noodles775> adeuring: yes, you're right.
[11:10] <adeuring> noodles775: OK, so r=me (including aonther nitpick about a typo)
[11:11] <noodles775> Thanks adeuring
[11:12] <adeuring> On call: adeuring || reviewing: - || queue: [] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[11:24] <noodles775> adeuring: the other one is ready now if you've time: https://code.edge.launchpad.net/~michael.nelson/launchpad/db-598464-priv-xmlrpc-access-getOrCreate/+merge/29059
[11:45] <adeuring> noodles775: I'll look
[12:09] <adeuring> noodles775: again, a nice branch. But I think it is a bit odd to have a class PersonSetAPIView (ie.e.,a quite generic name) where the method getorCreateByOpenIDIdentifier() unconditionally providesthe rationale "software centerpurchase" as a account creation rationale. Shouldn't we either make the class name more specific or provide the rationale as a parameter of the method?
[12:10] <adeuring> (or use a method name like getOrCreateForSoftwarePurchase())
[12:16] <rinze> adeuring, can you review a oneliner (testfix) ?
[12:16] <adeuring> rinze: sure
[12:16] <rinze> https://code.edge.launchpad.net/~jelmer/launchpad/fix-db-devel/+merge/29070
[12:18] <rinze> a warning about the completeness of the dailybuilds feature was added recently, I suspect that broke it
[12:19] <adeuring> rinze: well, the change looks straightforward, r=me.
[12:19] <rinze> adeuring: Thanks
[12:45] <noodles775> adeuring: yes - I thought the same when I pushed the last rev. So I'd be keen to change the application name... most of the other private xmlrpc apps have names like 'ICodehostingApplication' identifying who uses them.
[12:46] <bac> hello adeuring.  busy review day?
[12:46] <adeuring> bac: quite intersting branches from noodles775 :)
[12:46] <noodles775> So perhaps, ISoftwareCenterAgentAPI, http://..../softwarecenteragent etc.?
[12:47] <adeuring> noodles775: ok, so something like s/Person/Shopper/ ?
[12:47] <bac> yes, i saw.  looks like fun...
[12:47] <adeuring> ...in the app name?
[12:47]  * bac relocates
[12:48] <noodles775> The app name usually matches the API, so perhaps ISoftwareCenterAgentApplication...
[12:50] <adeuring> noodles775: ah, right. so, that, combined with a method name like getOrCreateShopCustomer()?
[12:50] <noodles775> Sounds good.
[12:50] <adeuring> noodles775: ok, r=me
[12:57] <noodles775> w 5
[13:55] <henninge> jtv: your reply looks good but you did not push your changes. Did you rename "get_all_important_translations" ?
[13:55] <jtv> henninge: knew I forgot something!  hang on
[13:55] <jtv> (Well, I didn't know you'd be this quick so technically I just hadn't gotten around to it yet)
[13:56] <henninge> jtv: I guess you'd like to be through with this before the game ... ;-)
[13:56] <henninge> jtv: also, since this branch only contains tests, it should not need a full test suite run, should it?
[13:57] <jtv> Thanks for reminding me of the game.  :-)  I started incredibly early today, so don't think I'll do much after.
[13:58] <jtv> henninge: that's the theory.  Practice doesn't always follow theory.  :)
[13:58] <henninge> jtv: one more branch after this and we can merge db-stable again?
[13:59] <jtv> henninge: yes, that's D.'s use-upstream-translations branch.
[13:59] <jtv> That will also enable us to run the "mirror" tests that I commented out.
[14:00] <jtv> henninge: oh, and I snuck something extra in: I defined flushing orders when disabling TMs
[14:00] <jtv> Push is underway.
[14:01] <henninge> Ah, once bitten ...
[14:04] <jtv> henninge: push is done, diff updated...
[14:07] <jtv> bac, got time for this one? https://code.launchpad.net/~jtv/launchpad/bug-181254/+merge/29056
[14:09] <bac> jtv: yes, please add to topic
[14:10] <jtv> bac: "reviewing" or "queue"?  :-)
[14:13] <bac> queue, of course
[14:14] <jtv> awwww
[14:15] <henninge> jtv: what?
[14:15] <henninge> oh, ok
[14:37] <jtv> bryceh: thanks for going the extra km in cleaning up the branch...  this is to let you know that I'm approving and you should still be able to land
[14:39] <jtv> henninge: my branch ok now?
[14:39] <henninge> jtv: most likely ... ;)
[14:39] <jtv> the suspense is killing me!
[14:42] <henninge> and
[14:42] <henninge> the
[14:42] <henninge> winner
[14:42] <henninge> is
[14:43] <henninge> ...
[14:43] <henninge> jtv!
[14:43] <henninge> jtv: r=me
[14:43] <henninge> thanks, looks very good ;-)
[14:43] <henninge> but now that you changed actual code, you will have to run it through ec2 :-P
[14:44] <bac> henninge:  which jtv branch did you review?
[14:44] <henninge> jtv: oh, not the one he asked about today.
[14:45] <henninge> bac: ^
[14:45] <henninge> this is three days old.
[14:45] <henninge> https://code.edge.launchpad.net/~jtv/launchpad/recife-limited-setcurrenttranslation/+merge/28715
[14:45] <bac> oh, ok. henninge just trying not to duplicate work
[14:49] <jtv> henninge: thank you for your kind approval...  I'm out of here!
[14:50] <jtv> bac: I'll keep an eye out for any questions you may have, but be aware that Holland is about to face Brazil and I may be distracted or sobbing uncontrollably.  :)
[14:50] <bac> jtv:  good luck.  you'll need it.
[14:51] <jtv> indeed!
[14:51] <bac> jtv: can you keep us posted here?
[14:51]  * bac is surprised there has not been more WC chatter
[14:51] <jtv> bac: ok...  my first time as a commentator
[14:52] <bac> i watched the ger-eng game on univision.  having spanish play-by-play was much less annoying
[14:52] <bac> gooooooooooooooooooooooooooool alemania!
[14:52] <henninge> YEah!
[14:52] <henninge> ;-)
[14:52]  * jtv relocates to TV
[14:52] <henninge> jtv, danilos: Err, the potemplate api branch was never approved.
[14:53] <henninge> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423
[14:58] <danilos> henninge, isn't there a resubmitted MP which jtv approved?
[14:58] <jtv> ?
[14:58] <henninge> danilos: I hope there is
[14:58] <jtv> (wc commentary: Brazilian players looked nervous and tense... is their national anthem particularly difficult like the Argentinian one?)
[14:59] <henninge> adiroiban: Hi!
[14:59] <jtv> (the Dutch players just unfolded a large banner "say no to racism"—a superfluous statement given the diverse composition of the team)
[15:00] <adiroiban> henninge: hi
[15:00] <henninge> adiroiban: which is the MP that jtv approved for the POTemplate API?
[15:01] <henninge> because this one isn't:
[15:01] <henninge> https://code.edge.launchpad.net/~adiroiban/launchpad/bug-525371/+merge/25423
[15:01] <adiroiban> henninge: Danilo was the one doing the final review
[15:02] <henninge> adiroiban: hm, it was landed as [r=jtv], though.
[15:02] <jtv> I did approve that one, didn't I?
[15:03] <jtv> bac: so far the pressure went straight onto the brazilians
[15:03] <bac> jtv: i don't know what that means.
[15:03] <adiroiban> ah... yep. Danilo did approve it first... but then we decide that it requires more work and the jtv was the one to appove and land it
[15:03] <bac> jtv: make lint on your branch shows a lot of stuff...mostly valid i think
[15:03] <bac> assigned but unused variables
[15:03] <jtv> bac: it means the Dutch have had the upper hand so far.
[15:04] <bac> jtv: goals only, please.  :)
[15:05] <jtv> bac: goals only?  The violence may be more interesting... they've deliberately taken down 3 dutch players in 5 minutes and they seem to know the referee will let it go.
[15:05] <henninge> adiroiban: only the MP does not show that. Wonder why. But if nobody objects to you story ... ;-)
[15:05] <jtv> bac: those variables were in there before...  they're not mine but they serve an explanatory role.
[15:06] <adiroiban> henninge: I guess that jtv forget to update the MP
[15:06] <henninge> adiroiban: yes he does forget things once in a while ... ;-)
[15:06] <jtv> adiroiban: I guess...  I may have told you by IRC
[15:08] <henninge> jtv: lucky sods
[15:09] <jtv> henninge: they wouldn't have gotten anywhere so far without unpunished fouls...  hope the game doesn't turn bilaterally ugly
[15:09]  * henninge just looked up sods ... probably not the term I meant to choose.
[15:10] <jtv> bac: Brazil just scored
[15:10] <henninge> awwwww
[15:11] <henninge> jtv: come on! We have a date in the final!
[15:11] <jtv> I hope so...
[15:12] <bac> jtv: imagine a pick up match between you and stevea vs. ursula and salgado.  you know how i'd bet...
[15:15] <jtv> bac: are you okay with those explanatory variables or do you want a cleanup round?
[15:17] <bac> jtv: it's up to you.  i don't think they add much by way of explanation.
[15:17] <jtv> bac: hell, I don't want the noise... I'll take care of it.
[15:18] <jtv> bac: the match may not be worth following with the referee already having made up his mind
[15:18] <jtv> open, nonfunctional violence by the brazilians now... no yellow card
[15:19] <bigjools> hi bac, can you do a small branch please?
[15:19] <bac> bigjools:  sure, next.
[15:19] <bigjools> oh abel is free
[15:19] <bigjools> I think?
[15:19] <bigjools> I don't mind who does it :)
[15:20]  * bigjools files MP
[15:21] <bac> jtv: the comment and code don't seem to match:
[15:21] <bac>  # Strip the last column off the result; it's just always True.
[15:21] <bac> 146	+ return set(result)
[15:21] <bac> s/146 +//
[15:21] <jtv> bac: yikes!  that comment is from an older iteration... I'll take it out
[15:21] <bac> thx
[15:30] <bac> jtv: r=me
[15:31] <jtv> bac: great, thanks!
[15:34] <bac> bigjools:  r=me
[15:34] <bigjools> bac: easiest soyuz branch evar, thanks )
[15:34] <bac> bigjools:  that's the truth!
[15:37] <jelmer_> bac_: Hi!
[15:37] <jelmer_> :-)
[15:37] <bac> jelmer_:  how are we supposed to find you with a new irc nick every day?  :)
[15:37] <jelmer_> sorry :-)
[15:38] <bac> jelmer_:  looking at your MP, i wonder about the import for Dsc
[15:39] <bac> jelmer_:  on our machines where this script runs do we really find Dsc in both places?
[15:39] <jelmer_> bac: Ah, I forgot to document that one.
[15:39] <jelmer_> bac: Newer versions of python-debian have renamed the upstream package from debian_bundle to debian
[15:39] <jelmer_> it still provides debian_bundle but will raise a DeprecationWarning
[15:40] <jelmer_> that warning causes various tests to break
[15:40] <bac> jelmer_:  and our machines are in flux between the two?
[15:40] <bac> or is it a dev vs production issue?
[15:40] <jelmer_> bac: Maverick has the newer version of python-debian, and we'll hopefully include a more up to date version in the PPA as well soon.
[15:42] <bac> jelmer_:  ok, but eventually we'll all find it in one place.  how about an XXX comment for cleaning up the import when things stabilize?
[15:43] <jelmer_> bac: That's a good idea.
[15:45] <bac> thanks jelmer_ .  r=bac
[15:45] <jelmer_> bac: Thanks!
[15:55] <bac> hey bigjools it'd be good to tell your reviewer if you're looking for an RC.  it really shouldn't matter but i would look at things much harder if i knew it was an rc candidate.
[15:55] <bac> otoh, i should've asked...
[15:55] <bigjools> well, I don't understand how that affects the review
[15:56] <bac> bigjools:  ideally it shouldn't.
[15:56] <bac> they should all be super stringent
[15:56] <bigjools> but I should have said - in fact I normally do
[15:56] <bigjools> so sorry 'bout dat
[15:56] <bac> but in reality i know i'd pay a lot more attention
[16:10] <henninge> bac: goooooooooooal!
[16:27] <henninge> bac: gooooooooooooooal!
[16:27] <henninge> NED - BRA 2:1
[16:28] <henninge> jtv is probably dancing Samba ... ;-)
[16:28] <jtv> henninge: good thing I'm not on Mumble
[16:28] <henninge> yeah, it'd probably burst my earphones
[16:30] <jtv> My God, that was a deserved red
[16:30] <henninge> I say
[16:30] <jtv> Stabbing your cleats right into the back of a prone man's leg...
[16:33] <jtv> Damn, that red card should've gone to Caca
[16:34] <henninge> jtv: that was yellow
[16:34] <jtv> yes, but what Caca did deserved red.
[16:35] <henninge> jtv: did you realize that we will be watching the final together in Prague?
[16:35] <jtv> henninge: I'm being very careful in my use of my country's very rich vocabulary for your countrymen  ;-)
[16:35] <jtv> Would be fun to watch de-nl together!
[16:36] <henninge> What I expect! ;)
[16:37] <henninge> They need higher goals for those kind of shots ...
[16:38] <jtv> We got lucky there...
[16:40] <jtv> ...and not there
[16:50] <henninge> jtv: Congratulations!
[16:50] <jtv> henninge: thanks!  Now you do the same and we'll meet in Prague!
[16:51] <henninge> jtv: We'll meet anyway ... :-P
[16:52] <jtv> :D
[16:56] <jtv> henninge: Sneijder just explained why he kept shooting into the goalie's arms: he knew the guy isn't so good at catching close to his body.
[16:57] <jtv> The surprising thing is I don't hear any vuvuzelas outside...  Excellent noise isolation my dad has.
[16:58] <jtv> bac: Holland passes 2—1, deservedly.  Loyalty aside I was sort of rooting for Brazil until the first minutes when the fouls started.
[16:59] <bac> jtv:  nice
[17:02] <henninge> jtv: enjoy your weekend! I'll be going there now, too. ;-)
[17:02] <jtv> henninge: thanks, you too... have a lemonade on us!
[17:02] <henninge> I will ! ;-)
[17:12] <bryceh> jtv, excellent, thank you!
[17:12] <jtv> bryceh: whoa, took me a while to figure out what that was about.  :)
[17:51] <jtv> bac: apparently the Brazilian fans were good sports and many said the Dutch team was simply better in the second half.  I think the referee let a few Dutch fouls slip later on to compensate for some really bad ones he let go from the other side earlier.
[17:53] <jtv> I wish we'd get instant replay soon... bad calls breed lots of resentment on both sides that they take out on each other.
[18:26] <sinzui> Hi bac. I have a branch for review. My apologies for the diff which I think is hard to read because of the many small changes needed to expose summary and copyright for packages.
[18:29] <bac> sinzui: ok
[18:44] <bac> sinzui: i get errors in test_highlighted_copyright_is_None
[18:44] <sinzui> :(
[18:46] <sinzui> I get them too
[18:46] <sinzui> one moment
[18:47] <sinzui> ah
[18:49] <sinzui> bac: The setup is wrong. The copyright must be set to None. Our factory is better than our data, or some of our uploaders
[18:50] <sinzui> bac: this is the fix for when there is no copyright file: http://pastebin.ubuntu.com/458417/
[19:11] <bac> sinzui: if you go to https://launchpad.dev/ubuntu/warty/+source/a52dec you'll see a warning that there is no current release
[19:11] <sinzui> yes, that is true
[19:11] <bac> clicking on the +changelog gives a nice but empty page
[19:11] <bac> +copyright oopses
[19:11] <sinzui> bac: sample data is very very bad
[19:11] <sinzui> remember that we have to use the factory to make SPPH for everything, that page is ehy
[19:12] <bac> so if it were fully fleshed out the oops would not happen
[19:12] <sinzui> pmount and mozilla-firefox have some data
[19:13] <bac> sinzui: ok.  i just want assurance that we're being suffiently defensive
[19:13] <sinzui> The page told the truth about no release, and we do not make links to impossible packages
[19:13] <sinzui> however, I will make copyright not oops when someone is being bad
[19:17] <sinzui> bac: this is the fix to see info that there is no copyright: http://pastebin.ubuntu.com/458422/
[19:17] <bac> great
[19:27] <mars> sinzui, ping, when you have a moment, could you please review https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ?
[19:28] <sinzui> mars: I do not think that will work
[19:28] <mars> neither do I
[19:28] <mars> '|' is not OR, is it
[19:28] <sinzui> mars tales is testing None or something
[19:29] <sinzui> False is something
[19:29] <mars> eewww, even worse
[19:29]  * sinzui thinks if there is a tales formatter for false>None
[19:29] <mars> surely there is some way to just do a binary expression in TALES?
[19:30] <sinzui> no
[19:30] <sinzui> but you can do:
[19:30] <sinzui> python: is_edge or is_lpnet
[19:30] <mars> do the variables come through?
[19:30] <mars> from TALES back to Python ?
[19:31] <sinzui> They do in this case I believe. We scoped them to the entire template
[19:31] <mars> simple, testing now
[19:31] <sinzui> mars, if not you need to do python: config.launchpad.is_edge ??
[19:33] <mars> sinzui, python: is_edge or is_lpnet works
[19:34] <sinzui> rock
[19:36] <mars> branch scanner is slow today
[19:36] <mars> pushed, just waiting for the update
[19:38] <mars> updated
[19:40] <mars> sinzui, r=you?
[19:40] <sinzui> mars: yes r=me
[19:41] <mars> I wonder if 'ec2 land' understands the release-critical tag
[19:41] <mars> thanks sinzui
[19:42] <mwhudson> mars: it adds [release-critical=] if there's a review with that tag iirc
[19:42] <bac> mwhudson: that is correct
[19:42] <mars> cool
[19:45] <mars> sinzui, could you please hit 'Approve' on the merge proposal?  https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116
[19:48] <mars> bac, care to give a second a review of https://code.edge.launchpad.net/~mars/launchpad/ga-deployment-only/+merge/29116 ?
[19:48] <bac> sure
[19:49] <bac> mars i'm confused
[19:49] <bac> you reviewed your own branch?
[19:49] <mars> bac, I don't know how to formally say 'Curtis reviewed it' - there is no Proxy Review button
[19:50] <bac> mars: why didn't he mark the MP?
[19:50] <mars> I can vote by proxy - why not review by proxy as well?
[19:50] <bac> the commit message is going to be a hoot
[19:50] <mars> :D
[19:50] <bac> [r=mars][rc=mars][bug=mars]
[19:51] <mars> bac, I don't know, he isn't answering
[19:51] <bac> i would actually suggest you get an rc from curtis since he was the last RM.  i'd be very uncomfortable giving myself an rc
[19:51] <mars> bac, I can do the r-c myself
[19:51] <bac> you certainly can.  i think it is a bad idea.  that's just me.
[19:51] <mars> you know, by being Release Manager and all
[19:52] <mars> true, but that is why I asked for your second code review
[19:52] <mars> from a RM perspective, this has to land to fix the lp_lucid builder
[19:52] <mars> so there is no question around the RC
[19:53] <mars> bac, ok, Curtis reviewed it - could you still take a look at the code, just to make sure it's sane?
[19:54] <bac> mars: doing so now
[19:54] <mars> thanks
[19:56] <bac> mars, what about staging and dogfood?
[19:56] <mars> bac, no, we do not want to track the usage of those servers.  They will just pollute the data.
[19:56] <mars> Since they are test environments
[19:57] <bac> mars: fair enough.  r=me
[19:57] <mars> thanks bac