Merge lp:~cjwatson/launchpad/opt-in-zopeless-immediate-mail into lp:launchpad

Proposed by Colin Watson on 2015-09-08
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/opt-in-zopeless-immediate-mail
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/more-pop-notifications
Diff against target: 421 lines (+67/-49)
14 files modified
daemons/buildd-manager.tac (+2/-8)
lib/lp/bugs/scripts/bugnotification.py (+11/-2)
lib/lp/bugs/scripts/tests/test_bugnotification.py (+1/-1)
lib/lp/bugs/tests/bugs-emailinterface.txt (+6/-11)
lib/lp/code/mail/codehandler.py (+1/-1)
lib/lp/services/job/celeryjob.py (+1/-3)
lib/lp/services/job/runner.py (+8/-7)
lib/lp/services/mail/basemailer.py (+7/-2)
lib/lp/services/mail/sendmail.py (+13/-1)
lib/lp/services/mail/tests/incomingmail.txt (+6/-2)
lib/lp/services/mail/tests/test_incoming.py (+2/-0)
lib/lp/services/scripts/base.py (+1/-6)
lib/lp/testing/layers.py (+1/-5)
scripts/mlist-import.py (+7/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/opt-in-zopeless-immediate-mail
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2015-09-08 Pending
Review via email: mp+270383@code.launchpad.net

Commit Message

Disable Zopeless immediate mail delivery, except for BaseMailer, job OOPS/error notifications, and a few other places that still need it.

Description of the Change

Disable Zopeless immediate mail delivery, except for BaseMailer, job OOPS/error notifications, and a few other places that still need it.

This is a step towards being able to disable it across the board so that we can avoid ever having situations where operation/mail/operation/mail sequences send duplicate mail if a non-first operation fails and has to be retried. There are still several things that need to be fixed, notably archiveuploader; but this branch at least means that it's disabled for tests by default, which will simplify the process of fixing archiveuploader.

To post a comment you must log in.

Unmerged revisions

17726. By Colin Watson on 2015-09-08

Drop immediate mail delivery from LaunchpadScript and LaunchpadZopelessLayer.

17725. By Colin Watson on 2015-09-08

Drop explicit immediate mail delivery from buildd-manager; build mail all goes via BaseMailer now, which handles that.

17724. By Colin Watson on 2015-09-08

Drop immediate mail delivery from jobs, except for OOPS and user error notifications.

17723. By Colin Watson on 2015-09-08

Fix CodeHandler to abort before sending error notifications.

17722. By Colin Watson on 2015-09-08

Force immediate mail delivery for MailingListImport.

17721. By Colin Watson on 2015-09-08

Force immediate mail delivery for bug notifications and BaseMailer, which rely on this for SMTP exception handling.

17720. By Colin Watson on 2015-09-08

Adjust some tests to stop assuming immediate mail delivery.

17719. By Colin Watson on 2015-09-08

Introduce an immediate_mail_delivery context manager, allowing more fine-grained control.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'daemons/buildd-manager.tac'
2--- daemons/buildd-manager.tac 2011-12-29 05:29:36 +0000
3+++ daemons/buildd-manager.tac 2015-09-08 11:52:24 +0000
4@@ -1,4 +1,4 @@
5-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
6+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
7 # GNU Affero General Public License version 3 (see the file LICENSE).
8
9 # Twisted Application Configuration file.
10@@ -6,20 +6,15 @@
11
12 from twisted.application import service
13 from twisted.scripts.twistd import ServerOptions
14-from twisted.web import server
15
16+from lp.buildmaster.manager import BuilddManager
17 from lp.services.config import dbconfig
18 from lp.services.daemons import readyservice
19 from lp.services.scripts import execute_zcml_for_scripts
20-from lp.buildmaster.manager import BuilddManager
21-from lp.services.mail.sendmail import set_immediate_mail_delivery
22 from lp.services.twistedsupport.loggingsupport import RotatableFileLogObserver
23
24 execute_zcml_for_scripts()
25 dbconfig.override(dbuser='buildd_manager', isolation_level='read_committed')
26-# XXX wgrant 2011-09-24 bug=29744: initZopeless used to do this.
27-# Should be removed from callsites verified to not need it.
28-set_immediate_mail_delivery(True)
29
30 options = ServerOptions()
31 options.parseOptions()
32@@ -34,4 +29,3 @@
33 # Service for scanning buildd slaves.
34 service = BuilddManager()
35 service.setServiceParent(application)
36-
37
38=== modified file 'lib/lp/bugs/scripts/bugnotification.py'
39--- lib/lp/bugs/scripts/bugnotification.py 2015-08-06 12:09:28 +0000
40+++ lib/lp/bugs/scripts/bugnotification.py 2015-09-08 11:52:24 +0000
41@@ -35,7 +35,10 @@
42 from lp.services.database.constants import UTC_NOW
43 from lp.services.mail.helpers import get_email_template
44 from lp.services.mail.mailwrapper import MailWrapper
45-from lp.services.mail.sendmail import sendmail
46+from lp.services.mail.sendmail import (
47+ immediate_mail_delivery,
48+ sendmail,
49+ )
50 from lp.services.scripts.base import LaunchpadCronScript
51 from lp.services.scripts.logger import log
52 from lp.services.webapp import canonical_url
53@@ -339,7 +342,7 @@
54
55
56 class SendBugNotifications(LaunchpadCronScript):
57- def main(self):
58+ def send_notifications(self):
59 notifications_sent = False
60 bug_notification_set = getUtility(IBugNotificationSet)
61 deferred_notifications = \
62@@ -392,3 +395,9 @@
63
64 if not notifications_sent:
65 self.logger.debug("No notifications are pending to be sent.")
66+
67+ def main(self):
68+ # XXX cjwatson 2015-09-05 bug=29744: SMTP exception handling here
69+ # currently relies on this.
70+ with immediate_mail_delivery(True):
71+ self.send_notifications()
72
73=== modified file 'lib/lp/bugs/scripts/tests/test_bugnotification.py'
74--- lib/lp/bugs/scripts/tests/test_bugnotification.py 2015-08-06 12:09:28 +0000
75+++ lib/lp/bugs/scripts/tests/test_bugnotification.py 2015-09-08 11:52:24 +0000
76@@ -1368,7 +1368,7 @@
77 "send-bug-notifications", config.malone.bugnotification_dbuser,
78 ["-q"])
79 script.txn = self.layer.txn
80- script.main()
81+ script.send_notifications()
82
83 self.assertEqual(1, len(self.oopses))
84 self.assertIn(
85
86=== modified file 'lib/lp/bugs/tests/bugs-emailinterface.txt'
87--- lib/lp/bugs/tests/bugs-emailinterface.txt 2015-07-21 09:04:01 +0000
88+++ lib/lp/bugs/tests/bugs-emailinterface.txt 2015-09-08 11:52:24 +0000
89@@ -81,8 +81,8 @@
90
91 >>> def process_email(raw_mail):
92 ... msg = construct_email(raw_mail)
93- ... handler.process(msg, msg['To'],
94- ... )
95+ ... handler.process(msg, msg['To'])
96+ ... transaction.commit()
97
98 >>> process_email(submit_mail)
99
100@@ -454,9 +454,7 @@
101 ... signature = None
102 >>> msg = email.message_from_string(
103 ... comment_mail, _class=MockUnsignedMessage)
104- >>> handler.process(
105- ... msg, msg['To'],
106- ... )
107+ >>> handler.process(msg, msg['To'])
108 True
109 >>> transaction.commit()
110
111@@ -514,9 +512,7 @@
112 ... return construct_email(edit_mail)
113
114 >>> def submit_command_email(msg):
115- ... handler.process(
116- ... msg, msg['To'],
117- ... )
118+ ... handler.process(msg, msg['To'])
119 ... transaction.commit()
120
121 >>> def submit_commands(bug, *commands):
122@@ -1805,10 +1801,9 @@
123 >>> msg = signed_message_from_string(submit_mail)
124 >>> import email.utils
125 >>> msg['Message-Id'] = email.utils.make_msgid()
126- >>> handler.process(
127- ... msg, msg['To'],
128- ... )
129+ >>> handler.process(msg, msg['To'])
130 True
131+ >>> transaction.commit()
132 >>> print_latest_email()
133 Subject: Submit Request Failure
134 To: test@canonical.com
135
136=== modified file 'lib/lp/code/mail/codehandler.py'
137--- lib/lp/code/mail/codehandler.py 2015-07-08 16:05:11 +0000
138+++ lib/lp/code/mail/codehandler.py 2015-09-08 11:52:24 +0000
139@@ -316,11 +316,11 @@
140 message, context.vote, context.vote_tags, mail)
141
142 except IncomingEmailError as error:
143+ transaction.abort()
144 send_process_error_notification(
145 str(user.preferredemail.email),
146 'Submit Request Failure',
147 error.message, mail, error.failing_command)
148- transaction.abort()
149 return True
150
151 @staticmethod
152
153=== modified file 'lib/lp/services/job/celeryjob.py'
154--- lib/lp/services/job/celeryjob.py 2015-08-03 07:06:45 +0000
155+++ lib/lp/services/job/celeryjob.py 2015-09-08 11:52:24 +0000
156@@ -1,4 +1,4 @@
157-# Copyright 2012 Canonical Ltd. This software is licensed under the
158+# Copyright 2012-2015 Canonical Ltd. This software is licensed under the
159 # GNU Affero General Public License version 3 (see the file LICENSE).
160
161 """Celery-specific Job code.
162@@ -34,7 +34,6 @@
163 install_feature_controller,
164 make_script_feature_controller,
165 )
166-from lp.services.mail.sendmail import set_immediate_mail_delivery
167 from lp.services.job.model.job import (
168 Job,
169 UniversalJobSource,
170@@ -139,7 +138,6 @@
171 return
172 transaction.abort()
173 scripts.execute_zcml_for_scripts(use_web_security=False)
174- set_immediate_mail_delivery(True)
175 needs_zcml = False
176
177
178
179=== modified file 'lib/lp/services/job/runner.py'
180--- lib/lp/services/job/runner.py 2015-08-04 00:19:36 +0000
181+++ lib/lp/services/job/runner.py 2015-09-08 11:52:24 +0000
182@@ -1,4 +1,4 @@
183-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
184+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
185 # GNU Affero General Public License version 3 (see the file LICENSE).
186
187 """Facilities for running Jobs."""
188@@ -72,8 +72,8 @@
189 IRunnableJob,
190 )
191 from lp.services.mail.sendmail import (
192+ immediate_mail_delivery,
193 MailController,
194- set_immediate_mail_delivery,
195 )
196 from lp.services.twistedsupport import run_reactor
197 from lp.services.webapp import errorlog
198@@ -177,7 +177,9 @@
199 """Report this oops."""
200 ctrl = self.getOopsMailController(oops['id'])
201 if ctrl is not None:
202- ctrl.send()
203+ # XXX cjwatson 2015-09-06 bug=29744: Can this be removed?
204+ with immediate_mail_delivery(True):
205+ ctrl.send()
206
207 def getOopsVars(self):
208 """See `IRunnableJob`."""
209@@ -187,7 +189,9 @@
210 """See `IRunnableJob`."""
211 ctrl = self.getUserErrorMailController(e)
212 if ctrl is not None:
213- ctrl.send()
214+ # XXX cjwatson 2015-09-06 bug=29744: Can this be removed?
215+ with immediate_mail_delivery(True):
216+ ctrl.send()
217
218 def makeOopsReport(self, oops_config, info):
219 """Generate an OOPS report using the given OOPS configuration."""
220@@ -406,9 +410,6 @@
221 scripts.execute_zcml_for_scripts(use_web_security=False)
222 signal(SIGHUP, handler)
223 dbconfig.override(dbuser=cls.dbuser, isolation_level='read_committed')
224- # XXX wgrant 2011-09-24 bug=29744: initZopeless used to do this.
225- # Should be removed from callsites verified to not need it.
226- set_immediate_mail_delivery(True)
227
228 @staticmethod
229 def __exit__(exc_type, exc_val, exc_tb):
230
231=== modified file 'lib/lp/services/mail/basemailer.py'
232--- lib/lp/services/mail/basemailer.py 2015-08-25 14:05:24 +0000
233+++ lib/lp/services/mail/basemailer.py 2015-09-08 11:52:24 +0000
234@@ -20,6 +20,7 @@
235 from lp.services.mail.sendmail import (
236 append_footer,
237 format_address,
238+ immediate_mail_delivery,
239 MailController,
240 )
241 from lp.services.utils import text_delta
242@@ -204,8 +205,12 @@
243
244 def sendAll(self):
245 """Send notifications to all recipients."""
246- for email, recipient in sorted(self._recipients.getRecipientPersons()):
247- self.sendOne(email, recipient)
248+ # XXX cjwatson 2015-09-05 bug=29744: We currently depend on this to
249+ # handle SMTPExceptions in the correct place.
250+ with immediate_mail_delivery(True):
251+ for email, recipient in sorted(
252+ self._recipients.getRecipientPersons()):
253+ self.sendOne(email, recipient)
254
255
256 class RecipientReason:
257
258=== modified file 'lib/lp/services/mail/sendmail.py'
259--- lib/lp/services/mail/sendmail.py 2015-07-21 09:04:01 +0000
260+++ lib/lp/services/mail/sendmail.py 2015-09-08 11:52:24 +0000
261@@ -1,4 +1,4 @@
262-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
263+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
264 # GNU Affero General Public License version 3 (see the file LICENSE).
265
266 """The One True Way to send mail from the Launchpad application.
267@@ -19,6 +19,7 @@
268 'format_address',
269 'format_address_for_person',
270 'get_msgid',
271+ 'immediate_mail_delivery',
272 'MailController',
273 'sendmail',
274 'set_immediate_mail_delivery',
275@@ -29,6 +30,7 @@
276
277
278 from binascii import b2a_qp
279+from contextlib import contextmanager
280 from email import charset
281 from email.encoders import encode_base64
282 from email.header import Header
283@@ -178,6 +180,16 @@
284 _immediate_mail_delivery = enabled
285
286
287+@contextmanager
288+def immediate_mail_delivery(enabled):
289+ """Context manager to temporarily change immediate mail delivery."""
290+ global _immediate_mail_delivery
291+ previous = _immediate_mail_delivery
292+ set_immediate_mail_delivery(enabled)
293+ yield
294+ set_immediate_mail_delivery(previous)
295+
296+
297 def simple_sendmail(from_addr, to_addrs, subject, body, headers=None,
298 bulk=True):
299 """Send an email from from_addr to to_addrs with the subject and body
300
301=== modified file 'lib/lp/services/mail/tests/incomingmail.txt'
302--- lib/lp/services/mail/tests/incomingmail.txt 2015-09-08 11:52:24 +0000
303+++ lib/lp/services/mail/tests/incomingmail.txt 2015-09-08 11:52:24 +0000
304@@ -45,16 +45,20 @@
305
306 Now we send a few test mails to foo.com, bar.com, and baz.com:
307
308+ >>> import transaction
309 >>> from lp.services.mail.tests.helpers import read_test_message
310 >>> from lp.services.mail.sendmail import sendmail as original_sendmail
311 >>> from lp.testing.dbuser import switch_dbuser
312
313 For these examples, we don't want the Precedence header added. Domains
314 are treated without regard to case: for incoming mail, foo.com and
315-FOO.COM are treated equivalently.
316+FOO.COM are treated equivalently. We also commit to ensure that mail is
317+sent immediately.
318
319 >>> def sendmail(msg, to_addrs=None):
320- ... return original_sendmail(msg, to_addrs=to_addrs, bulk=False)
321+ ... msgid = original_sendmail(msg, to_addrs=to_addrs, bulk=False)
322+ ... transaction.commit()
323+ ... return msgid
324
325 >>> switch_dbuser('launchpad')
326 >>> msgids = {'foo.com': [], 'bar.com': [], 'baz.com': []}
327
328=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
329--- lib/lp/services/mail/tests/test_incoming.py 2015-07-08 16:05:11 +0000
330+++ lib/lp/services/mail/tests/test_incoming.py 2015-09-08 11:52:24 +0000
331@@ -75,6 +75,7 @@
332 email_address, 'to@example.com', 'subject', invalid_body,
333 bulk=False)
334 ctrl.send()
335+ transaction.commit()
336 handleMail()
337 self.assertEqual([], self.oopses)
338 [notification] = pop_notifications()
339@@ -101,6 +102,7 @@
340 email_address, 'to@example.com', 'subject', fat_body,
341 bulk=False)
342 ctrl.send()
343+ transaction.commit()
344 handleMail()
345 self.assertEqual([], self.oopses)
346 [notification] = pop_notifications()
347
348=== modified file 'lib/lp/services/scripts/base.py'
349--- lib/lp/services/scripts/base.py 2014-08-29 01:34:04 +0000
350+++ lib/lp/services/scripts/base.py 2015-09-08 11:52:24 +0000
351@@ -1,4 +1,4 @@
352-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
353+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
354 # GNU Affero General Public License version 3 (see the file LICENSE).
355
356 __metaclass__ = type
357@@ -44,7 +44,6 @@
358 install_feature_controller,
359 make_script_feature_controller,
360 )
361-from lp.services.mail.sendmail import set_immediate_mail_delivery
362 from lp.services.scripts.interfaces.scriptactivity import IScriptActivitySet
363 from lp.services.scripts.logger import OopsHandler
364 from lp.services.webapp.errorlog import globalErrorUtility
365@@ -307,10 +306,6 @@
366 self._init_zca(use_web_security=use_web_security)
367 self._init_db(isolation=isolation)
368
369- # XXX wgrant 2011-09-24 bug=29744: initZopeless used to do this.
370- # Should be called directly by scripts that actually need it.
371- set_immediate_mail_delivery(True)
372-
373 date_started = datetime.datetime.now(UTC)
374 profiler = None
375 if self.options.profile:
376
377=== modified file 'lib/lp/testing/layers.py'
378--- lib/lp/testing/layers.py 2013-08-14 11:15:51 +0000
379+++ lib/lp/testing/layers.py 2015-09-08 11:52:24 +0000
380@@ -1,4 +1,4 @@
381-# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
382+# Copyright 2009-2015 Canonical Ltd. This software is licensed under the
383 # GNU Affero General Public License version 3 (see the file LICENSE).
384
385 """Layers used by Launchpad tests.
386@@ -1498,10 +1498,6 @@
387 @profiled
388 def testSetUp(cls):
389 dbconfig.override(isolation_level='read_committed')
390- # XXX wgrant 2011-09-24 bug=29744: initZopeless used to do this.
391- # Tests that still need it should eventually set this directly,
392- # so the whole layer is not polluted.
393- set_immediate_mail_delivery(True)
394
395 # Connect Storm
396 reconnect_stores()
397
398=== modified file 'scripts/mlist-import.py'
399--- scripts/mlist-import.py 2013-01-07 02:40:55 +0000
400+++ scripts/mlist-import.py 2015-09-08 11:52:24 +0000
401@@ -24,6 +24,7 @@
402
403 from lp.registry.scripts.mlistimport import Importer
404 from lp.services.config import config
405+from lp.services.mail.sendmail import set_immediate_mail_delivery
406 from lp.services.scripts.base import LaunchpadScript
407
408
409@@ -56,6 +57,12 @@
410
411 def main(self):
412 """See `LaunchpadScript`."""
413+ # XXX cjwatson 2015-09-06 bug=29744: We need immediate mail delivery
414+ # for the current implementation of the --notifications switch, and
415+ # because the tests expect notifications to end up in
416+ # LayerProcessController.smtp_controller.
417+ set_immediate_mail_delivery(True)
418+
419 team_name = None
420 if len(self.args) == 0:
421 self.parser.error('Missing team name')