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

[Jeroen T. Vermeulen]

[Jeroen T. Vermeulen (jtv)]

= 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

[Henning Eggers (henninge)]

Good job, Kermit! ;-)

review: Approve (code)

=== 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 @@
     True
     >>> subject
     u'Import problem - Serbian (sr) - ...'
+
+
+No Contact Address
+------------------
+
+Not every user has a valid email address.  For instance, Kermit the
+Hermit has none at the moment.
+
+    >>> from canonical.launchpad.interfaces.emailaddress import (
+    ...     EmailAddressStatus)
+    >>> from canonical.launchpad.helpers import get_contact_email_addresses
+    >>> hermit = factory.makePerson(
+    ...     name='hermit', email_address_status=EmailAddressStatus.OLD)
+
+    >>> len(get_contact_email_addresses(hermit))
+    0
+
+Kermit uploads a translation, which gets approved.
+
+    >>> pofile = factory.makePOFile('lo')
+
+    >>> entry = translation_import_queue.addOrUpdateEntry(
+    ...     'lo.po', 'Invalid content', True, hermit,
+    ...     pofile=pofile, potemplate=pofile.potemplate,
+    ...     productseries=pofile.potemplate.productseries)
+    >>> entry.setStatus(RosettaImportStatus.APPROVED)
+    >>> transaction.commit()
+
+The import fails.  The importer would like to send Kermit an email about
+this, but is unable to.  This is unfortunate, but does not faze the
+importer.  It completes normally.
+
+    >>> process = ImportProcess(transaction, FakeLogger())
+    >>> process.run()
+    INFO Importing: Lao ...
+    INFO Import requests completed.
+
+    >>> print entry.status.name
+    FAILED

=== 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 @@
                         else:
                             to_email = helpers.get_contact_email_addresses(
                                 entry_to_import.importer)
-                        text = MailWrapper().format(mail_body)
-
-                        # XXX: JeroenVermeulen 2007-11-29 bug=29744: email
-                        # isn't transactional in zopeless mode.  That
-                        # means that our current transaction can fail
-                        # after we already sent out a success notification.
-                        # To prevent that, we commit the import (attempt)
-                        # before sending out the email.  That way, the worst
-                        # that can happen is that an email goes missing.
-                        # Once bug 29744 is fixed, this commit must die so
-                        # the email and the import will be in a single
-                        # atomic operation.
-                        self.ztm.commit()
-                        self.ztm.begin()
-
-                        simple_sendmail(
-                            from_email, to_email, mail_subject, text)
+
+                        if to_email:
+                            text = MailWrapper().format(mail_body)
+
+                            # XXX: JeroenVermeulen 2007-11-29 bug=29744: email
+                            # isn't transactional in zopeless mode.  That
+                            # means that our current transaction can fail
+                            # after we already sent out a success
+                            # notification.  To prevent that, we commit the
+                            # import (attempt) before sending out the email.
+                            # That way the worst that can happen is that an
+                            # email goes missing.
+                            # Once bug 29744 is fixed, this commit must die so
+                            # the email and the import will be in a single
+                            # atomic operation.
+                            self.ztm.commit()
+                            self.ztm.begin()
+
+                            simple_sendmail(
+                                from_email, to_email, mail_subject, text)
 
                 except KeyboardInterrupt:
                     self.ztm.abort()