[00:00] wallyworld_: mumble? [00:01] yep === wallyworld_ changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - wallyworld (*jtv) | https://code.launchpad.net/launchpad-project/+activereviews | Critical bugs:209 - 0:[######=_]:256 [00:48] sinzui: Did you see the fallout from the flag-expired-memberships.py fix? [00:51] Project windmill-devel build #143: STILL FAILING in 1 hr 10 min: https://lpci.wedontsleep.org/job/windmill-devel/143/ [02:01] lifeless, sorry, I'm a novice to lp... not sure I understand what you're saying on bug #724740. I thought that adding the mutator was how to run business logic when the attribute is set... [02:01] <_mup_> Bug #724740: setting a ppa private cannot be done over the API

< https://launchpad.net/bugs/724740 > [02:01] lifeless, here's my code so far: http://bazaar.launchpad.net/~timrchavez/launchpad/set_ppa_private_from_api_724740/revision/13052 [02:03] (sorry there are still some inconsistencies in the comments and no tests yet :/) [02:03] * timrc ducks [02:24] wgrant: Most of the failures for archiveuploder look like behaviour changes in notify_spr_less() [02:24] wgrant: Which I'm fixing [02:25] wgrant: Running -t archiveuploader to see what the fall-out is [02:46] StevenK: Hmm. [02:51] Project windmill-devel build #144: STILL FAILING in 1 hr 9 min: https://lpci.wedontsleep.org/job/windmill-devel/144/ [02:57] wgrant: Hm? [02:57] Indeed. [02:57] ! [02:59] wgrant: Down to one failure [02:59] StevenK: But who knows how many you broke in Soyuz :) [02:59] Anyway, let me review your diff. [03:00] I have local changes [03:00] timrc: hi [03:00] Including putting back sanitize_string :-( [03:00] StevenK: Why did the abomination need to return? [03:00] timrc: mutators can be used yes, but if we need it always then just using a property is better [03:02] wgrant: Since I can't call guess_encoding on a unicode string, and the mail text seems to change [03:02] For some tests it's unicode, and others it's ascii [03:02] StevenK: It'll automatically become unicode when a unicode is interpolated into it. [03:02] Because fuckyeah Python 2. [03:03] StevenK: This indicates that there are unicode-safety bugs. [03:03] StevenK: It should either always be a unicode or never be. [03:04] wgrant: Then there likely already were [03:04] Sure. [03:04] r13103 pushed [03:12] Hm. [03:12] The branch is shorter now? [03:12] Interesting. [03:12] StevenK: notify_spr_less seems to assume there's always an archive. [03:13] But that's not the case. [03:13] Or at least it shouldn't be. [03:13] StevenK: Perhaps s/notify_spr_less/reject_changes_file/? [03:14] I'd also continue to encourage you to remove get_status. [03:15] Oh, and just replace it with a dictionary? [03:23] The dictionary, yeah. [03:23] I'm doing a proper rereview now. [03:23] I've done those two suggestions locally [03:23] ACTION_DESCRIPTIONS[action] is not much worse than get_status(action) :) [03:23] Thanks. [03:24] The only test that still fails is testSourcePackageRecipeBuild_success_mail. Didn't poolie break that on purpose? [03:25] Ah, no. I've broken it so it sends mail [03:30] Ahh. I can't check PU.getSourceBuild() [03:30] :-( [03:32] lifeless, this may sound dumb but when you say property are you referring to Python's @property? just making sure I'm understanding you correctly? [03:35] wgrant: I have those two changes to push, along with the fix for SPRB_success_mail. [03:38] hi wallyworld_ [03:39] StevenK: I'm reworking _sendNotification [03:39] wgrant: Oh, you're going to give me a patch? [03:39] Yes. [03:40] 'cause I was about to shred it in the review. [03:40] Decided I would be a bit kinder. [03:40] Who are you, and what have you done with the real wgrant? [03:43] jtv-afk: hello [03:43] Oh, old nick === jtv-afk is now known as jtv [03:43] wallyworld_: any reviews you need me to mentor? === StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: wallyworld (*jtv) | https://code.launchpad.net/launchpad-project/+activereviews | Critical bugs:209 - 0:[######=_]:256 [03:44] jtv: thanks for asking. i assigned 2 to you. did you not get the email? [03:44] That '- ' was annoying me [03:44] wallyworld_: Probably. But email is one of those "it's got to be in there somewhere" things. [03:44] :-) [03:48] Ah, there they are. [03:53] StevenK: Slow tests :( [03:53] But they seem happy with my +11/-53 so far. [03:56] Crumbs! [03:57] wgrant: My local branch has all of the archiveuploader tests passing [04:00] StevenK: Have some comments. [04:04] wgrant: Your commit messages makes me sad [04:05] StevenK: It sucked years before you got to it. [04:05] True [04:06] timrc: yes, I am [04:06] StevenK: send_mail/_sendMail/_sendNotification/send_message still seem like they're designed to confuse me. Let's see if we can merge/rename some of them. [04:07] And they're not to be confused with sendmail(), which does the actual work! [04:08] wgrant: I'm thinking about solving the two calls to _sendNotification in notify() [04:08] The only difference is the summary [04:08] send_mail can also easily be merged into _sendNotification. [04:10] wgrant: You say the logging of the mail is still duplicated -- however the level is different? [04:11] ... huh. [04:11] Make it all debug. [04:11] If you're doing a dryrun you should be using -v anyway, surely. [04:13] StevenK: What's the difference between the two _sendNotification calls? rejected doesn't send the file summary with OK/NEW/etc.? [04:13] That seems sort of trivial to sort out. [04:14] wgrant: summarystring for !rejected, summary_text for rejected [04:14] Only [04:16] StevenK: Do you like my slight _sendNotification simplification? [04:16] I do [04:16] lifeless, gotcha... so when is it favorable to use a mutator vs. @property out of curiosity? [04:17] wgrant: Re: your "What did it do before?" question, exactly that. [04:17] Huh. [04:17] I hadn't seen that before. [04:17] OK. [04:18] timrc: when clients through the API should do different things than clients inside the appserver. [04:18] wgrant: It was inside notify() [04:20] wgrant: I'd love to drop the hasattr() hack, too [04:20] wgrant: But tests insist on passing in StringIO as the file_object [04:25] StevenK: We should find some other way. [04:25] StevenK: How was it done before? [04:25] Did it use packageupload.changesfile.filename or something? [04:25] lifeless, okay. so if a mutator is used then clients within the appserver would basically need to call setPrivate explicitly rather then .private = boolean [04:25] lifeless, @property would give a common interface for both [04:26] wgrant: It used packageupload.changesfile.filename [04:26] StevenK: :( OK [04:27] StevenK: We may want to rationalise this eventually. [04:27] So we take a filename and either changesfile string or object... not all three. [04:27] Mmmmm [04:28] wgrant: Does http://pastebin.ubuntu.com/613578/ satisfy your "Can haz map?" comment? [04:28] StevenK: It does indeed, but the location could be better. [04:29] * StevenK ponders lunch. [04:30] StevenK: So, do you have any more ideas for merging the Confusing Four? [04:30] send_mail can be pretty trivially folded into _sendNotification. [04:30] I agree about that [04:31] send_message() is also used by notify_spr_less's new name [04:31] _sendNotification can be folded into notify() [04:31] Right. [04:31] StevenK: Hm, notify_spr_less doesn't want to use _sendMail? [04:32] No, it's first argument is an apr [04:32] *SPR [04:32] It looks like it can be optional. [04:32] It says 'if spr:' [04:32] Indeed, it is optional. [04:32] Use it if you can. [04:33] wgrant: I did not see the fallout. [04:33] Then send_message can be folded into there. [04:33] sinzui: It hadn't been sending warnings. [04:33] sinzui: So about 300 memberships expired without warning. [04:33] timrc: right [04:33] sinzui: But only 20ish were in restricted teams that they could have renewed themselves, and I think the owners have fixed those. [04:33] mutators are basically fugly [04:33] wgrant: :( [04:33] lifeless, aye [04:34] I had answered a users question today saying all was fixed. He had noticed his membership was expiring and had not gotten an email [04:35] lifeless: Let's rewrite Launchpad in Java. [04:35] wgrant: indeed, why have one problem when we can have two! [04:35] wgrant, I think we should rewrite it in something more intuitive, like COW (http://www.bigzaphod.org/cow/) [04:37] timrc: But one of the instructions is actually intuitive :( [04:38] wgrant: The log level change is going to cause widespread changes to doctests [04:39] StevenK: Indeed... We'll see how widespread it is. [04:39] StevenK: If it's too hard, use a different log method for each, but still dedupe. [05:28] wgrant: Changes made, and your branch merged. [05:28] +16/-14 for nascent-announcements.txt [05:28] Which is much less than I feared. [05:32] wgrant: _sendNotification folded into notify() [05:32] Project windmill-db-devel build #328: STILL FAILING in 1 hr 9 min: https://lpci.wedontsleep.org/job/windmill-db-devel/328/ [05:35] StevenK: Hexcellent. [05:37] Hmm, we haven't harrassed the LOSAs at all today :( [05:37] We are not up to our usual standard. [05:37] * StevenK looks at merging send_mail() into notify() [05:37] StevenK: Yay [05:37] Er. [05:37] No. [05:38] Indeed, we call it twice [05:38] [13:30] < wgrant> send_mail can be pretty trivially folded into _sendNotification. [05:38] :-) [05:39] StevenK: Oh, right, send_mail, not send_message. [05:39] send_mail can be folded in. [05:39] send_message should be merged into _sendMail, but that first requires that reject_changes_file use the latter instead. [05:41] wgrant: However, we call send_mail twice -- once for annoucements and once for the message itself. [05:42] StevenK: The args are the same except for action, recipients, from_addr and bcc_addr, AFAICT. [05:42] Also, the subject can be shared, but the body cannot. [05:43] Possibly reintroduce the do_send_mail closure. [05:43] But at the end this time. [05:43] Will save like 13 arguments and a confusing function name. [05:53] poolie: you wanted to catch up more generally ? === almaisan-away is now known as al-maisan [05:57] wgrant: notification.py | 18 ++---------------- [05:57] wgrant: From killing send_message [05:58] StevenK: I notice that DSDJob.run finds its DSD by {distroseries, sourcepackagename}. Doesn't that become ambiguous with multi-parent? [05:58] jtv: Yes, that should change. [05:59] wgrant: Make that +2/-19 [05:59] StevenK: do we have a bug/card for that? [05:59] I can't recall off the top of my head [06:00] OK, I'll look. [06:00] StevenK: :) [06:00] StevenK: bug 758906 may be a bit general [06:00] <_mup_> Bug #758906: Allow a derived series to have multiple parents < https://launchpad.net/bugs/758906 > [06:00] AssertionError: Invalid recipient: '' in ['D', 'a', ... [06:00] * StevenK digs [06:00] StevenK: Does that mean that send_mail and send_message are both dead? [06:01] send_mail is not, send_message is [06:01] I am disappoint. [06:01] Of course you are. [06:02] I'm hoping that notification.py drops below 500 lines. [06:02] hi lifeless [06:03] o/ poolie [06:03] wgrant: 509 [06:03] sure [06:03] now if you like [06:03] StevenK: Victory is near. [06:03] poolie: that would be great. shall we try skype? [06:04] wgrant: What was your plan to kill send_mail? [06:09] StevenK: Give me a sec. [06:14] wgrant: My branch has also fallen behind Full HD [06:16] StevenK: Hm, I have an odd failure in distroseriesqueue-notify.txt. [06:16] Have you fixed that one yet? [06:16] I've not been running the soyuz tests [06:19] r13104 broke it. [06:19] :-( [06:20] Ah, that could be the DEBUG vs INFO [06:20] Hm, possibly. [06:20] Doesn't look like it, though. [06:20] I'll dig when archiveuploader stops running [06:21] Also, just found a bug in r13104 [06:21] - for bpr in bprs: [06:21] - names.add(bpr.build.source_package_release.name) [06:21] - version = bpr.build.source_package_release.version [06:21] + elif bprs: [06:21] + names.add(bpr[0].build.source_package_release.name) [06:21] + version = bpr[0].build.source_package_release.version [06:21] You need to add the names even if there are sprs. [06:21] Just not the version. [06:22] Oh. [06:22] Heh. [06:22] It was the DEBUG/INFO change: the unification added some new fields. [06:22] Ah [06:22] Sorry, I fixed that locally [06:23] In r13105 [06:23] Great. [06:23] Do you have a diff for the -notify change? [06:23] Just pushing. [06:23] Or have you not fixed it? [06:24] Oh. [06:24] No, that one I haven't fixed. [06:24] There will be many more like it. [06:24] I'm pushing the send_mail merge. [06:24] Done. [06:24] I've just pushed r13107 [06:24] Hm. [06:25] Have you? [06:25] Pushing [06:25] When that returns, I'll re-merge your branch [06:25] Hm. No progress either [06:26] connect(4, {sa_family=AF_INET, sin_port=htons(443), sin_addr=inet_addr("91.189.89.225")}, 16 [06:26] Odd [06:30] ssh: connect to host bazaar.launchpad.net port 22: Connection timed out [06:30] Hmmm [06:31] But ssh -vv connects [06:31] WFM :( [06:31] So I'm confused [06:31] Ah, there it goes [06:31] wgrant: Sorry, pushed. [06:34] * StevenK attempts to merge in wgrant's latest branch, dies from conflicts [06:34] :( [06:34] wgrant: I can't make sense of them either [06:34] Oh? [06:34] Let me see. [06:34] zomg conflicts [06:35] Oh. [06:35] You demolished _sendNotification. [06:35] I see. [06:36] You said to? :-)( [06:36] s/(// [06:41] StevenK: Pushed. You should just be able to pull. [06:41] 497 lines. [06:42] \o/ [06:44] * StevenK looks at distroseriesqueue [06:45] StevenK: Can you also fix bcc_addr calculation? [06:45] StevenK: It should use the spr name and then fall back to the first bpr name. [06:45] StevenK: And not duplicate the template. [06:47] wgrant: http://pastebin.ubuntu.com/613612/ [06:48] StevenK: elif bprs [06:53] StevenK: I'm still not quite sure how notification.py has ended up 20% shorter, despite some stuff being moved into it from queue.py... [06:53] Haha [06:53] Iz bonus [06:54] wgrant: Fixed distroseriesqueue* [06:55] StevenK: I would avoid further elision like the nascentupload-announcements.txt fix. [06:55] It may conceal unexpected Bcc and From additions. [06:55] :-( [06:56] How did you know that's how I fixed them? [06:56] Heh [06:56] I was actually typing that before you said you'd fixed them :( [06:56] Oh, so since I've done it, I can leave it? :_) [06:57] No. [06:58] Should I fix nascentupload-announcements.txt as well? [06:58] StevenK: Ideally. [06:58] cody-somerville: your bug is fixed; will be deployed monday [07:03] % psql -l | egrep -c 'ftest_[0-9]' [07:03] 8 [07:03] With no test suite running :-/ [07:08] wgrant: distroseriesqueue* fixed properly [07:08] Great. [07:08] I think this is just about done. [07:09] The only thing I can think of is the hasattr() hack [07:09] Yes. [07:09] That's only used by notify(), right? [07:10] An extra arg at that single level won't hurt too much. [07:10] No, reject_changes_file [07:10] Well, yes, but that's called immediately by notify() [07:10] So it doesn't affect the majority of the code. [07:11] Bah, I called it reject_changes_file*s* ? [07:11] :( fixit [07:11] Fixed [07:17] wgrant: r13111 pushing [07:18] Hmm, it's back over Full HD again. === al-maisan is now known as almaisan-away [07:21] wgrant: Diff in the MP has updated, once more with feeling? [07:28] StevenK: Soyuz tests are passing? [07:29] Still running archivepublisher [07:29] But I can let you know in 30 minutes :-( [07:30] So, I'll rereview in a sec. [07:30] wgrant: Running soyuz now, no failures in archivepublisher [07:30] What are your next steps? [07:30] After this branch? [07:31] I was going to say hook this into the package copier, but that is dangerous [07:31] It might be worth seeing how hard it is to extract reject_changes_file() from notify(), making archiveuploader call it directly. [07:31] But if you want to go directly to the copier goal, you'll need to wean notify() and co. off the changesfile. [07:31] I tried that, archiveuploader deals with both forms of rejection in one method [07:31] Stop using it for Changed-By and Maintainer, for example. [07:31] Bah. [07:31] That's stupid. [07:32] But OK. [07:32] My concern with the package copier calling it is that notify() is fairly heavyweight, and the package copier needs to be lean and fast [07:33] Does it? [07:33] Otherwise copying times out and people yell at me. :-( [07:33] These copies won't be done inline. [07:33] Ahh [07:34] Well, for security copies they probably will for the short-term. [07:34] So call notify() in PCJ, *not* do_copy [07:34] But this is pretty light-weight... [07:34] No, do_copy. [07:34] But only security copies and PCJ will use it for now. [07:34] That's fairly easy guarded [07:34] On a vaguely related note, did the job to pro[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[C[Ccess all the old copyright files ever run? [07:34] No need. [07:34] The other types of copies are unsupported. [07:35] persia: Old changelog files? Yes. === almaisan-away is now known as al-maisan [07:35] Oh, excellent [07:35] persia: It took one month to run [07:35] persia: And processed roughly 580,000 SPRs. [07:35] Only 'cause you threw away my good one :P [07:35] StevenK: It took about one year to schedule, so I'm happy with the run time :) [07:35] So we thanked it by deleting it from the codebase. [07:35] wgrant: I got lifeless'd [07:36] wgrant: And I didn't throw it away, it mutated ... [07:36] wgrant: Uh? What about ppa-to-ppa copies? [07:36] StevenK: They shouldn't announce. [07:37] We will handle that in the integration. [07:37] Clearly [07:37] But it should be handled with the rest of the do-not-announce logic. [07:37] Not by not calling notify(). [07:37] Right [07:37] wgrant: Are you reviewing, or waiting for soyuz test results? [07:38] Or doing something else entirely :-) [07:38] Will look shortly. [08:07] wgrant: 2 failures in soyzu [08:07] *soyuz [08:07] They're both doctests, so I guess it's more INFO vs DEBUG fallout [08:22] StevenK: Have you disposed of _filename yet? [08:24] wgrant: Yes [08:24] Trying to work out the xx-queue-pages.txt failure [08:24] soyuz-set-of-uploads failed due to INFO vs DEBUG [08:26] The rejection message seems to be None. [08:26] Yes, I think I've found the bug. [08:27] And found a stray place _filename hasn't been erricated yet [08:31] wgrant: r13112 pushed. [08:32] wgrant: Did you notice my drive-by to drop pocketsuffix from queue? === al-maisan is now known as almaisan-away [08:32] StevenK: I WTF'd at it for a minute and then worked out it was a drive-by, yes. [08:32] *Not* really the right sort of branch for drive-bys, but I won't hurt you too much. [08:33] Well, true, it's large and hurty enought [08:33] s/t$// [08:34] wgrant: Diff updated. [08:40] StevenK: The summarystring stuff could do with a shuffle. [08:40] wgrant: Er, which bit? [08:40] I'd move the buildSummary bit (line 144) into an else block after the "if action == 'rejected'" [08:40] Since the rejected bit clobbers it. [08:42] wgrant: Done [08:51] StevenK: Could you also de-underscore and de-CamelCase the remaining functions? [08:51] Apart from that I think it is pretty much landable. [08:51] We will see Julian scream in a few minutes, I guess? [08:56] How do? [08:59] wgrant: Does mean you're willing to Approve it? [09:00] s/s m/s that m/ === jtv1 is now known as jtv [09:01] huwshimi: hi [09:01] jml: Hello! [09:02] wgrant: r13113 pushed. [09:02] huwshimi: let's talk :) [09:02] jml: Skype? [09:05] wallyworld_: Are you still the OCR? [09:05] StevenK: perhaps not anymore [09:05] i should change the topic [09:05] * wallyworld_ is about to eat dinner === wallyworld_ changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: | Critical bugs:209 - 0:[######=_]:256 === StevenK changed the topic of #launchpad-dev to: https://dev.launchpad.net/ | On call reviewer: - | Critical bugs:209 - 0:[######=_]:256 [09:06] I wonder if that 209 count is still right [09:07] It's 210 now, I think. [09:07] Possibly more. [09:07] it seems to be stuck around 200 :-( [09:07] It's decreasing. [09:07] But then stuff gets escalated. [09:08] right [09:08] http://webnumbr.com/launchpad-critical-bugs [09:08] nice graph. i hadn't seen that before [09:10] There are 9 escalated bugs. [09:10] wgrant: The diff has updated, can haz comment/approve? [09:11] bigjools must be subjected to it first. [09:11] bigjools: https://code.launchpad.net/~stevenk/launchpad/refactor-notification/+merge/61919 [09:12] bzr repacking a commit when you just want to push before your battery runs out is sub optimal [09:12] hey wallyworld_ you're up late [09:12] goodmorning [09:13] The notification.py diff is pretty much useless, but the new file is short and pretty readable. [09:13] And self-contained. [09:19]