[00:02] wgrant: Sorry, was breakfasting [00:13] wgrant: Gah, it's been so long since I've updated mawson. bzr pull in devel and then ../rocketfuel-get.sh ? [00:15] StevenK: ~/bin/upgrade-dogfood-launchpad or so [00:15] Does it all. [00:40] 2012-01-06 00:40:09 INFO Upload was rejected: [00:40] 2012-01-06 00:40:09 INFO Invalid Maintainer. [00:52] wgrant: Shall I put up a NDT? [04:18] wgrant, maybe you could have a look at this as well? I feel entirely unqualified to assess or Q/A this, so the more eyes the merrier. https://code.launchpad.net/~jtv/launchpad/bug-876594/+merge/87624 [04:29] jtv: What does TestSyncNotification.makeUploader do that Archive.newComponentUploader doesn't? [04:36] jtv: I think signingkey should be preferred to SPPH.creator [04:36] But it probably doesn't matter too much. [04:37] It also looks like it's Changed-By that gets spammed, not Maintainer. [04:37] Possibly. And if so, it'll be a lot harder to hammer out of notify() without risking unforeseen consequences. [04:37] Hm? [04:38] This should suppress Changed-By, not Maintainer. [04:38] I don't see how this can suppress Maintainer. [04:38] if blamer is None: [04:38] debug( [04:38] logger, "Changes file is unsigned; adding changer as recipient.") [04:38] candidate_recipients.append(changer) [04:40] * lifeless whispers statemachine [04:41] Changing notify() terrifies me. [04:42] I'm not saying you need to. [04:42] I'm saying that the this isn't doing what your test says it does. [04:42] The problem may have been misinterpreted, and this will fix it anyway. [04:46] I think I'll add a changer to the test. [04:58] wgrant: my eye now falls on a detail that I had hitherto ignored — is_valid_uploader. [04:59] jtv: Right. [04:59] jtv: That projects the only Maintainer usage that I can see. [04:59] My branch replaces the paragraph you just quoted with one that also adds the changer, but only if the changer is an uploader. [04:59] s/rpojects/protects/ [04:59] No match. [04:59] :-P [04:59] Your branch doesn't seem to touch notifications.py [05:00] That's right. [05:00] Who would want to? [05:00] Thanks ever so. [05:00] The paragraph I quoted is from notifications.py [05:00] notification.py, sorry [05:00] So if your branch doesn't touch that file, it probably didn't replace the paragraph that I quoted. [05:00] StevenK: Nothing personal. For context, I've been saying I don't want to change it because I can't oversee the consequences. [05:02] StevenK: If I change the logic in notifications.py, it may affect lots of other things. That's why I don't think anybody would want to touch notifications.py to change just one particular behaviour. [05:03] wgrant: I didn't mean that my branch replaces it textually; it replaces it dynamically by going through a different code path. [05:03] wgrant: the bit you quoted is for the case where blamer is None, and it adds changer to the recipients list unconditionally. [05:04] hmm, notification looks roughly the right place to build up from to get a clean notification api [05:04] jtv: Right, that's what I meant. [05:04] jtv: It changes Changed-By from unconditional to conditional. [05:04] By supplying a non-None blamer, I make it go through a different clause that also adds the changer — but only if the changer is an uploader. [05:04] Right. [05:04] But doesn't change Maintainer unless they are an uploader. [05:04] Which in this case they aren't. [05:04] So it doesn't change the Maintainer case. [05:05] Change Maintainer? [05:05] You mean add? [05:05] So either the problem is misstated and your test is wrong, or your fix is wrong. [05:05] Right. [05:05] The maintainer also gets added only if it's an uploader. [05:06] Right. [05:06] But you're right that the test needs re-stating. [05:06] Which they're clearly not. [05:06] This change can only make it more likely that the maintainer is notified. [05:06] It makes it less likely that Changed-By is notified. [05:07] Yes. That sucks, but that's where we get into the internals of notification.py. :( [05:07] I realise you don't want to upscope this - but is the basic model I proposed for a notification service useful (even in-app) for trying to make this easier to reason about ? [05:07] No. [05:08] The problem is not notifying people. It's working out from our terrible data model who to notify and when and how to avoid breaking things. [05:08] And rewriting the whole thing is not a good way to not break things :) [05:08] I didn't actually suggest that [05:08] but, breaking it into two problems - even conceptually: [05:08] - who is involved [build the sets of involved folk, including how they are involved] [05:09] That's exactly the problem. [05:09] - notify [05:09] That's how it is now. [05:09] but it sounds to me that you're examining conditional code that takes the event into account when determining who is involved [05:10] I'm not entirely sure how one can determine the involved people without knowing context... [05:11] perhaps I misunderstand the issue you're wending your way through [05:11] I have to wrap up some stuff, so I will get out of your hair :) [05:11] I may come back if you're still bloodying up the wall in a while [05:11] lifeless: for the record, I started out by strengthening that separation, pulling the "identify recipients" into the call site. It didn't help this case. [05:12] jtv: identify recipients is actually against my point, its not equivalent to identifying roles, if you get my drift (Again, terminology, I probably misunderstand) [05:12] Potato, potato. [05:18] wgrant: I suppose the immediate question is whether it's okay to notify changers who are uploaders. [05:18] Sorry, no: maintainers who are uploaders. [05:26] wgrant: I don't suppose it's likely to be too problematic for a maintainer or change author to be notified about builds in archives they are uploaders for? [05:26] jtv: I would argue that it's pointless. But it has precedent. [05:26] And it's not harmful. [05:26] Looking on the bright side, it's something they're involved in and can do something about, right? [05:27] "harmful" for Soyuz is roughly defined as "pissing off Debian developers who aren't involved in Ubuntu" [05:30] Or its derived distros. [05:30] That's not really an issue. [05:31] Distros derived from us are less likely to want to kill us. [05:31] Not meant to annoy you; just wondering if there might be non-obvious scenarios where a Debian maintainer might be made an uploader through indirect membership, or to work around some completely unrelated problem. [05:32] It's unlikely. [05:32] Ideally we would entirely ignore Maintainer and Changed-By, just notifier signer/requester + sponsoree [05:32] But we don't have that modelled sufficiently at present. [05:32] Clutching at the straws of negativity, any chance that we might generate unacceptable mail volume for someone out there? [05:33] Aren't we just returning to the previous behaviour here? [05:33] Mm, although Changed-By was changed. [05:34] Anyway, I suspect it's Good Enough for Now™ [05:35] I'm inclined to feel the same, for what little that's worth; but good to have this sanity chat. [05:35] The Changed-By author is only affected in the direction of less chance of getting notified. [05:37] Yes [05:39] wgrant: so… shall I just land it then? I just added the changed-by author to the test (and they do not get notified). [05:39] jtv: I'd like to see the test failing with the rest of the code reverted, at least. [05:40] My word that that's what happened before I wrote the fix is not good enough? :) [05:40] The test was wrong before, so no :) [05:41] Not wrong; it failed in exactly the expected ways. It was just incomplete. Okay, I'll run that. [05:41] Hmm, that's shouldn't have failed, though. [05:42] Maintainer shouldn't have been notified before. [05:43] Hmm [05:49] wgrant: is it because I made the maintainer sign the changes file? [05:50] jtv: That sounds bad. [05:52] Yeah. The maintainer is no longer notified (by the old code) when I change that. [05:53] Hm, does your test actually use a build at all? [05:53] If not, it's probably not using your code at all. [05:56] But then how could it fail without my change and pass with my change? [05:56] An extremely good question. [05:56] Oh, you hide a build in makeNascentUpload [05:56] That makes more sense. [05:57] Anyway, I need to wander off for a while and prepare stuff for BUD. [05:57] Thanks greatly for your help so far. [05:58] Thanks for trying to understand and fix this madness :/ [06:02] Understand? Fix? Julian told me what to do and we gradually discover how the change works around it. :) [07:53] stub: yo [07:54] stub: could you rebuild the bug search indices? If they are bloated (what are the odds... :P) [07:54] k [07:55] And number one on the bloated index list is... bug_fti [07:56] I'm shocked. [07:56] Hmmm... wonder if I should try that new script to rebuild indexes live... [08:00] losas have a script from (isd or u1?) that should rebuild any index live [08:01] But not deployed on our boxes yet so I'll use my canned statements [08:04] Master is done [08:05] Sometimes wonder if this should be an automated, daily rebuild like data warehouse sites do. But they don't have to do it live... [08:05] heh, uhm, like rebooting windows servers daily. [08:05] I sure hope we can do better :) [08:06] statik was very keen to hear of your new-db-tech interest btw [08:07] Interesting stuff there is going to be U1 with the primary backend to U1DB [08:11] Stats are saying with current usage we need to rebuild bug_fti every two weeks [08:14] May change with bug heat changes, though. [08:17] Yes, but I can schedule myself to check that now :) [08:31] stub: are langpack exports in BAD_USERS or whatnot ? [08:31] stub: https://bugs.launchpad.net/launchpad/+bug/684664 [08:35] wgrant: q for you on https://bugs.launchpad.net/launchpad/+bug/685076 [08:35] <_mup_> Bug #685076: PPA deletion leaves unremoved publications < https://launchpad.net/bugs/685076 > [08:44] lifeless: yes, got landed yesterday in fact. [08:45] stub: I'm guessing there is a duplicate bug then ? [08:45] lifeless: its a different bug [08:46] stub: k [08:46] Hey [08:46] mrevell: hey, FWIW stub has reset the fti index [08:46] mrevell: so, that should help with fti searches [08:46] That's good to hear. [08:46] And scheduled a recurring check every two weeks === almaisan-away is now known as al-maisan [09:09] stub: FYI - work in progress only - bzr+ssh://bazaar.launchpad.net/~lifeless/launchpad/use-lazr-postgresql and bzr+ssh://bazaar.launchpad.net/~canonical-launchpad-branches/lazr-postgresql/trunk/ [09:10] good morning [09:11] lifeless: I'll have a look next week probably. Ta. [09:19] lifeless: Probably very negligible. [09:28] anyone have an opinion on ?https://bugs.launchpad.net/launchpad/+bug/668381 [09:28] <_mup_> Bug #668381: start_codebrowse invokes "make build" unnecessarily < https://launchpad.net/bugs/668381 > [09:30] lifeless: Pretty sure we can just use compile. [09:30] allenap: How do you propose to QA r14641, given it has been rolled back three times? [09:31] allenap: If you say "Wait until next week and beg myself and wgrant" I may be forced to pack a cricket bat. :-P [09:31] StevenK: Ha. Hahaha. HAhahahahahfahfahahah. [09:31] On the list of bad times to roll out an infamously explosive change, I would rank sprints where the whole team is in a single timezone fairly highly :) [09:31] StevenK: If you're offering, that would be most kind, thank you. [09:32] allenap: Do I look insane? :-P [09:33] StevenK: Do you really want me to answer that? [09:33] do you really need a reply to that? [09:33] ? [09:33] Now I remember why I was happy moving from Red to Teal/Purple. :-P [09:33] StevenK: You were never truly Red! [09:34] gmb: oh hi; there was a bug - one you unassigned, where in the bug you said 'testing xyz needs to be done' -> I'm curious if you did said testing or not, to aid handoff [09:34] ok; haltingish. See you guys in buda [09:34] Night lifeless. [09:35] lifeless: Have safe flights. [09:39] wgrant: And note I said QA. I didn't say anything about deployment. :-) === al-maisan is now known as almaisan-away [10:13] https://dev.launchpad.net/ | On call reviewer: adeuring | Firefighting: - | Critical bugtasks: 3*10^2 === jtv is now known as jtv-afk [11:42] Bug 911943 - rollback or fix? (garbo jobs that raise an exception don't actually break anything particularly, but ...) [11:42] <_mup_> Bug #911943: gina imports SourcePackageRelease.dsc_binaries incorrectly < https://launchpad.net/bugs/911943 > [11:43] If it needs to be rolled back, I'll need somebody to do that for me [11:44] adeuring: can you look this over when you get a chance please? https://code.launchpad.net/~rharding/launchpad/oops_912178/+merge/87675 [11:44] rick_h__: sure [11:45] adeuring: ty [11:46] Oh, and can I push the fix to the same branch and remerge, or does it need to be a separate branch? [11:48] cjwatson: It's not overly critical, and the fix seems trivial, so might as well just fix it. [11:48] Same branch is OK if you want. [11:49] Add "chunk_size = int(chunk_size + 0.5)", yes? [11:50] And I'm going to *document this* [11:50] I suppose so :/ [11:52] Fixed. Re-review/landing would be lovely. [11:53] * cjwatson idly notes that there's already a garbo job that shoots itself in the face on dogfood: http://paste.ubuntu.com/794772/ [11:53] Yeah, but that's just DF being itself :) [11:53] Ah, unique to DF? OK [11:53] cjwatson: Can you create a new MP quickly? [11:54] Yeah, that table has existed on prod for a yearish now. [11:55] wgrant: https://code.launchpad.net/~cjwatson/launchpad/gina-dsc-binaries/+merge/87732 [11:56] timeout ftw [11:56] rick_h__: the changes look good. But I think the JS code needs a test [11:57] adeuring: ok [12:03] cjwatson: It breaks test_SourcePackageReleaseDscBinariesUpdater_updates_incorrect [12:04] Really? I ran that test here and didn't see it [12:04] File "/home/wgrant/launchpad/lp-branches/gina-dsc-binaries/lib/lp/scripts/garbo.py", line 1084, in main [12:04] raise SilentLaunchpadScriptFailure(self.failure_count) [12:04] SilentLaunchpadScriptFailure: 1 [12:04] make schema [12:04] Ah, you changed sampledata? [12:04] there's a security.cfg change [12:04] Oh [12:04] Of course. [12:05] (public.sourcepackagerelease += UPDATE) [12:44] flacoste, hi, do you happen to have a minute? === adeuring changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: adeuring | Firefighting: - | Critical bugtasks: 3*10^2 === bac changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: adeuring, bac | Firefighting: - | Critical bugtasks: 3*10^2 [13:28] adeuring: ping, updated with test if you get another look please https://code.launchpad.net/~rharding/launchpad/oops_912178/+merge/87675 [13:28] rick_h__: cool [13:32] rick_h__: r=me thanks for adding the test and the additional changes! [13:33] adeuring yep, thanks for pushing me on it. :) === almaisan-away is now known as al-maisan [14:13] hi danilos [14:15] flacoste, heya, up for short skyping? [14:16] danilos: sure, i have another call in 15 mins [14:17] flacoste, ack, that should be more than enough === matsubara is now known as matsubara-lunch === jtv-afk is now known as jtv [15:27] * deryck will be back online shortly, changing work locations === al-maisan is now known as almaisan-away [16:22] jcsackett, I think you are missing a test [16:27] sinzui: you are correct. i'll add the proposed-membership test and switch to the set-based recommendation. [16:27] thanks. [16:27] fab === matsubara-lunch is now known as matsubara [16:50] deryck: chat? [16:51] abentley, 10-15 minutes and I'll be ready. cool? [16:51] deryck: sure. [16:51] abentley, great, thanks! I'll ping shortly. === salgado is now known as salgado-lunch [17:28] bac, will you have time to review https://code.launchpad.net/~sinzui/launchpad/unassign-bugs-0/+merge/87785 today === deryck is now known as deryck[lunch] [17:36] sinzui: got sidetracked for a bit with thunderdome prep, but changes have been pushed. [17:36] jcsackett, thanks [17:44] jcsackett, r=me [17:45] sinzui: i may in a bit [17:48] bac, if you have time, this should be very straightforward (doc changes only): https://code.launchpad.net/~gary/launchpad/bug578854/+merge/87787 [17:48] (I see others are ahead of me in line :-) ) [17:54] gary_poster, I will review your branch [17:54] thank you sinzui! [17:55] sinzui, if bac doesn't get to your branch in a bit, I can take yours. Trying to wrap a few things up for now, and will take lunch sometime too [17:55] gary_poster, sinzui: i'll be available in a few minutes [17:55] cool === salgado-lunch is now known as salgado [17:59] gary_poster, r=me [17:59] thanks sinzui [18:29] sinzui: looking at your branch now. sorry about the delay. [18:29] thats okay === deryck[lunch] is now known as deryck [18:36] bac, I claimed your branch for review, but is the proposed merge right? [18:37] sinzui: no, it is not [18:37] sinzui: i forgot to specify the pre-req branch. let me redo it [18:37] oaky [18:39] sinzui: this one should be more better: https://code.launchpad.net/~bac/launchpad/904335-milestone-edit/+merge/87796 [18:39] thank you [18:41] * bac afk for a bit [19:08] bac r=me with a suggestion [19:11] sinzui: thanks [19:12] sinzui: excellent suggestions, especially the second [19:12] b/c you're right, i always have to scratch my head over the other syntax === bac changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: [] | Firefighting: - | Critical bugtasks: 3*10^2 === matsubara is now known as matsubara-afk [20:42] deryck: are you up for a review? https://code.launchpad.net/~abentley/launchpad/bugcomment-interface/+merge/87808 [20:42] abentley, I don't mind doing it, but just about to step out for day. Can I do it through tonight/travel, or do you need it today? [20:42] deryck: No rush. [20:43] abentley, ok, cool. I'll claim it then. [20:43] deryck: thanks [20:43] abentley, np! [23:08] bdmurray: small request for you; we find it works better for bugs for them to description the situation, not the desire [23:09] e.g. "cannot search on bugs 'left_closed_since' via the API" [23:10] lifeless: okay