[23:03] wgrant: Have you had a chance to figure out how to QA https://bugs.launchpad.net/launchpad/+bug/1489674 ? [23:03] Bug #1489674: BugNotificationBuilder.build crashes with Unauthorized on invisible private teams <403>

[23:03] cjwatson: Oh yeah, I should try that. [23:04] Let me see. [23:05] wgrant: Thanks [23:05] blr: Were you going to land https://code.launchpad.net/~blr/launchpad/diff-code-select-bug-1483925/+merge/268017 ? [23:05] cjwatson: oh, forgot about that. Yes I should do [23:06] I figured that was probably buried in flu. [23:06] thanks cjwatson [23:36] wgrant: A quick test suggests that there are various BaseMailer call sites that could trigger this too, for example modifying a branch in the web UI that has a subscription from an inaccessible private team, so I think my first job tomorrow is going to be figuring out a central fix for that. [23:37] Or as central as possible. [23:41] cjwatson: Sounds reasonable. I don't know how central it can be, but it needs investigation. [23:42] Maybe BaseMailer needs to grow a rule that all uses of it need to be zopeless-style. Fixing this "centrally" would still involve sprinkling rSP over all the various constructors. [23:42] Zopeless-style? [23:42] Jobs, scripts, or whatever. Not with a user interaction. [23:42] Whatever the correct term is. [23:43] Ah, right. The defining feature is PermissiveSecurityPolicy. [23:43] But that makes it rather difficult to use from the large number of email callsites that aren't presently asynchronous, unfortunately. [23:43] That's the one, thanks. [23:44] Making those async would not be too hard nowadays. [23:44] I'll have to see whether it's worth it. [23:45] I always thought sending mail directly from something like a page rendering operation was a bit odd, though. [23:46] Oh, it would certainly be a positive change. [23:46] The question is how much work it is to fix. [23:47] I could stick an assert getSecurityPolicy() == PermissiveSecurityPolicy into BaseMailer and see how much of the test suite explodes. [23:48] All of +queue, for one. [23:50] From a quick scan I think the currently affected mailers are Branch, BranchMergeProposal, GitRepository, and PackageUpload. All the rest are only used from jobs or scripts. [23:51] oh right, answers is async nowadays, isn't it. [23:51] Answers doesn't use BaseMailer (yet). [23:51] oh [23:51] But I think its mail notifications are indeed async. [23:52] Bugs notifications are mostly async, except for the send_bug_details_to_new_bug_subscribers case you ran into. [23:53] If by "mostly async" you mean "lies" [23:53] The footer is calculated in the webapp. [23:53] Very, very expensively. [23:53] Right, but only in the new-bug case. [23:53] Most bug edit timeouts go away if we make notification sending properly async. [23:53] Er, new-bug-subscriber. [23:53] Nope. [23:54] The webapp populates BugNotificationRecipient [23:54] Oh, yikes. [23:54] So the POST request calculates the set of people who should be notified and inserts a row for each of them, including the rationale text and footer etc. [23:55] The correct solution is of course a job that says "this change happened to the bug when it was in this state, now please populate bugnotification" [23:56] I'll have a look at the BaseMailer cases tomorrow at least. Would like to spend finite time in this swamp, but draining a bit of it is a prereq for landing team-mail safely [23:56] (I strongly suspect) [23:56] Anyway, bed. Night [23:56] Night. [23:56] You weren't even here today, were you? [23:56] Nope. And mostly actually not, just nipping in briefly [23:57] If you can make it all PermissiveSecurityPolicy then that indeed makes things substantially simpler. [23:57] Code has a suitable job framework already, so probably easy. Soyuz might need a bit of fiddling. [23:57] Yup [23:58] Though there's actually already a bug about the Soyuz one. [23:58] https://bugs.launchpad.net/launchpad/+bug/566339 [23:58] Bug #566339: Cannot accept package which would notify private email addresses