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
=== modified file 'lib/lp/translations/doc/poimport.txt'
--- lib/lp/translations/doc/poimport.txt 2009-08-13 15:12:16 +0000
+++ lib/lp/translations/doc/poimport.txt 2009-09-03 09:10:49 +0000
@@ -1000,3 +1000,42 @@
1000 True1000 True
1001 >>> subject1001 >>> subject
1002 u'Import problem - Serbian (sr) - ...'1002 u'Import problem - Serbian (sr) - ...'
1003
1004
1005No Contact Address
1006------------------
1007
1008Not every user has a valid email address. For instance, Kermit the
1009Hermit has none at the moment.
1010
1011 >>> from canonical.launchpad.interfaces.emailaddress import (
1012 ... EmailAddressStatus)
1013 >>> from canonical.launchpad.helpers import get_contact_email_addresses
1014 >>> hermit = factory.makePerson(
1015 ... name='hermit', email_address_status=EmailAddressStatus.OLD)
1016
1017 >>> len(get_contact_email_addresses(hermit))
1018 0
1019
1020Kermit uploads a translation, which gets approved.
1021
1022 >>> pofile = factory.makePOFile('lo')
1023
1024 >>> entry = translation_import_queue.addOrUpdateEntry(
1025 ... 'lo.po', 'Invalid content', True, hermit,
1026 ... pofile=pofile, potemplate=pofile.potemplate,
1027 ... productseries=pofile.potemplate.productseries)
1028 >>> entry.setStatus(RosettaImportStatus.APPROVED)
1029 >>> transaction.commit()
1030
1031The import fails. The importer would like to send Kermit an email about
1032this, but is unable to. This is unfortunate, but does not faze the
1033importer. It completes normally.
1034
1035 >>> process = ImportProcess(transaction, FakeLogger())
1036 >>> process.run()
1037 INFO Importing: Lao ...
1038 INFO Import requests completed.
1039
1040 >>> print entry.status.name
1041 FAILED
10031042
=== modified file 'lib/lp/translations/scripts/po_import.py'
--- lib/lp/translations/scripts/po_import.py 2009-07-17 00:26:05 +0000
+++ lib/lp/translations/scripts/po_import.py 2009-09-03 09:10:49 +0000
@@ -116,23 +116,26 @@
116 else:116 else:
117 to_email = helpers.get_contact_email_addresses(117 to_email = helpers.get_contact_email_addresses(
118 entry_to_import.importer)118 entry_to_import.importer)
119 text = MailWrapper().format(mail_body)119
120120 if to_email:
121 # XXX: JeroenVermeulen 2007-11-29 bug=29744: email121 text = MailWrapper().format(mail_body)
122 # isn't transactional in zopeless mode. That122
123 # means that our current transaction can fail123 # XXX: JeroenVermeulen 2007-11-29 bug=29744: email
124 # after we already sent out a success notification.124 # isn't transactional in zopeless mode. That
125 # To prevent that, we commit the import (attempt)125 # means that our current transaction can fail
126 # before sending out the email. That way, the worst126 # after we already sent out a success
127 # that can happen is that an email goes missing.127 # notification. To prevent that, we commit the
128 # Once bug 29744 is fixed, this commit must die so128 # import (attempt) before sending out the email.
129 # the email and the import will be in a single129 # That way the worst that can happen is that an
130 # atomic operation.130 # email goes missing.
131 self.ztm.commit()131 # Once bug 29744 is fixed, this commit must die so
132 self.ztm.begin()132 # the email and the import will be in a single
133133 # atomic operation.
134 simple_sendmail(134 self.ztm.commit()
135 from_email, to_email, mail_subject, text)135 self.ztm.begin()
136
137 simple_sendmail(
138 from_email, to_email, mail_subject, text)
136139
137 except KeyboardInterrupt:140 except KeyboardInterrupt:
138 self.ztm.abort()141 self.ztm.abort()