=== vednis is now known as mars
[04:39] <wallyworld_> StevenK: are you able to take another look at those recipe build listing branches? - i've made some changes as suggested by you and tim
[07:42] <jtv> Hi gmb, got a very light review for you, with a bigger one to follow: https://code.launchpad.net/~jtv/launchpad/recife-pre-stats/+merge/42079
=== jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:00] <jtv> No reviewers today?
=== jtv changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [jtv, jtv] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:23] <jtv> stub: are you as busy as henning is?  Need some reviews!
[08:24] <henninge> jtv: what makes you think I am busy? ;-)
[08:24] <henninge> No staging - no QA
[08:26] <jtv> henninge: okay, then I need some reviews!
[08:26] <henninge> let's do one at a time ... ;)
=== henninge changed the topic of #launchpad-reviews to: On call: jtv || Reviewing: - || queue: [jtv, jtv] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:26] <jtv> Yes, let's.  :)
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [ jtv] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:29] <henninge> wow, in-line diffs for updates.
[08:32] <jtv> Yes, we have those now.
[08:43] <henninge> jtv: very nice simplification of the test.
[08:44] <henninge> jtv: I just don't see what you describe as "runs in both old-model and new-model statistics calculation."
[08:44] <jtv> In all honesty, I did it to get the test to start up faster.  :-)
[08:44] <jtv> I clear both flags as an alternative to deleting the message.
[08:46] <henninge> ah! so lines 148-151 could use a comment I think ;)
[08:46] <henninge> jtv: but apart from that, r=me ;)
[08:46]  * jtv looks
[08:46] <jtv> OK, I'll note that.
[08:47] <henninge> thanks
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [ ] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:48] <henninge> ok, the next branch is a bit larger
[08:48] <jtv> Yup, though this pre-branch got it under the 800 line.
[08:49] <jtv> henninge: actually, instead of a comment, how about I just fix getPOFilesWithTranslationCredits?
[08:50] <henninge> in what way?
[08:52] <jtv> By making it look at the translation side that's appropriate for the POFile, rather than always at is_current_ubuntu.
[08:52] <jtv> (In other words, updating it to the Recife model).
[08:52] <jtv> By the way, my router had come back to life but went back into a coma again, so I'm on a slow edge connection today.  Makes it hard to  get to the kanban board. :(
[08:55] <henninge> jtv: so I guess voice chat is out of the question?
[08:55] <jtv> henninge: pretty much :(
[08:56] <henninge> :(
[08:59] <henninge> jtv: if the test works as it is now you should finish the statistics stuff first - although I fear that is all inter-related.
[09:00] <jtv> henninge: might as well finish this while you're reviewing—it's not a big change.
[09:00] <jtv> Better this than forget to fix it!
[09:01] <henninge> that's cool, too
[09:03] <jelmer> g'morning!
[09:03] <henninge> Hi jelmer ;)
[09:03] <jelmer> henninge: Can I add another branch to the queue?
[09:04] <henninge> jelmer: you always can ydd a branch to the queue ... :-P
[09:04] <henninge> s/ydd/add/
=== jelmer changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [jelmer ] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[09:05] <jelmer> henninge: thanks (-:
[09:05] <henninge> jtv: what does that change "Merging preparatory branch" mean? Where do all the property changes come from?
[09:06] <jtv> henninge: I merged the branch you just reviewed, but since that one was branched off db-devel whereas this branch was branched off recife (which in turn was branched off an even older db-stable), it brought in a lot of other changes that won't show up in a diff against db-devel.
[09:07] <henninge> I see
[09:07] <jtv> In other words: irrelevant updates.  :-)
=== gmb` is now known as gmb
=== jtv changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [jelmer, jtv] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[10:24] <henninge> jtv: what's the trick in this?
[10:24] <henninge> 137	+        for count, in IStore(self).execute(query):
[10:24] <henninge> 138	+            return count
[10:24] <henninge> Would [0] not do the same?
[10:24] <henninge> or I think there is even a "first" method.
[10:25] <jtv> Only if I listify first.  I wish I had a really concise way of stating it, but I'm willing to go with any suggestion you may have.  :)
[10:25] <jtv> Oh, first, that might work.
[10:26] <henninge> jtv: and what about "one"?
[10:26] <henninge> I am still trying to figure out why you'd only want the first, or why the query returns more than one.
[10:27] <jtv> There is only one.
[10:28] <jtv> henninge: first() does the trick.  Also magically turns the tuple-shaped row into a single int, which is what I really want.
[10:28] <henninge> jtv: one() does not do that?
[10:29] <jtv> The whole trick to that code was that I had a result set that looked like an iterator over [(n)], and I just wanted the n.
[10:30] <jtv> Oh, hang on, first() did _not_ work.  What I saw was a screenful of errored tests.
[10:31] <jtv> There is no "one" (or first) on this type of result set.
[10:31] <jtv> Duck-typing to the descue.
[10:31] <jtv> Or whatever the opposite of rescue is.
[10:31] <henninge> jtv: what is "this type of result set" ?
[10:32] <jtv> PostgresResultSet.
[10:32] <jtv> Store.execute returns something different than Store.find.
[10:33] <henninge> jtv: it has get_one, though
[10:33] <henninge> http://people.canonical.com/~jkakar/storm/storm.databases.postgres.PostgresResult.html
[10:34] <jtv> ah!
[10:35] <jtv> Now trying return IStore(self).execute(query).get_one()[0]
[10:35] <jtv> That works.
[10:35] <jtv> Thanks.  I always look through the documentation in the source code, but didn't find anything helpful.
[10:41] <jtv> Change pushed.
[10:47] <henninge> cool
[10:53] <jtv> Meanwhile, could anyone else review this other very small branch?  https://code.launchpad.net/~jtv/launchpad/recife-getpofileswithtranslationcredits/+merge/42083
[10:54] <jtv> jelmer, is that something you could do?
[10:55] <jelmer> jtv: Sure, should we trade branches? Mine is very small as well.
[10:55] <jtv> jelmer: sure!
[10:56] <jtv> Got a direct link?  My connection's so slow right now it'd save me a lot of time.
[10:56] <jelmer> https://code.launchpad.net/~jelmer/launchpad/681974-building-commit/+merge/42081
[10:57]  * jtv goes into a frenzy of browsing attempts
=== matsubara-afk is now known as matsubara
[11:02] <jtv> jelmer: good cover letter.  I'm surprised it's the buildd manager confusing the upload processor by moving the file before committing, and not another way around.
[11:03] <jelmer> jtv: This is happening with binary builds that the buildd manager has fetched from the builders.
[11:04] <jelmer> jtv: There is another archive uploader instance that processes the source uploads from users, which the buildd manager then sends off to the builders.
[11:08] <jtv> jelmer: ah so the data flows in the opposite direction from what I thought.
[11:09] <jtv> Depending on what the upload processor's transactions look like and even which isolation level it uses, there may _conceivably_ still be race conditions but even then this should narrow it own.  And I'm not assuming that you got it wrong anyway.  :)
[11:10] <jtv> I'm also assuming that you're not committing in some untenable state, and therefore r=jtv.
[11:12] <jelmer> jtv: Thanks :-)
[11:17] <jelmer> jtv: I don't understand why line 27 of your change is now necessary, how is it related to your other changes?
[11:17] <jelmer> + clauses.append(POTemplate.id == POFile.potemplateID)
[11:19] <jtv> jelmer: yes it is a bit obscure.  It pulls POTemplate into the join.
[11:19] <jtv> I did reference it, but in a nested query.  And so that join doesn't get pulled into the outer query.
[11:19] <jelmer> ahh, I see
[11:20] <jtv> The ORM probably couldn't do that if it wanted, because it'd be ambiguous whether I wanted it in the outer query or just the inner one.
[11:21] <jtv> (In this case it doesn't matter much, but I'm assuming in a hand-wavy way that it might in other cases)
[11:21] <jelmer> jtv: yeah, I can see how that could make a difference.
[11:22] <jelmer> jtv: That was all I had. r=me
[11:22] <jtv> dankuwel
[11:51] <jtv> phew, my router is working again
=== jtv changed the topic of #launchpad-reviews to: n call: henninge || Reviewing: jtv || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== jtv changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: jtv || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[12:11] <jtv> jelmer: if you use "ec2 land," there's no need to include the [r=…] tag etc. in your commit message.  That gets added automatically.
[12:12] <jelmer> jtv: I didn't add anything, ec2 land did!
[12:12] <jtv> oh!
[12:12] <jelmer> I guess it's now modifying the commit message on lp before send it off to ec2.
[12:12] <jtv> there's no stopping progress, I suppose
[12:12] <jelmer> :)
[12:13] <jtv> That could actually be helpful when landing fails because of testfix.
[12:13] <jtv> No endless re-composing your commit message for every landing attempt.
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[12:34] <henninge> jelmer, jtv: thanks guys for taking each other's reviews.
[12:34] <henninge> jtv: r=me with a few comments to consider. ;-)
[12:34]  * jtv jelmer & henninge all bow at each other
[12:34] <jtv> henninge: great, thanks—sorry for the heavy review loa
[12:34] <jtv> d
[12:34] <henninge> :-D
[12:35] <henninge> jtv: np. I will see that I get the migration script run now that staging is alive again. Or did you already take action?
[12:35] <jtv> henninge: no, I didn't know it was up!
[12:43] <jtv> henninge: you're right that there's no point to renaming that method I'm replacing—I just did it to keep your diff to a reasonable size, and left cleaning up the now-unused method as one of the TODO items.
[12:43] <jtv> I can remove it now if you're willing to rubber-stamp it—there are no calls to it.
[12:44] <henninge> jtv: that's what I meant - no calls. ;-) rs=me
[12:46] <jtv> Thanks.
=== mrevell is now known as mrevell-lunch
=== NCommand1r is now known as NCommander
=== matsubara is now known as matsubara-lunch
[14:05] <abentley> henninge: Could you please review https://code.launchpad.net/~abentley/launchpad/bzr-2.2.2/+merge/41981?
[14:13] <henninge> abentley: r=me
=== mrevell-lunch is now known as mrevell
=== salgado is now known as salgado-lunch
=== bac` is now known as bac
=== matsubara-lunch is now known as matsubara
=== salgado-lunch is now known as salgado
[15:46] <jam1> henninge: can I get a quick review of: https://code.edge.launchpad.net/~jameinel/launchpad/lp-service/+merge/42129
[15:46] <jam1> That was already merged and approved into db-devel, but I had meant to target devel
=== jam1 is now known as jam
[15:47] <jam> hmm... the diff appears to be empty, maybe db-devel has already become devel?
[15:48] <henninge> jam: the branch's status is "Merged"
[15:48] <jam> anyone know when that happened? l-osas said the functionality wasn't present last Wed
[15:49] <jam> henninge: right, it *was* merged into db-devel, but I meant to target 'devel'
[15:49] <henninge> jam: when did that happen? Merging into db-devel, I mean?
[15:49] <jam> I thought I saw that db-devel was going to be merged/pushed over devel, but I don't know for sure that happened
[15:49] <jam> henninge: *My* branch was merged into db-devel probably 2 weeks ago, or so
[15:49] <henninge> jam: it happened on the 16th
[15:49] <jam> I didn't realize I had the wrong target unti the 24th
[15:50] <jam> (well, I didn't realize until today :)
[15:50] <henninge> jam: devel and db-devel are merged before each roll-out.
[15:50] <jam> I was trying to get someone to use the new code, and they said it didn't work on the 24th.
[15:50] <jam> but right now, it appears to be properly merged into devel
[15:50] <jam> which is confusing me
[15:53] <henninge> jam: it was merged on the 9th as r9951, r9973 was rolled out (if I remember correctly)
[15:53] <henninge> jam: so, yes, your code is deployed.
[15:54] <jam> henninge: thanks. Now I have to figure out why it wasn't available on qastaging...
[15:54] <jam> or if something else weird is going on
[15:54] <jam> anyway, thanks for your help
[15:54] <henninge> jam: it is there for sure (unless it was reverted by a leter revision)
[15:54] <henninge> s/leter/later/
[15:55] <jam> henninge: I just cleared out the mp, sorry about the noise
[16:28] <salgado> hi henninge.  can you review a trivial one for me?
=== henninge changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[16:29] <henninge> salgado: sure
[16:29] <henninge> last onen for today ;)
[16:29] <salgado> henninge, https://code.launchpad.net/~salgado/launchpad/refactor-blueprints-tests/+merge/42139
[16:29] <salgado> thanks!
[16:52] <bac> hi jcsackett, sorry for taking so long but i think your branch looks great
[16:52] <jcsackett> bac: cool! glad to hear it.
[16:59] <henninge> salgado: r=me ;)
[17:00] <salgado> thanks henninge
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[17:17] <salgado> abentley, how about a trivial one to start the day? ;)
[17:17] <salgado> https://code.launchpad.net/~salgado/launchpad/expose-blueprint-dependencies/+merge/42145
=== benji is now known as benji-lunch
[18:05] <abentley> salgado: sorry, was lunching.
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: salgado || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[18:06] <salgado> abentley, np.  I've just pushed another tiny change there, though, so you might want to wait for the diff to update
[18:09] <abentley> salgado: Is this test refactoring something I should review?
[18:10] <salgado> abentley, the ones to use assertContentEqual?  Yes, although they're not related to the changes on the branch I thought you wouldn't mind reviewing them together?
=== benji-lunch is now known as benji
[18:11] <abentley> salgado: Sure.  I just noted they were merged from another branch, so i wasn't sure.
[18:11] <salgado> but the "merge test refactoring from other branch" has landed on devel already
[18:12] <salgado> abentley, I just had to merge that directly from the branch because at that time it wasn't on devel
[18:13] <abentley> salgado: So you could have set that other branch as a prerequisite branch.
[18:14] <salgado> abentley, yeah, although it wasn't really.  I just merged to make sure there'd be no conflicts
[18:14] <abentley> salgado: But now that you've merged it, you can't land this branch until that one's approved.
[18:15] <salgado> abentley, but the other one has landed already
[18:15] <abentley> salgado: now, it has, and I see that in the updated diff.  I was looking at an older one a minute ago.
[18:16] <salgado> it landed 15 mins ago or so.   but 1h ago it hadn't landed and I merged to make sure there'd be no conflicts
[18:16] <salgado> abentley, sorry for the confusion
[18:21] <abentley> salgado: do you know about lp.testing.ws_object?  It's a more general version of getSpecOnWebservice.
[18:21] <salgado> abentley, nope, I didn't know about that
[18:21]  * salgado checks
[18:23] <abentley> salgado: The functionality looks fine.  At 23, you should use a multi-line import.  r=me.
[18:25] <salgado> abentley, I could use that in getSpecOnWebservice() but I don't see much benefit in doing so
[18:25] <salgado> abentley, oh, so multi-line imports are to be used even when they're not necessary?
[18:25] <abentley> salgado: yes.
[18:25] <abentley> salgado: I don't think you need getSpecOnWebservice at all.
[18:26] <salgado> you mean to inline its functionality everywhere?
[18:26] <abentley> salgado: I mean use ws_object instead.
[18:27] <salgado> that's basically the same as I still need to get the Launchpad instance.  although I could store that as an instance variable on the testcase class
[18:27] <salgado> that doesn't sound like a good idea
[18:28] <salgado> abentley, or do you mean something else?
[18:29] <abentley> salgado: It just seems like wasted effort to have different functions for getting objects via the web service, one for each type  of object, when you could just have one.
[18:32] <abentley> salgado: If you'd like to enhance ws_object so that the launchpad parameter is optional, that would be a nice improvement for everyone.
[18:33] <salgado> abentley, I'm not sure that'd help in this case as these tests need a 'devel' Launchpadlib and I don't think that should be the default
[18:33] <lifeless> you could curry something
[18:34] <salgado> curry?
[18:37] <abentley> salgado: so we could have a parameter on ws_object to specify the launchpadlib version.  That would also be helpful for everyone.
[18:38] <abentley> salgado: I don't think wanting to vary the launchpadlib version is a good reason to have different implementations of ws_object for different objects.
[18:39] <abentley> salgado: At most, it's a reason to have different versions of ws_object for different launchpadlib versions.
[18:39] <salgado> I don't think so either; I'm just saying that the current one is not a good fit
[18:39] <salgado> but I can change it as you suggest
[18:40] <abentley> salgado: I've already approved this, so it's only a suggestion to consider.  I just think it's better to spend effort in ways that help the whole team, where we can.
[18:41] <salgado> agreed, and that's why I'm happy to do the change you suggested. :)
[18:42] <abentley> salgado: cool.
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== matsubara is now known as matsubara-afk
[19:03] <salgado> abentley, the annoying thing is that the launchpad object is the first argument to ws_object() and it doesn't make sense to make both arguments optional so I'd have to change the method's signature and update all the callsites. :(
[19:04] <abentley> salgado: It's true.
[19:05] <salgado> abentley, I think I'll save this work for later, then
[19:05] <abentley> salgado: sure.
[20:06] <jcsackett> abentley: have one to throw on your queue. https://code.edge.launchpad.net/~jcsackett/launchpad/distribution-enum-selection-677536/+merge/42161
=== jcsackett changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: - || queue: [jcsackett] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[20:06] <abentley> jcsackett: ack
=== salgado is now known as salgado-afk
=== Guest66935 is now known as EdwinGrubbs
[21:31] <abentley> jcsackett: r=me
[21:31] <jcsackett> abentley: thanks!
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[21:38] <gary_poster> abentley: 4 lines when you get a chance: https://code.launchpad.net/~gary/launchpad/ec2lucid/+merge/42179
[21:39] <abentley> gary_poster: r=me
[21:39] <gary_poster> thanks abentley
=== jcsackett_ is now known as jcsackett
=== abentley changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [] ||  This channel is logged:  http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews