Colin Watson has proposed merging
lp:~cjwatson/launchpad/avoid-copy-archive-spam into lp:launchpad.
Commit message:
Don't mail the maintainer and changer for uploads to copy archives.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1103491 in Launchpad itself: "Natively-synced packages in copy archives
spam Debian developers with disallowed-component warnings"
https://bugs.launchpad.net/launchpad/+bug/1103491
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/avoid-copy-archive-spam/+merge/144611
== Summary ==
This is the first part of a fix for bug 1103491, which is currently spamming at
least one Debian developer for no good reason. It doesn't fix the reason the
uploads in question are being rejected, but that will take a little longer to
disentangle; for now, I'd at least like to squash the e-mails.
== Proposed fix ==
Don't send mail to maintainer/changed-by for copy archive uploads.
== Implementation details ==
The change itself is trivial. I noticed that utilities/update-copyright wasn't
correctly handling lib/lp/soyuz/adapters/tests/test_notification.py and fixed
that along the way, and I did some refactoring to bring this branch to
LoC-neutrality.
== Tests ==
bin/test -vvct lp.soyuz.adapters.tests.test_notification
== Demo and Q/A ==
Not sure if there's a lightweight way; we'll probably have to create a small
copy archive on dogfood with just lha in it, attempt a build upload, and make
sure its failure doesn't send mail. If we still have no dogfood builders, then
we could inject the upload that Tom attached to the bug.
== Lint ==
Pre-existing lint relating to an indented e-mail message, so I think not worth
fixing:
./lib/lp/soyuz/adapters/tests/test_notification.py
218: E501 line too long (85 characters)
219: E501 line too long (82 characters)
223: E501 line too long (82 characters)
--
https://code.launchpad.net/~cjwatson/launchpad/avoid-copy-archive-spam/+merge/144611
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/avoid-copy-archive-spam into lp:launchpad.
=== modified file 'lib/lp/soyuz/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py 2012-02-10 09:31:39 +0000
+++ lib/lp/soyuz/adapters/notification.py 2013-01-23 23:59:22 +0000
@@ -1,4 +1,4 @@
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Notification for uploads and copies."""
@@ -471,17 +471,14 @@
bprs=None):
"""Return a list of recipients for notification emails."""
debug(logger, "Building recipients list.")
- candidate_recipients = [
- blamer,
- ]
+ candidate_recipients = [blamer]
info = fetch_information(spr, bprs, changes)
changer = email_to_person(info['changedby'])
maintainer = email_to_person(info['maintainer'])
if blamer is None:
- debug(
- logger, "Changes file is unsigned; adding changer as recipient.")
+ debug(logger, "Changes file is unsigned; adding changer as recipient.")
candidate_recipients.append(changer)
if archive.is_ppa:
@@ -491,6 +488,10 @@
candidate_recipients.extend([
permission.person
for permission in archive.getUploadersForComponent()])
+ elif archive.is_copy:
+ # For copy archives, notifying anyone else will probably only
+ # confuse them.
+ pass
else:
# If this is not a PPA, we also consider maintainer and changed-by.
if blamer is not None:
@@ -548,8 +549,7 @@
if customfiles:
files.extend(
- [(file.libraryfilealias.filename, '', '')
- for file in customfiles])
+ [(file.libraryfilealias.filename, '', '') for file in customfiles])
return files
@@ -581,8 +581,7 @@
try:
# The 2nd arg to s_f_m() doesn't matter as it won't fail since every-
# thing will have already parsed at this point.
- (rfc822, rfc2047, name, email) = safe_fix_maintainer(
- fullemail, "email")
+ rfc822, rfc2047, name, email = safe_fix_maintainer(fullemail, "email")
return getUtility(IPersonSet).getByEmail(email)
except ParseMaintError:
return None
=== modified file 'lib/lp/soyuz/adapters/tests/test_notification.py'
--- lib/lp/soyuz/adapters/tests/test_notification.py 2012-02-10 09:31:39 +0000
+++ lib/lp/soyuz/adapters/tests/test_notification.py 2013-01-23 23:59:22 +0000
@@ -2,7 +2,7 @@
# NOTE: The first line above must stay first; do not move the copyright
# notice to the top. See http://www.python.org/dev/peps/pep-0263/.
#
-# Copyright 2011 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from email.utils import formataddr
@@ -127,8 +127,7 @@
# to the announce list, which is the one that gets the overridden
# From:
self.assertEqual(
- "Lemmy Kilmister <[email protected]>",
- notifications[1]["From"])
+ "Lemmy Kilmister <[email protected]>", notifications[1]["From"])
def test_notify_from_person_override_with_unicode_names(self):
# notify() takes an optional from_person to override the calculated
@@ -238,8 +237,7 @@
'Maintainer': 'Foo Bar <[email protected]>',
'Changes': ' * Foo!',
}
- info = fetch_information(
- None, None, changes)
+ info = fetch_information(None, None, changes)
self.assertEqual('2001-01-01', info['date'])
self.assertEqual(' * Foo!', info['changelog'])
fields = [
@@ -353,12 +351,20 @@
notifications = pop_notifications()
self.assertEqual(0, len(notifications))
- def _run_recipients_test(self, changes, blamer, maintainer, changer):
+ def _setup_recipients(self):
+ blamer = self.factory.makePerson()
+ maintainer = self.factory.makePerson(
+ '[email protected]', displayname='Maintainer')
+ changer = self.factory.makePerson(
+ '[email protected]', displayname='Changer')
+ return blamer, maintainer, changer
+
+ def assertRecipientsEqual(self, expected, changes, blamer, maintainer,
+ changer, purpose=ArchivePurpose.PRIMARY):
distribution = self.factory.makeDistribution()
archive = self.factory.makeArchive(
- distribution=distribution, purpose=ArchivePurpose.PRIMARY)
- distroseries = self.factory.makeDistroSeries(
- distribution=distribution)
+ distribution=distribution, purpose=purpose)
+ distroseries = self.factory.makeDistroSeries(distribution=distribution)
# Now set the uploaders.
component = getUtility(IComponentSet).ensure('main')
if component not in distroseries.components:
@@ -366,69 +372,64 @@
store.add(
ComponentSelection(
distroseries=distroseries, component=component))
- archive.newComponentUploader(maintainer, component)
- archive.newComponentUploader(changer, component)
- return get_upload_notification_recipients(
+ distribution.main_archive.newComponentUploader(maintainer, component)
+ distribution.main_archive.newComponentUploader(changer, component)
+ observed = get_upload_notification_recipients(
blamer, archive, distroseries, logger=None, changes=changes)
+ self.assertContentEqual(
+ [format_address_for_person(person) for person in expected],
+ observed)
def test_get_upload_notification_recipients_good_emails(self):
# Test get_upload_notification_recipients with good email addresses..
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- '[email protected]', displayname='Maintainer')
- changer = self.factory.makePerson(
- '[email protected]', displayname='Changer')
+ blamer, maintainer, changer = self._setup_recipients()
changes = {
'Date': '2001-01-01',
'Changed-By': 'Changer <[email protected]>',
'Maintainer': 'Maintainer <[email protected]>',
'Changes': ' * Foo!',
}
- recipients = self._run_recipients_test(
+ self.assertRecipientsEqual(
+ [blamer, maintainer, changer],
changes, blamer, maintainer, changer)
- expected = [
- format_address_for_person(person)
- for person in (blamer, maintainer, changer)]
- self.assertContentEqual(expected, recipients)
def test_get_upload_notification_recipients_bad_maintainer_email(self):
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- '[email protected]', displayname='Maintainer')
- changer = self.factory.makePerson(
- '[email protected]', displayname='Changer')
+ blamer, maintainer, changer = self._setup_recipients()
changes = {
'Date': '2001-01-01',
'Changed-By': 'Changer <[email protected]>',
'Maintainer': 'Maintainer <maintainer at example.com>',
'Changes': ' * Foo!',
}
- recipients = self._run_recipients_test(
- changes, blamer, maintainer, changer)
- expected = [
- format_address_for_person(person) for person in (blamer, changer)]
- self.assertContentEqual(expected, recipients)
+ self.assertRecipientsEqual(
+ [blamer, changer], changes, blamer, maintainer, changer)
def test_get_upload_notification_recipients_bad_changedby_email(self):
# Test get_upload_notification_recipients with invalid changedby
# email address.
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- '[email protected]', displayname='Maintainer')
- changer = self.factory.makePerson(
- '[email protected]', displayname='Changer')
+ blamer, maintainer, changer = self._setup_recipients()
changes = {
'Date': '2001-01-01',
'Changed-By': 'Changer <changer at example.com>',
'Maintainer': 'Maintainer <[email protected]>',
'Changes': ' * Foo!',
}
- recipients = self._run_recipients_test(
- changes, blamer, maintainer, changer)
- expected = [
- format_address_for_person(person)
- for person in (blamer, maintainer)]
- self.assertContentEqual(expected, recipients)
+ self.assertRecipientsEqual(
+ [blamer, maintainer], changes, blamer, maintainer, changer)
+
+ def test_get_upload_notification_recipients_copy_archive(self):
+ # Notifications for uploads to copy archives only go to the archive
+ # owner.
+ blamer, maintainer, changer = self._setup_recipients()
+ changes = {
+ 'Date': '2001-01-01',
+ 'Changed-By': 'Changer <[email protected]>',
+ 'Maintainer': 'Maintainer <[email protected]>',
+ 'Changes': ' * Foo!',
+ }
+ self.assertRecipientsEqual(
+ [blamer], changes, blamer, maintainer, changer,
+ purpose=ArchivePurpose.COPY)
def test_assemble_body_handles_no_preferred_email_for_changer(self):
# If changer has no preferred email address,
=== modified file 'utilities/update-copyright'
--- utilities/update-copyright 2012-01-01 03:10:25 +0000
+++ utilities/update-copyright 2013-01-23 23:59:22 +0000
@@ -1,6 +1,6 @@
#!/usr/bin/python
#
-# Copyright 2010 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2013 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Update the year in copyright notices.
@@ -33,7 +33,7 @@
def update_copyright(lines):
"""Update the copyright notice in the given file lines."""
- for line in range(min(len(lines), 3)):
+ for line in range(min(len(lines), 5)):
match = copyright_pattern.search(lines[line])
if match is not None:
old_years = match.group('years')
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
