[00:02] <thumper> https://code.edge.launchpad.net/~thumper/launchpad/code-import-event-garbo/+merge/38675
[00:04] <thumper> lifeless: this branch adds garbo for code import evenst
[00:04] <thumper> lifeless: which I'd love to get cp'ed to loganberry asap
[00:12] <mwhudson> drop table codeimportevent?
[00:12] <mwhudson> it was kind of a nice idea, but never fully implemented and now it seems to be just a hindrance
[00:35] <lifeless> thumper: it looks fine to me; no need to cp it in a hurry though - just put the rev that it lands in on LPS special deployment instructions
[00:36] <thumper> mwhudson: useful really for machines, not for code imports themselves
[00:36] <thumper> mwhudson: I've been thinking of refactoring the code import model code to not use it
[00:36] <mwhudson> yeah, that's true
=== 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
[08:11] <lifeless> hi henninge
[08:13] <henninge> Hi lifeless ! ;)
[08:14] <lifeless> I have some parallel testing stuff that is moderately working
[08:14] <lifeless> would you like to eyeball it ?
[08:14] <henninge> lifeless: I can have a look ...
[08:15] <lifeless> https://code.edge.launchpad.net/~lifeless/launchpad/uniqueconfig/+merge/38689 and https://code.edge.launchpad.net/~lifeless/launchpad/databasefixture/+merge/38694
[08:18] <lifeless> I'm not particularly worried about polish here - we've such a long trip to go, I'm primarily interested in *getting there* and then filling things out
[08:19] <lifeless> otoh I don't want to land stuff that isn't understandable either
[08:21] <henninge> lifeless: for starters, why are you adding stuff to lib/canonical when the doctring says it's launchpad specific? ("Create a unique launchpad config.")
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: lifeless || queue: [lifeless] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[08:22] <lifeless> henninge: because, for all intents and purposes, canonical.config *is* launchpad specific.
[08:22] <lifeless> henninge: its a YAGNI to claim otherwise. lazr.config is generic, and its in a different tree
[08:23] <lifeless> henninge: that said, my code is -so far at least- agnostic, so i can alter the docstring if you like
[08:25] <henninge> lifeless: why not put it in lp.testing?
[08:25] <lifeless> henninge: two reasons; its able to be used outside of test code, and its tightly coupled to the config concept, which lives in canonical.config
[08:26] <lifeless> it would split a single concept across the code base to split them up
[08:37] <henninge> lifeless: do you know what tha project is called that holds the production configs?
[08:38] <lifeless> launchpad-production-configs
[08:38] <lifeless> bzr+ssh://bazaar.launchpad.net/~launchpad-pqm/lp-production-configs/trunk/
[08:38] <henninge> Cheers
[09:07] <henninge> lifeless: where does "EnvironmentVariableFixture" come from? I cannot find it in the fixtures source code?
[09:07] <henninge> ah, you are using 0.3.2
[09:31] <bac> hi henninge
[10:31] <lifeless> henninge: hey, so how are you going with those branches; I worry that I've sent you off into a tailspin or something :)
[10:38] <henninge> lifeless: otp
[11:02] <lifeless> jml: you can see in backlog two branches for parallel testing; henninge has had them for a few hours; I don't know what outcome will be; you might be interested regardless, or if henninge is too busy, to just review em ;)
[11:02] <lifeless> gnight all
[11:03] <jml> lifeless: sure :)
[11:04] <henninge> lifeless: sorry
[12:22] <bac> hi henninge, will you have time for another review?
[12:23] <bac> henninge: unfortunately i probably won't be around to chat about it.  but if you do have time:
[12:23] <bac> https://code.edge.launchpad.net/~bac/launchpad/bug-652149/+merge/38705
=== bac changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: lifeless || queue: [lifeless,bac] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[12:24] <henninge> bac: I am off to lunch now but can look at it later ...
[12:24] <bac> henninge: ok
=== Ursinha-afk is now known as Ursinha
=== jelmer_ is now known as jelmer
=== matsubara-afk is now known as matsubara
[14:04] <leonardr> henninge, if you get to it i'd appreciate a review of https://code.edge.launchpad.net/~leonardr/launchpad/oauth-doctest-to-unit-test/+merge/38715
[14:17] <henninge> leonardr: sure
=== leonardr changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: lifeless || queue: [lifeless,bac,leonardr] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[14:26] <abentley> henninge: could you please review https://code.edge.launchpad.net/~abentley/launchpad/detect-xen/+merge/38718 ?
=== Ursinha is now known as Ursinha-afk
=== henninge changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: lifeless || queue: [abentley,bac,lifeless,lifeless] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== Ursinha-afk is now known as Ursinha
=== sinzui changed the topic of #launchpad-reviews to: On call: leonardr || Reviewing: lifeless || queue: [abentley,bac,lifeless,lifeless, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[15:22] <bigjools> jml: your comment about "If you change the constructor of NewBuildersScanner ..." makes no sense to me.  Can you explain please?
[15:23] <jml> bigjools: the constructor currently does a query to populate current_builders
[15:23] <jml> bigjools: basically, you don't have to do that
[15:23] <jml> *and*
[15:23] <jml> you don't have to have most of the code in startService
[15:24] <jml> just schedule the NewBuilders... call
[15:24] <jml> and the first run will be exactly equivalent to the query in the constructor
[15:25] <bigjools> jml: ok - I don't want to change this right because there's a bug somewhere that causes the manager to crash when a new builder is detected
[15:25] <bigjools> s/right/right now/
[15:26] <bigjools> I can address this with the bug if you think that's ok?
[15:26] <jml> bigjools: sure.
[15:26] <bigjools> well I say crash - it stops dispatching builds
[15:26] <bigjools> but the log continues as if there were no issues
[15:26] <bigjools> which is utterly bizarre
=== salgado is now known as salgado-lunch
=== Ursinha is now known as Ursinha-lunch
[16:04] <abentley> rockstar: could you please review https://code.edge.launchpad.net/~abentley/launchpad/detect-xen/+merge/38718 ?
[16:04] <rockstar> abentley, sure.
[16:04] <abentley> rockstar: thanks.
[16:09] <henninge> leonardr: approved
[16:09] <leonardr> henninge, great
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: - || queue: [abentley,bac,lifeless,lifeless, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[16:09] <henninge> abentley: you just found yourself another reviewer?
[16:09] <henninge> Hi rockstar! ;-)
[16:10] <abentley> henninge: yes.
[16:10] <henninge> ok, I'll move on to bac then.
[16:10] <rockstar> henninge, hi!
[16:10] <leonardr> henninge: i think i need to recreate request_token a lot because the lifespan of a request_token is not very long
[16:10] <leonardr> well, i guess that's not what you were saying...
[16:26] <leonardr> henninge: if i should use a mixin instead of a base class, but a mixin can't have a setup method, what should i do?
[16:27] <henninge> leonardr: I am just saying that this pattern (a TestCase base class) cannot be used if the base class defines test methods, too.
[16:27] <henninge> The test machinery will try to run them ,too.
[16:27] <henninge> leonardr: for your case, you can leave it as it is. Sorry for the confusion.
[16:27] <leonardr> ok, thanks
[16:28] <leonardr> henninge: sorry, i'm going to keep bugging you
[16:29] <leonardr> what do you mean that that "key = " line is needed for all tests? i don't think it's ever used again
[16:29] <leonardr> do you want that to be the consumer key for self.consumer?
[16:29] <henninge> let me look again, maybe I got mixed up.
[16:30] <henninge> I mean ... never mind.
[16:30] <henninge> ^ignore that line
[16:31] <henninge> leonardr: yes, if you split the tests (in TestConsumerSet) up, you will need "key" in all but the "verifyObject" test.
[16:32] <leonardr> henninge: i see what you mean. but, if i split up the tests, i can use self.consumer for all but the first one
[16:33] <henninge> leonardr: My main theme is that the tests can be split up.
[16:34] <henninge> Each test will need to be set up of course.
[16:34] <leonardr> ok, i will split them up as much as possible and have you take another look
[16:34] <henninge> So you'll have to decide if the objects you need for the tests to run are created in the test method or in the setUp.
[16:34] <leonardr> got it
[16:35] <henninge> good ;-)
=== henninge changed the topic of #launchpad-reviews to: On call: henninge || Reviewing: bac || queue: [lifeless,lifeless, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== benji is now known as benji-lunch
=== Ursinha-lunch is now known as Ursinha
[17:25] <deryck> adeuring1, hey, my MP is up to date now.  https://code.edge.launchpad.net/~deryck/launchpad/better-testing-for-status-changes/+merge/38552
[17:25] <adeuring1> deryck: ok, I'll look
[17:26] <deryck> adeuring1, I went with two tests, one for regular user and one for privileged users, even though there is some overlap because it read better.  And too avoid `if regular_user` type constructions.
[17:26] <deryck> adeuring1, but if you want one test with that type check, I can do that.
[17:26] <adeuring1> deryck: nah, I think that's fine
=== henninge changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [lifeless,lifeless, sinzui, sinzui] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[17:47] <adeuring1> deryck: r=me; two nitpicks
[17:47] <deryck> adeuring1, great, thanks!
[17:51] <deryck> adeuring1, ah, I forgot about the other celebrities.  Sorry.
[17:51] <adeuring1> deryck: no problem -- that's what reviews are for ;)
[17:51] <deryck> indeed! :-)
[17:52] <bigjools> jml: I pushed up a load of changes - I've not done everything you talked about, my will to live is slowly being sapped by this branch.  I've got some more changes to make tomorrow as described in the MP reply.
[17:53] <jml> bigjools: sure
[17:53] <jml> bigjools: I've got to go now, I think I've reviewed all of your changes from today though.
[17:54] <bigjools> jml: thanks muchly
[17:54] <bigjools> I am offski too
=== benji-lunch is now known as benji
=== salgado-lunch is now known as salgado
=== matsubara is now known as matsubara-lunch
[18:16] <henninge> leonardr: can you please give me an incremental diff?
[18:16] <leonardr> henninge, sure
[18:41] <leonardr> henninge_: http://pastebin.ubuntu.com/515781/
[18:41] <leonardr> sorry for the delay
=== henninge_ is now known as henninge
[18:55] <abentley> rockstar: ping?
[18:56] <rockstar> abentley, hi
[18:56] <abentley> rockstar: chat?
[18:57] <rockstar> abentley, sure.
[18:57] <abentley> rockstar: Skype or mumble?
[18:57] <rockstar> abentley, skype
=== bdmurray changed the topic of #launchpad-reviews to: On call: - || Reviewing: - || queue: [lifeless,lifeless, sinzui, sinzui, bdmurray] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
=== matsubara-lunch is now known as matsubara
[19:23] <abentley> sinzui, bdmurray: please ask someone to perform an on-call review before adding yourself to the on-call review queue.
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: - || queue: [lifeless,lifeless, sinzui, sinzui, bdmurray] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[19:26] <bdmurray> abentley: okay
[19:26] <abentley> bdmurray: thanks.
[19:28] <abentley> sinzui: I do not know what foaf means.  Can you please explain it?
[19:30] <abentley> sinzui: why are you writing the same module for each import, e.g. 25-26?
[19:30] <sinzui> abentley, FOAF is an RDF vocabulary to describe a Friend of a Friend. It about 7 years old. It is one of the oldest RDF vocabs and many tools like python-rdflib support it out-of-the-bix
[19:30] <abentley> sinzui: What is Friend of a Friend?
[19:31] <sinzui> abentley, that is a by-product of the script's expansion of identifiers identifiers
[19:31] <sinzui> abentley, http://xmlns.com/foaf/spec/
[19:32] <sinzui> abentley, our RDF for projects and people mix our proprietary vocab with standard vocabs
[19:33] <sinzui> We do not want to reinvent FOAF.
[19:33] <sinzui> We do not want to reinvent DOAP (Description of a Project), but DOAP does not define concepts like series
[19:34] <sinzui> So we ignore DOAP
[19:36] <abentley> sinzui: Several of your examples say "Accoun" rather than "Account".
[19:36] <sinzui> oh!
[19:36] <sinzui> I think that means the script is bad
[19:37] <sinzui> abentley, I ran the entire test suite without error Sunday!
[19:38] <abentley> sinzui: I guess you need to make sure those scripts are being run, then...
[19:41] <sinzui> abentley, sorry. I just had a panic. I was thinking of my apocalypse branch. That is importing correctly.
[19:41] <abentley> sinzui: Ah, sorry.
=== matsubara is now known as matsubara-afk
[19:42] <abentley> sinzui: for the apocalypse branch, have you considered updating the script to avoid repeating modules?  It seems like a small effort to avoid lots of work later.
[19:43] <sinzui> abentley, I did and considered it an extra 4 hours to run and test to fix imports in tests I would rather delete
[19:44] <sinzui> I certainly will investigate sorted multiline imports it you want me too
[19:47] <abentley> sinzui: would you please?  I agree that it would be nice to convert/delete these.
[19:48] <sinzui> okay
[20:00] <sinzui> abentley, I must have been running tests in the wrong branch. This RDF branch has another idiotic problem be defining a user as a group. the tests rightly fails
[20:00] <sinzui> s/be defining/by defining/
[20:01] <abentley> sinzui: Ah.  Okay.  Please ping me when it's working.
=== abentley changed the topic of #launchpad-reviews to: On call: abentley || Reviewing: bdmurray|| queue: [lifeless,lifeless] || This channel is logged: http://irclogs.ubuntu.com/ || https://code.launchpad.net/launchpad/+activereviews
[20:08] <abentley> bdmurray: was there a preimplementation call for https://bugs.edge.launchpad.net/malone/+bug/114766 ?
[20:08] <_mup_> Bug #114766: Only bug supervisor should be able to nominate a bug for a release <Launchpad Bugs:Triaged> <https://launchpad.net/bugs/114766>
[20:08] <abentley> bdmurray: or rather https://code.edge.launchpad.net/~brian-murray/launchpad/bug-supervisor-nominate-for-release/+merge/38733?
[20:09] <bdmurray> abentley: yes, deryck and I discussed it
[20:09] <abentley> bdmurray: it's helpful to note that in your merge proposall.
[20:09] <abentley> bdmurray: did you check lint?
[20:10] <bdmurray> abentley: no, doing so now
[20:11] <abentley> bdmurray: If you install lpreview_body, then lp-propose will supply a cover letter template with the lint stuff filled in.
[20:11] <bdmurray> abentley: where is lpreview_body?  I hadn't heard of this
[20:13] <abentley> bdmurray: it's in the Launchpad team PPA and on Launchpad.
[20:14] <abentley> bdmurray: https://edge.launchpad.net/lpreview-body
[20:16] <abentley> bdmurray: on line 9, why are you checking target.bug_supervisor before doing check_permission("launchpad.Driver", target)?
[20:18] <bdmurray> abentley: hmm, that does seem backwards
[20:20] <abentley> bdmurray: The idea is that the BugDriver or bug supervisor can always do this, but if the bug supervisor is set, normal users cannot?
[20:20] <bdmurray> abentley: yes, that is correct
[20:22] <abentley> bdmurray: that seems a) complicated and b) like it could be provided as a permission.
[20:23] <lifeless> https://code.edge.launchpad.net/~lifeless/launchpad/databasefixture/+merge/38694
[20:24] <abentley> bdmurray: it also means error messages that say "Only bug supervisors can nominate bugs." are not entirely accurate.
[20:26] <bdmurray> abentley: okay, its not clear to me what you mean by providing it as a permission
[20:27] <abentley> bdmurray: I mean "launchpad.NominateBug" as a permission that encapsulates the logic you're doing in the view code.
[20:30] <bdmurray> abentley: hmm, if you look at security.py there is a "launchpad.BugSupervisor" permission that checks to see if the user is in obj.owner or in_admin. so perhaps the check_permission for launchpad.Driver is redundant?
[20:31] <abentley> bdmurray: lemme see...
[20:31] <sinzui> abentley, I fixed the two tests and fixed the person-rdf template: https://code.launchpad.net/~sinzui/launchpad/rdf-links-1/+merge/38573
[20:33] <abentley> bdmurray: It seems like launchpad.Driver is meant to be "launchpad.NominateBug", doesn't it?
[20:34] <abentley> sinzui: r=me.  There are some blank lines introduced at 64.  If they were not added deliberately, you may want to remove them before submitting.
[20:34] <bdmurray> abentley: I was under the impression that sinzui was reluctant to add lots of permissions
[20:35] <sinzui> abentley, thanks. I also need to fix the 2-space indentation in several of the tests. I will fix blank lines too
[20:37] <abentley> bdmurray: I don't know that sinzui's preferences are authoritative here.  My own preference is to have a single point of truth.  We can't reuse that browser code everwhere we might want to check that permission.
[20:39] <bdmurray> abentley: to be clear this is the modification I'm suggesting https://pastebin.canonical.com/38756/
[20:39] <abentley> bdmurray: in fact, you could eliminate most of 100-113 if you did that.
[20:39] <sinzui> bdmurray, abentley. We should not introduce new kinds of permissions [flacoste]. But we have always had bug_supervisor and it is not clearly defined in security.py, so I advocate defining it so I do not need to guess
[20:41] <abentley> sinzui: I am getting confused about the difference between launchpad.Driver and this.  Is it the ability to unset bugs that would differentiate launchpad.Driver?
[20:41] <sinzui> abentley, bdmurray: I believe there is agreement that models can define security checks that are imported in security.py
[20:42] <abentley> sinzui: I would be fine with that.
[20:42] <sinzui> bug_supervisors are a specialisation of drivers.
[20:43] <sinzui> I think (because there is know authoritative checker) that bug_supervisors are effectively drivers with elevated permission to manage bugs
[20:43] <sinzui> There is no requirement that a bug_supervisor be a driver, or vice-versa
[20:45] <abentley> sinzui: This permission would allow all to nominate bugs unless a supervisor is set, in which case only the supervisor and driver would be able to.  Does that seem reasonable?
[20:46] <bdmurray> abentley: all can already nominate bugs for a release, this restricts it if a bug supervisor is set
[20:46] <abentley> bdmurray: that is what I was trying to day.
[20:46] <abentley> s/day/say
[20:46] <bdmurray> okay cool
[20:47] <sinzui> abentley, I must defer to deryck, gmb, and allenap on that issue. I certain do not want anyone to ever make a nomination in any project I run
[20:48] <deryck> yes, I feel convinced after talking to bdmurray that nominations are useless since anyone can make them.
[20:48]  * sinzui looks for a bat to hit someone every time someone uses +nominate to vote for his personal issue
[20:49] <abentley> deryck: if so, is it a good idea to allow anyone to make nominations if the bug supervisor is unset?
[20:50] <sinzui> And why can someone nominate to fix a bug in trunk or an obsolete series? +nominate only works for projects that create forward series or have to manage backports to maintained series
[20:51] <deryck> abentley, yeah, that's a good point.  I think bdmurray was trying to have as little impact as possible on other projects.
[20:51] <deryck> abentley, bdmurray -- I think only driver or bug supervisor should be able to nominate for series.
[20:51] <deryck> we ACL assign to milestone, why not series?
[20:52] <bdmurray> deryck: drivers have their nominations approved automatically so that effectively removes nominations if there is no bug supervisor
[20:52] <deryck> bdmurray, right
[20:52] <deryck> that's how milestones work.  Shouldn't this be the same?
[20:53] <bdmurray> but nominations are a way of escalating (in my mind) a bug report.  this removes this escalation path for projects w/o bug supervisors
[20:53] <bdmurray> s/this removes/this would remove/
[20:54]  * deryck is thinking....
[20:54] <abentley> bdmurray: I don't know what nominations are, but according to the bug, normal people don't know how to use them correctly.
[20:55] <deryck> I don't think anyone actually uses them as escalation.  ubuntu users do, but that's why we're adding this restriction. :-)
[20:56] <deryck> sinzui, do you see any issue in treating series like milestone, and not make it available at all if you're not the owner or bug supervisor?
[20:57] <sinzui> deryck, I think they should be the same
[20:57] <deryck> ok, cool.
[20:58] <sinzui> deryck, I think they are not the same because we implemented 2, not 1, way of planing bugs and features
[20:59] <deryck> bdmurray, since sinzui and I are agreed, I think we should do as abentley suggests and make this only for owners and bug supervisors.
[20:59] <deryck> sinzui, right, agreed
[21:00] <bdmurray> okay, got it
[21:00] <sinzui> I favour one implementation that allows users to say this should be fix by z, or is inclusive in x without needing to know how milestones or series are implemented
[21:01] <deryck> sinzui, I get what you mean now.  but until we can get to that, do you have any concerns about having series behave slightly more like milestones in this way?
[21:01] <sinzui> no. similar is easier to learn
[21:02] <deryck> ok, cool.
[21:02] <deryck> thanks
[21:02] <sinzui> deryck, if we made series "look like" milestones, I could target a bug to series and then undo it!
[21:02] <deryck> indeed!  Though that wouldn't be covered by bdmurray's current work.
[21:03] <deryck> ok, so I need to bail for the day.
[21:04] <deryck> Catch you all tomorrow.
[21:05] <bdmurray> abentley: so is okay to use check_permission in model/bug.py to cleanup lines 100-113 ?
[21:20] <rockstar> abentley, https://code.edge.launchpad.net/~rockstar/launchpad/merge-queues-model/+merge/38762
[21:20] <abentley> bdmurray: works for me.
[21:35] <henninge> leonardr: Hi! Now I know what got me confused - you did not push you changes.
[21:36] <henninge> leonardr: but what I see in the incremental diff looks good ;-)
=== salgado is now known as salgado-afk
[22:05] <rockstar> abentley, hello.
[22:05] <abentley> rockstar: hi.  OTP
[22:06] <rockstar> abentley, ack. Just wanted to say "Can I please get a review?"  That is all.  :)
=== 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
[22:12] <abentley> rockstar: You need another blank line at 73
[22:13] <rockstar> abentley, indeed I do.
[22:16] <abentley> rockstar: I would rather see BranchMergeQueue.new as a classmethod, not a staticmethod, so you don't need to repeat the classname.
[22:16] <rockstar> abentley, ack.  Will make that change as well.
[22:19] <abentley> rockstar: r=me with those changes.
[22:19] <rockstar> abentley, thank you sir!
=== Ursinha is now known as Ursinha-afk
[23:52] <wallyworld_> lifeless: you may want to claim this one https://code.edge.launchpad.net/~wallyworld/launchpad/fix-hardcoded-test-urls/+merge/38779
[23:53] <wallyworld_> i decided to land it independent of your uniqueconfig one - i can come back and fix the hard coded config name once both have landed
[23:53] <lifeless> thats great