[07:10] <jml> hello!
[07:10] <jml> fun & short patch here: https://code.edge.launchpad.net/~jml/launchpad/import-fascist-tweaks/+merge/16560
[07:13] <al-maisan> hello jtv, bugging you re. https://code.edge.launchpad.net/~al-maisan/launchpad/pending-jobs-499861/+merge/16550 as requuested :)
[07:22] <al-maisan> hello jml, could we trade reviews please?
[07:23] <al-maisan> jml: I am looking at your import fascist branch right now :)
[07:24] <jml> al-maisan, thanks.
[07:24] <jml> al-maisan, which branch do you want me to review?
[07:24] <al-maisan> jml: https://code.edge.launchpad.net/~al-maisan/launchpad/pending-jobs-499861/+merge/16550
[07:24] <jml> al-maisan, will do.
[07:24] <al-maisan> jml: thanking you.
[07:26] <wgrant> Does anybody know what happened about bug #500018?
[07:27] <al-maisan> wgrant: I guess nothing so far
[07:29] <wgrant> al-maisan: OK, there was some discussion about finding somebody to cowboy it.
[07:29] <al-maisan> wgrant: I'll need to touch base with jelmer and/or get the hole plugged myself
[07:30] <al-maisan> wgrant: I will start on it in 20 minutes or so.
[07:36] <al-maisan> jml: the imports code you removed from lib/lp/soyuz/model/archive.py : were they just superfluous?
[07:36]  * jml looks
[07:37] <jml> al-maisan, they were, after I deleted the two lines like:
[07:37] <jml> -        source_package_name = getUtility(ISourcePackageNameSet)[source_name]
[07:37] <jml> al-maisan, those lines assigned to an unused local variable
[07:37] <jml> al-maisan, and have no obvious side-effects
[07:37] <al-maisan> jml: I see, thanks!
[07:42] <al-maisan> jml: the changes in lib/lp/scripts/utilities/importfascist.py look reasonable, did you test them in any way?
[07:43] <jml> al-maisan, the fascist itself doesn't have tests that I can find.
[07:43] <jml> al-maisan, I've been running the fascist locally.
[07:43] <al-maisan> jml: but I guess running anything will exercise it
[07:43] <jml> al-maisan, correct.
[07:58] <jml> al-maisan, I've hammered & sent out an email reply to your review
[07:58] <jml> al-maisan, to your patch, rather.
[07:59] <jml> al-maisan, I'm going to get some food now, but I'll be back in a bit to handle round #2
[07:59] <al-maisan> jml: thanks, I am just running "bin/test -vv -t mail -t archive" in your branch to be on the safe side
[08:02] <jml> al-maisan, heh. thanks. though I was certainly going to submit it via ec2 test in any case.
[08:06] <al-maisan> jml: FYI, the test on line 2407 in doc/archive.txt is failing
[08:07] <al-maisan> http://pastebin.ubuntu.com/345778/
[08:08] <jml> al-maisan, I guess those lines do have side effects!
[08:09] <al-maisan> jml: yeah .. either deliberate or inadvertent ones :P
[08:13] <jml> al-maisan, I see what's going on....
[08:13] <jml> al-maisan, getting the sourcepackagename object there gives a good error for those functions if it doesn't exist
[08:14] <jml> but that's all it does
[08:14] <jml> I'll remove the local assignment to keep pyflakes happy and add a comment saying why we do something that appears useless at first glance.
[08:14] <al-maisan> jml: ah, OK, it's a deliberate point of failure.
[08:15] <jml> al-maisan, http://pastebin.ubuntu.com/345781/
[08:16]  * jml really dining now.
[08:18] <al-maisan> jml: with the change in the pastebin above: r=me
[08:19] <al-maisan> hmm .. clicking on "Claim review" on https://code.edge.launchpad.net/~jml/launchpad/import-fascist-tweaks/+merge/16560 results in an OOPS
[08:20] <al-maisan> .. a reproducible one
[08:20] <al-maisan> http://pastebin.ubuntu.com/345785/
[09:03] <jml> al-maisan, I've heard mention of a similar bug...
[09:04] <al-maisan> jml: after refreshing the page, the review would appear to be claimed by me and I was able to finish the review approval workflow.
[09:06] <jml> al-maisan, hmm. I can't find a record of that in the bug tracker.
[09:06] <al-maisan> jml: would you like me to add a bug?
[09:06] <jml> al-maisan, yes please. assign it to thumper, I reckon.
[09:06] <al-maisan> jml: will do.
[09:08] <jml> al-maisan, I'm just about to sign off for the year. Do you want to go one more round on the buildJobQuery branch first?
[09:10] <al-maisan> jml: ideally yes, I am about to reply to your review email. It will take another 5 minutes or so.
[09:12] <jml> al-maisan, no worries.
[09:16] <al-maisan> jml: reply sent.
[09:17] <jml> al-maisan, thanks. 
[09:19] <adiroiban> al-maisan: hi. Do you have time for landing this branch https://code.edge.launchpad.net/~adiroiban/launchpad/bug-492375/+merge/16279 ? Thanks! I have done a full test and everthing was ok.
[09:20] <al-maisan> adiroiban: yes, I'll do it in the course of the next hour or so..
[09:22] <al-maisan> jml: fixes to the branch under review so far: http://pastebin.ubuntu.com/345813/
[09:37] <jml> al-maisan, replied.
[09:37] <jml> al-maisan, if the classProvides / interface thing is too much, I guess adding a XXX would be OK, but I think that for this particular area of work, we should be trying very hard to avoid tech debt.
[09:37] <jml> al-maisan, (he says by way of a postfix)
[09:38] <jml> al-maisan, good night. I'll see you in the new year.
[09:39] <al-maisan> jml: thanks for looking at this, have a great break!
[12:00] <jtv> henninge: I'm working on an MP for our buildfarm work... you up for a review?
[12:00] <jtv> al-maisan: which reminds me... did you find a reviewer for yours?
[12:01] <henninge> jtv: sorry no, my year is almost done.
[12:01] <al-maisan> hello jtv, yes, jml reviewed the branch but it would be good if you could rubber-stamp it once I have the changes he requested in place.
[12:01] <al-maisan> jtv: how long will you be available today?
[12:01] <jtv> al-maisan: ok, I'll be around for a few more hours
[12:02] <al-maisan> jtv: I should have something in the next hour or so..
[12:02] <jtv> I'm just putting up my MP, though if you land an extension to IBuildFarmJob first, it will break my test.  :-)
[12:02] <al-maisan> henninge: how about you? Are you working normal hours today?
[12:03] <al-maisan> jtv: that's a concern .. why would it break the tests though ?
[12:04] <al-maisan> there is a minimal implementation of the 3 methods added to the interface
[12:04] <jtv> al-maisan: because my test verifies that my IBuildFarmJob implementation really does provide all of the interface's API
[12:04] <jtv> oh, if there is a minimal implementation in BuildFarmJobBase then it won't break things
[12:04] <henninge> al-maisan: no, got the afternoon off.
[12:05] <al-maisan> OK
[12:55] <jtv> al-maisan: my mp is up
[12:55] <jtv> al-maisan: want to review each other mutually?
[13:04] <al-maisan> jtv: I can review your branch but mine is not ready yet
[13:04] <jtv> al-maisan: then you can't review mine either, because you've got other things to do.  :-)
[13:05] <al-maisan> jtv: if you give me 30 minutes it may work out for both of us.. 
[13:05] <jtv> \o/
[13:45] <al-maisan> jtv: OK .. ready now .. please note that jml already approved of the branch (https://code.edge.launchpad.net/~al-maisan/launchpad/pending-jobs-499861/+merge/16550) ..
[13:45] <jtv> al-maisan: I was just reading up on the preceding review thread
[13:45] <al-maisan> .. provided I fix a few things
[13:46] <al-maisan> the fixes are here: http://pastebin.ubuntu.com/345918/
[13:46] <al-maisan> .. and I am just pushing the branch
[13:47] <jtv> al-maisan: btw what exactly does virtualization mean here?  That the slave must be in a vm?
[13:47] <al-maisan> yes
[13:48] <jtv> al-maisan: that'll become relevant when I update the branch I just put up for review to match the one you're having reviewed.  :-)
[13:49] <al-maisan> jtv: as I said before: if your branch lands first I will add a perfunctory getPendingJobsQuery() to your class in my branch to avoid breakage
[13:50] <jtv> oh, I read that differently earlier... thanks.
[13:59] <jtv> al-maisan: "get" as a bit generic and over-used as an opening verb for method names...  how about composePendingJobsQuery to reflect that it puts together a textual query?
[13:59] <jtv> (This is a verb I use a lot in these situations)
[13:59] <al-maisan> jtv: OK .. that's fine.
[14:00] <jtv> thanks
[14:05] <jtv> al-maisan: and in its docstring, the docs for min_score and processor end without punctuation.
[14:05] <al-maisan> jtv: will fix that.
[14:05] <jtv> thx
[14:07] <jtv> al-maisan: in the test you use processor='386'...  not 'i386'?
[14:07] <al-maisan> yes, you can see it in the psql excerpt I have in the test as well
[14:08] <jtv> oh well, not our problem
[14:08] <al-maisan> jtv: line 232 for example
[14:11] <jtv> al-maisan: here's my mp btw: https://code.edge.launchpad.net/~jtv/launchpad/db-bug-499404/+merge/16564
[14:11] <al-maisan> jtv: thanks -- I am free to look at it now .. sorry for the delays
[14:12] <jtv> stop apologizing!
[14:12]  * al-maisan apologizes for apologizing ;P
[14:13]  * jtv glares back
[14:13] <al-maisan> jtv: the changes you suggested: http://pastebin.ubuntu.com/345928/
[14:14] <jtv> looks fine
[14:15] <al-maisan> jtv: thanks!
[14:15] <jtv> The comment you added in TestBuildPackageJob.setUp chains two "so that"s
[14:15] <al-maisan> Oh!
[14:16] <jtv> And I see that elsewhere you use x86 instead of 386 (or i386 or IA32 or 80x86...)
[14:17] <jtv> But not worth changing that last bit.
[14:17] <al-maisan> hmm .. ok
[14:18] <jtv> I didn't know about classProvides btw; it's a bit meta but it does look useful
[14:18] <jtv> Wonder how we test for it.
[14:20] <al-maisan> jtv: the tests in the branch exercise the method specified in the interface mentioned in classProvides()
[14:21] <jtv> al-maisan: I was thinking of "make a test verify that the class really provides the complete interface"
[14:22] <al-maisan> jtv: it's only one method in this case
[14:23] <jtv> True...  sometimes though I come across methods whose interface definitions and model-class definitions have grown apart
[14:23] <jtv> Imagine that you add a parameter to that one method: it'd be nice if all classes had tests verifying that they support the new parameter.
[14:24] <al-maisan> jtv: hmm ... I see what you mean .. /me starts grep'ing for classProvides in tests
[14:29]  * al-maisan finds from canonical.launchpad.webapp.testing import verifyObject
[14:30] <jtv> Who knows, it might work on classes and classProvides as well as objects and implements
[14:30] <al-maisan> hmm..
[14:30] <al-maisan>     def test_providesInterface(self):
[14:30] <al-maisan>         """Ensure that BranchDiffJob implements IBranchDiffJob."""
[14:30] <al-maisan>         verifyObject(
[14:30] <al-maisan>             IBranchDiffJob, BranchDiffJob.create(1, '0', '1'))
[14:30] <jtv> Yes, you normally pass it an interface and an object
[14:31] <jtv> I've been looking for mentions of interfaces that this is used for in tests, but zip so far
[14:32] <jtv> ah! lib/lp/code/model/tests/test_diff.py
[14:32] <jtv> verifyObject(IMyInterface, MyClass)
[14:32] <al-maisan> yep
[14:38] <al-maisan> jtv: the changes discussed: http://pastebin.ubuntu.com/345943/
[14:39] <jtv> al-maisan: perfect, thanks
[14:39] <al-maisan> jtv: thank *you*
[14:39] <jtv> I'm about ready to give that rubber stamp now.  :-)
[14:39] <al-maisan> jtv: you are one picky reviewer ;)
[14:39] <jtv> yup
[14:40]  * al-maisan feels compelled to type "meine Rache wird bitter sein" for some reason :-)
[14:40] <jtv> Hmm... nicht süß?
[14:41] <al-maisan> jtv: na ja, das auch :)
[14:41] <jtv> I've gone through enough horrible crap code to be really, really grateful and environmentally conscious about this stuff...
[14:41] <jtv> one of my favourites:
[14:41] <al-maisan> jtv: fully agree :)
[14:41] <jtv> for (i=0; i < result.size(); ++i) {
[14:42] <jtv> switch (i % 3) {
[14:42] <jtv> case 0:
[14:42] <jtv> blablah();
[14:42] <jtv> break;
[14:42] <jtv> case 1:
[14:42] <jtv> foofoo();
[14:42] <jtv> break;
[14:42] <jtv> case 2:
[14:42] <al-maisan> all specimen of finest code :)
[14:42] <jtv> barbar();
[14:42] <jtv> break;
[14:42] <jtv> case 3:
[14:42] <jtv> foofoo();
[14:42] <jtv> barbar();
[14:42] <jtv> break;
[14:42] <jtv> }
[14:42] <jtv> return;
[14:42] <jtv> }
[14:43] <jtv> ah, there was an "if" as well
[14:43] <al-maisan> jtv: no goto?
[14:43]  * al-maisan is slightly disappointed :)
[14:43] <jtv> ah! no goto, but I forget the "default:" case
[14:43] <al-maisan> :-)
[14:43] <jtv> Note how the "case 3:" won't be exercised except in cases where you have much, much worse things to worry about
[14:44] <al-maisan> like broken CPUs?
[14:44] <jtv> or broken compilers
[14:44] <jtv> and this one was broken... oh God was it broken
[14:44] <al-maisan> jtv: if you continue like this your branch will never be reviewed ..
[14:45] <al-maisan> :)
[14:45]  * jtv swallows some great anecdotes about Microsoft's compiler
[14:45] <al-maisan> we can chat next year :-)
[14:45] <jtv> ok :)
[14:45] <al-maisan> in NZ
[14:45] <jtv> ok
[14:48] <jtv> al-maisan: I added my approval
[14:50] <al-maisan> jtv: thank you!
[14:50] <jtv> np
[15:04] <EdwinGrubbs> sinzui: ping
[15:04] <sinzui> Hi EdwinGrubbs
[15:04] <EdwinGrubbs> sinzui: are you working today?
[15:04] <sinzui> yes and no
[15:05] <sinzui> EdwinGrubbs: I am hacking on some critical items, but I am also om my day of
[15:05] <sinzui> off
[15:05] <sinzui> EdwinGrubbs: How can I help you?
[15:06] <EdwinGrubbs> sinzui: just wondering if you wanted to review a 2810 line branch that only has 731 lines which are not formulaic changes, but that is not a good way to spend your day off.
[15:07] <EdwinGrubbs> sinzui: bye now. Have a good holiday.
[15:44] <jtv> al-maisan: any remarks from the review so far?
[15:46] <al-maisan> jtv: none so far .. 
[15:47] <al-maisan> jtv: thanks for cleaning up the doc strings for some of the IBuildFarmJob methods!
[15:48] <jtv> nearly removed them for fear of causing conflicts with your work :-)
[15:49] <al-maisan> jtv: why the "pylint: disable-msg=E0211,E0213" on line 119?
[15:49] <jtv> al-maisan: IIRC that's the warnings about "hey this method doesn't take a 'self' argument"
[15:49] <jtv> lemme check
[15:49] <al-maisan> thanks
[15:51] <jtv> al-maisan: actually, it seems I only need to suppress 213
[15:51] <jtv> so I'll remove the other one
[15:51] <al-maisan> thanks again
[15:53] <al-maisan> my first thought re. `IBuildFarmJobBehavior`: do we really have to impose an implementation of this interface upon each job class? It seems pretty generic.
[16:01] <jtv> Yes, I was wondering about that too... but then again the best way to find out is to duplicate and then unify
[16:10] <al-maisan> jtv: yeah .. I guess there's no way around it.
[16:11] <jtv> I think it won't be too hard to factor most of it away, but first we'll have to find out what differences we need...
[16:12] <al-maisan> hmm..
[16:17] <al-maisan> jtv: I like your '4010' job score :)
[16:18] <jtv> al-maisan: we may want to improve on it later, but right now I just statically computed what seemed the closest equivalent for package build jobs
[16:18] <al-maisan> as you said in the comment: if the duration is short it may not matter much ..
[16:19] <al-maisan> ..except if you flood the build farm with gazillions of these jobs 
[16:19] <jtv> ...at which point we'll probably just want to tweak the score statically to keep them down a bit.  :-)
[16:22] <al-maisan> jtv: what would happen if we dropped one of these on the build farm as it stands? Would TranslationTemplatesBuildBehavior.dispatchBuildToSlave() work?
[16:22] <jtv> al-maisan: my impression was that it wouldn't, because BuildQueue isn't really ready for that
[16:23] <jtv> (at least until your branch lands...)
[16:24] <al-maisan> Well, there's a much more fundamental build farm piece that needs adjustment: candidate job selection
[16:25] <jtv> that, too—though I guess that would not affect this particular method
[16:25] <al-maisan> the branch you approved previously is just a part of the work that needs to get done in order to have job dispatch time estimation working across different job types
[16:25] <al-maisan> right.
[16:28] <jtv> one problem is that I don't really know what affects what there; it's actually not unthinkable that the method might work as-is
[16:29] <al-maisan> jtv: OK .. so, I did not look at the db part .. the code part looks reasonable .. but if I approve it what have we gained .. not quite sure where we stand..?
[16:29] <jtv> [Nice: D'Addario colour-codes its guitar strings... no individually labeled packaging needed]
[16:31] <jtv> al-maisan: we'll have the BuildFarmJob part for our type of job basically ready.  Next I add testing for the dispatch method; implement the method you just defined; and register the new job type in specific_job_types.
[16:31] <jtv> (Of course I'll also get a db review)
[16:32] <al-maisan> jtv: OK .. makes sense .. do you want me to approve the branch in that context?
[16:32] <jtv> al-maisan: not sure I understand the question
[16:33] <al-maisan> jtv: re. "register the new job type in specific_job_types": this part is in doubt, there are some other ideas floating around how this can be implemented in a cleaner way in BuildQueue
[16:34] <jtv> al-maisan: I understand... all the more reason why I didn't implement those in-flux parts yet
[16:34] <jtv> my impression though was that we have some pretty stable interfaces already, and those parts it seems to make sense to land
[16:35] <al-maisan> jtv: I have no qualms about the ITranslationTemplatesBuildJob* part
[16:36] <al-maisan> the build behaviour part though is more obscure and untested
[16:36] <al-maisan> just a minute please
[16:36]  * jtv strings up another string
[16:38]  * al-maisan looks for any tests for dispatchBuildToSlave()
[16:39] <al-maisan> jtv: this is still very much work in progress and as such looks good, r=me (code)
[16:40] <jtv> al-maisan: wonderful, thanks
[16:40] <al-maisan> jtv: thank you!
[16:40] <jtv> I don't like going in with code that's not complete enough to be operational, but it's better than letting it bit-rot unmerged
[16:40] <al-maisan> jtv: yeah .. I understand
[16:41] <jtv> [Some things I do miss in this guitar: the monkey grip and having the tuning knobs all on one side of the head]
[16:42] <al-maisan> jtv: enjoy your break!
[16:42] <jtv> thanks, same to you too!  I believe there's some custom-made egg-nog waiting for me
[16:42] <al-maisan> ..and thanks for bearing with me for such a protracted period of time ;)
[16:43] <jtv> no worries... kinda peaceful here tonight.  Good night, happy holidays, & see you in 2553 BE!
[16:43]  * al-maisan is being called to the dinner table
[16:43]  * al-maisan will google "2553 BE" later :P
[16:48] <bac> jelmer: you can submit the *Publishing fix with rs=bac if you want, assuming there is nothing else in it.
[16:54] <jelmer> bac: Thanks!
[17:39] <bac> jelmer: please reply to the email on the launchpad list about the buildbot failure stating you've submitted a fix.  otherwise someone else may waste time investigating.
[17:49] <jelmer> bac: ok
[17:50] <bac> jelmer: do you know anything about the new flood of import warnings spewed by bin/test?
[17:56] <jelmer> bac: No, sorry
[18:30] <bac> jelmer: jml's branch 10090 changed the importfascist and caused all of the new warnings.
[18:46] <al-maisan> bac: yep, http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/revision/10090
[18:47] <bac> al-maisan: i haven't looked into the warnings to see if they are accurate or not...
[18:49] <al-maisan> same here, 
[18:50] <al-maisan> but jml's branch looked good.
[18:50]  * al-maisan needs to go
[18:50] <al-maisan> have a nice vacation!