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

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 11568
Proposed branch: lp:~jtv/launchpad/bug-594220
Merge into: lp:launchpad
Diff against target: 143 lines (+54/-20)
2 files modified
lib/lp/translations/scripts/po_import.py (+11/-3)
lib/lp/translations/scripts/tests/test_translations_import.py (+43/-17)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-594220
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+35770@code.launchpad.net

Commit message

Stop notifying vcs-imports about import of generated translation templates.

Description of the change

= Bug 594220 =

Translations uploads can enter the queue in several ways:
 * Soyuz can upload ones generated during package build.
 * Users can upload files.
 * We can import them from bzr branches.
 * The build farm can generate templates every time a branch changes.

In that last avenue, the resulting entries on the translations import queue are owned by whoever owns the branch. But what if the branch is mirrored? In that case, there is no reliably identifiable owner in Launchpad and such branches are owned by the vcs-imports celebrity.

Each of these imports normally results in an email message (either a success confirmation or a failure notice). And thus, vcs-imports got inundated with email when we implemented the generate-templates-from-branches feature. In this branch I stop these notifications from being sent.

We discussed alternatives, including sending the notifications to some other party, but we don't want to dump excessive email messages onto unsuspecting users. If there are problems with the imports that the emails would warn about, the import-queue UI will provide the same diagnostic information.

To test:
{{{
./bin/test -vvc lp.translations.scripts.tests.test_translations_import
}}}

For Q/A, we can verify that:
 * templates generated from mirrored branches still hit the import queue,
 * notification emails are still sent out, and
 * vcs-imports stops receiving emails.

What with email filtering on staging, the surest way to verify that last point is to inquire after rollout. We should do that in addition to pre-rollout Q/A.

No lint,

Jeroen

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) :
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/scripts/po_import.py'
--- lib/lp/translations/scripts/po_import.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/scripts/po_import.py 2010-09-17 08:04:48 +0000
@@ -1,4 +1,4 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4"""Functions used with the Rosetta PO import script."""4"""Functions used with the Rosetta PO import script."""
@@ -119,13 +119,21 @@
119119
120 return True120 return True
121121
122 def _shouldNotify(self, person):
123 """Is `person` someone we should send notification emails?"""
124 # We don't notify the vcs-imports team, which owns all mirrored
125 # branches. Templates generated in the build farm based on
126 # mirrored branches are uploaded in the name of this team, but
127 # there is no point in sending out notifications to them.
128 return person != getUtility(ILaunchpadCelebrities).vcs_imports
129
122 def _importEntry(self, entry):130 def _importEntry(self, entry):
123 """Perform the import of one entry, and notify the uploader."""131 """Perform the import of one entry, and notify the uploader."""
124 self.logger.info('Importing: %s' % entry.import_into.title)
125 target = entry.import_into132 target = entry.import_into
133 self.logger.info('Importing: %s' % target.title)
126 (mail_subject, mail_body) = target.importFromQueue(entry, self.logger)134 (mail_subject, mail_body) = target.importFromQueue(entry, self.logger)
127135
128 if mail_subject is not None:136 if mail_subject is not None and self._shouldNotify(entry.importer):
129 # A `mail_subject` of None indicates that there137 # A `mail_subject` of None indicates that there
130 # is no notification worth sending out.138 # is no notification worth sending out.
131 from_email = config.rosetta.admin_email139 from_email = config.rosetta.admin_email
132140
=== modified file 'lib/lp/translations/scripts/tests/test_translations_import.py'
--- lib/lp/translations/scripts/tests/test_translations_import.py 2010-08-20 20:31:18 +0000
+++ lib/lp/translations/scripts/tests/test_translations_import.py 2010-09-17 08:04:48 +0000
@@ -1,13 +1,17 @@
1# Copyright 2009 Canonical Ltd. This software is licensed under the1# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4import logging4import logging
5import re5import re
6from unittest import TestLoader6import transaction
7from zope.component import getUtility
78
9from canonical.launchpad.interfaces.launchpad import ILaunchpadCelebrities
8from canonical.launchpad.webapp import errorlog10from canonical.launchpad.webapp import errorlog
9from canonical.testing.layers import LaunchpadScriptLayer11from canonical.testing.layers import LaunchpadScriptLayer
12from lp.services.mail import stub
10from lp.testing import TestCaseWithFactory13from lp.testing import TestCaseWithFactory
14from lp.testing.fakemethod import FakeMethod
11from lp.translations.interfaces.translationimportqueue import (15from lp.translations.interfaces.translationimportqueue import (
12 RosettaImportStatus,16 RosettaImportStatus,
13 )17 )
@@ -25,16 +29,6 @@
25 """Very serious system error."""29 """Very serious system error."""
2630
2731
28class Raiser:
29 """Raise a given exception."""
30 def __init__(self, exception):
31 self.exception = exception
32
33 def __call__(self, *args, **kwargs):
34 """Whatever the arguments, just raise self.exception."""
35 raise self.exception
36
37
38class TestTranslationsImport(TestCaseWithFactory):32class TestTranslationsImport(TestCaseWithFactory):
3933
40 layer = LaunchpadScriptLayer34 layer = LaunchpadScriptLayer
@@ -52,8 +46,26 @@
5246
53 def _makeEntry(self, path, **kwargs):47 def _makeEntry(self, path, **kwargs):
54 """Produce a queue entry."""48 """Produce a queue entry."""
49 uploader = kwargs.pop('uploader', self.owner)
55 return self.queue.addOrUpdateEntry(50 return self.queue.addOrUpdateEntry(
56 path, '# Nothing here', False, self.owner, **kwargs)51 path, '# Nothing here', False, uploader, **kwargs)
52
53 def _makeApprovedEntry(self, uploader):
54 """Produce an approved queue entry."""
55 path = '%s.pot' % self.factory.getUniqueString()
56 series = self.factory.makeProductSeries()
57 template = self.factory.makePOTemplate(series)
58 entry = self._makeEntry(
59 path, uploader=uploader, potemplate=template,
60 productseries=template.productseries)
61 entry.status = RosettaImportStatus.APPROVED
62 return entry
63
64 def _getEmailRecipients(self):
65 """List the recipients of all pending outgoing emails."""
66 return sum([
67 recipients
68 for sender, recipients, text in stub.test_emails], [])
5769
58 def test_describeEntry_without_target(self):70 def test_describeEntry_without_target(self):
59 productseries = self._makeProductSeries()71 productseries = self._makeProductSeries()
@@ -136,7 +148,8 @@
136 self.assertNotEqual(None, entry.import_into)148 self.assertNotEqual(None, entry.import_into)
137149
138 message = "The system has exploded."150 message = "The system has exploded."
139 self.script._importEntry = Raiser(OutrageousSystemError(message))151 self.script._importEntry = FakeMethod(
152 failure=OutrageousSystemError(message))
140 self.assertRaises(OutrageousSystemError, self.script.main)153 self.assertRaises(OutrageousSystemError, self.script.main)
141154
142 self.assertEqual(RosettaImportStatus.FAILED, entry.status)155 self.assertEqual(RosettaImportStatus.FAILED, entry.status)
@@ -153,7 +166,8 @@
153 self.assertNotEqual(None, entry.import_into)166 self.assertNotEqual(None, entry.import_into)
154167
155 message = "Nobody expects the Spanish Inquisition!"168 message = "Nobody expects the Spanish Inquisition!"
156 self.script._importEntry = Raiser(UnexpectedException(message))169 self.script._importEntry = FakeMethod(
170 failure=UnexpectedException(message))
157 self.script.main()171 self.script.main()
158172
159 self.assertEqual(RosettaImportStatus.FAILED, entry.status)173 self.assertEqual(RosettaImportStatus.FAILED, entry.status)
@@ -167,6 +181,18 @@
167 self.assertEqual(default_reporting.oops_prefix,181 self.assertEqual(default_reporting.oops_prefix,
168 errorlog.globalErrorUtility.oops_prefix)182 errorlog.globalErrorUtility.oops_prefix)
169183
184 def test_notifies_uploader(self):
185 entry = self._makeApprovedEntry(self.owner)
186 transaction.commit()
187 self.script._importEntry(entry)
188 transaction.commit()
189 self.assertEqual(
190 [self.owner.preferredemail.email], self._getEmailRecipients())
170191
171def test_suite():192 def test_does_not_notify_vcs_imports(self):
172 return TestLoader().loadTestsFromName(__name__)193 vcs_imports = getUtility(ILaunchpadCelebrities).vcs_imports
194 entry = self._makeApprovedEntry(vcs_imports)
195 transaction.commit()
196 self.script._importEntry(entry)
197 transaction.commit()
198 self.assertEqual([], self._getEmailRecipients())