Launchpad itself

Merge lp://staging/~jtv/launchpad/bug-410293 into lp://staging/launchpad

  1. bug-410293
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp://staging/~jtv/launchpad/bug-410293
Merge into: lp://staging/launchpad
Diff against target: None lines
To merge this branch: bzr merge lp://staging/~jtv/launchpad/bug-410293
Reviewer Review Type Date Requested Status
Henning Eggers (community) code Approve
Review via email: mp+11115@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

= Bug 410293 =

This branch is meant for cherry-picking. Its fix may be crucial for the
Karmic translation process.

The translations importer mails out notifications about imports it has
processed. Recently this has been failing a lot with assertion failures
about the outgoing email having no "To" address.

To some extent the importer is prepared for this. It sends the email in
a separate transaction, ensuring that the offending file is fully
processed before the failure happens and won't be the same spanner in
the works on the next run.

On the other hand, an assertion failure does stop the script, halting
all imports until the next script run up to 10 minutes later. We've
been seeing a terrible slowdown over the past two days, roughly from
2009-01-01 15:00 to 2009-09-02 23:00 (times may be BST or UTC; we're not
quite sure). After that things got going again of their own volition,
but a two-hour relapse shows that we are still not safe.

Here you see a simple fix: only send out emails if there are email
addresses to send the email to. It is not guaranteed that a user will
have a valid email address at any given time after registration.

An added test attempts to import a malformed file imported by a user
without valid email address. The importer notes the error but has no
email address to notify. Without the fix it aborts as in production;
with the fix it completes normally. (Of course the malformed file still
fails to import).

Jeroen

Revision history for this message
Henning Eggers (henninge) wrote :

Good job, Kermit! ;-)

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/doc/poimport.txt'
2--- lib/lp/translations/doc/poimport.txt 2009-08-13 15:12:16 +0000
3+++ lib/lp/translations/doc/poimport.txt 2009-09-03 09:10:49 +0000
4@@ -1000,3 +1000,42 @@
5 True
6 >>> subject
7 u'Import problem - Serbian (sr) - ...'
8+
9+
10+No Contact Address
11+------------------
12+
13+Not every user has a valid email address. For instance, Kermit the
14+Hermit has none at the moment.
15+
16+ >>> from canonical.launchpad.interfaces.emailaddress import (
17+ ... EmailAddressStatus)
18+ >>> from canonical.launchpad.helpers import get_contact_email_addresses
19+ >>> hermit = factory.makePerson(
20+ ... name='hermit', email_address_status=EmailAddressStatus.OLD)
21+
22+ >>> len(get_contact_email_addresses(hermit))
23+ 0
24+
25+Kermit uploads a translation, which gets approved.
26+
27+ >>> pofile = factory.makePOFile('lo')
28+
29+ >>> entry = translation_import_queue.addOrUpdateEntry(
30+ ... 'lo.po', 'Invalid content', True, hermit,
31+ ... pofile=pofile, potemplate=pofile.potemplate,
32+ ... productseries=pofile.potemplate.productseries)
33+ >>> entry.setStatus(RosettaImportStatus.APPROVED)
34+ >>> transaction.commit()
35+
36+The import fails. The importer would like to send Kermit an email about
37+this, but is unable to. This is unfortunate, but does not faze the
38+importer. It completes normally.
39+
40+ >>> process = ImportProcess(transaction, FakeLogger())
41+ >>> process.run()
42+ INFO Importing: Lao ...
43+ INFO Import requests completed.
44+
45+ >>> print entry.status.name
46+ FAILED
47
48=== modified file 'lib/lp/translations/scripts/po_import.py'
49--- lib/lp/translations/scripts/po_import.py 2009-07-17 00:26:05 +0000
50+++ lib/lp/translations/scripts/po_import.py 2009-09-03 09:10:49 +0000
51@@ -116,23 +116,26 @@
52 else:
53 to_email = helpers.get_contact_email_addresses(
54 entry_to_import.importer)
55- text = MailWrapper().format(mail_body)
56-
57- # XXX: JeroenVermeulen 2007-11-29 bug=29744: email
58- # isn't transactional in zopeless mode. That
59- # means that our current transaction can fail
60- # after we already sent out a success notification.
61- # To prevent that, we commit the import (attempt)
62- # before sending out the email. That way, the worst
63- # that can happen is that an email goes missing.
64- # Once bug 29744 is fixed, this commit must die so
65- # the email and the import will be in a single
66- # atomic operation.
67- self.ztm.commit()
68- self.ztm.begin()
69-
70- simple_sendmail(
71- from_email, to_email, mail_subject, text)
72+
73+ if to_email:
74+ text = MailWrapper().format(mail_body)
75+
76+ # XXX: JeroenVermeulen 2007-11-29 bug=29744: email
77+ # isn't transactional in zopeless mode. That
78+ # means that our current transaction can fail
79+ # after we already sent out a success
80+ # notification. To prevent that, we commit the
81+ # import (attempt) before sending out the email.
82+ # That way the worst that can happen is that an
83+ # email goes missing.
84+ # Once bug 29744 is fixed, this commit must die so
85+ # the email and the import will be in a single
86+ # atomic operation.
87+ self.ztm.commit()
88+ self.ztm.begin()
89+
90+ simple_sendmail(
91+ from_email, to_email, mail_subject, text)
92
93 except KeyboardInterrupt:
94 self.ztm.abort()