launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05329
[Merge] lp:~jtv/launchpad/pre-876594 into lp:launchpad
Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/pre-876594 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jtv/launchpad/pre-876594/+merge/80537
= Summary =
We need to make some changes to Soyuz upload notification. Actually this also covers copying, building, and syncing of packages.
Unfortunately the code has become a bit of a mess and some of the tests are painfully brittle.
== Proposed fix ==
Before fixing the bug itself, clean up the code. Make tests independent of arbitrary ordering of email addressee lists; eliminate needless repetition of code; make the name of the function we'll be changing a bit more specific; make complicated code a bit more concise so we can reason about it more effectively.
== Pre-implementation notes ==
Discussed the bigger picture with Julian. We're going to exempt package maintainers and authors from notification for packages that are being accepted into different archives than their releases are in. We'll also want to see if we can remove complexity from notify() that reflects knowledge inherent in the callsite.
== Implementation details ==
The recipients-gathering code no longer checks for various recipients being the same. Instead it just lumps all recipients into a set. Duplicates disappear as a matter of course.
This will affect the logging a bit: the code will now happily say it's adding both the maintainer and the last author to the set of recipients, even if they're the same person. It may actually be clearer; certainly the code becomes easier to manage.
== Tests ==
{{{
./bin/test -vvc lp.archiveuploader
./bin/test -vvc lp.soyuz
}}}
== Demo and Q/A ==
There are no functional changes, so as long as nothing oopses, it should be fine.
= Launchpad lint =
I'll address the lint in distroseriesqueue-notify.txt in a separate lint branch. The other stuff is a bit harder to clean up because it involves whitespace-sensitive tests.
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/soyuz/scripts/packagecopier.py
lib/lp/soyuz/model/queue.py
lib/lp/archiveuploader/tests/nascentupload-announcements.txt
lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt
lib/lp/soyuz/adapters/notification.py
lib/lp/soyuz/doc/distroseriesqueue-notify.txt
lib/lp/soyuz/adapters/tests/test_notification.py
./lib/lp/archiveuploader/tests/nascentupload-announcements.txt
186: want exceeds 78 characters.
187: want exceeds 78 characters.
659: want exceeds 78 characters.
726: want exceeds 78 characters.
728: want exceeds 78 characters.
774: want exceeds 78 characters.
776: want exceeds 78 characters.
./lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt
145: want exceeds 78 characters.
148: want exceeds 78 characters.
./lib/lp/soyuz/doc/distroseriesqueue-notify.txt
1: narrative uses a moin header.
6: source has bad indentation.
9: narrative has trailing whitespace.
10: narrative has trailing whitespace.
13: source has bad indentation.
21: source exceeds 78 characters.
21: source has bad indentation.
41: source has bad indentation.
60: source has bad indentation.
67: source has bad indentation.
74: source has bad indentation.
79: source has bad indentation.
84: source has bad indentation.
102: narrative has trailing whitespace.
103: narrative has trailing whitespace.
106: source has bad indentation.
113: source has bad indentation.
133: source has bad indentation.
140: source has bad indentation.
144: source has bad indentation.
150: source has bad indentation.
155: source has bad indentation.
161: source has bad indentation.
168: source has bad indentation.
173: source has bad indentation.
178: source has bad indentation.
200: source has bad indentation.
218: source has bad indentation.
222: source has bad indentation.
229: source has bad indentation.
234: source has bad indentation.
256: source has bad indentation.
277: source has bad indentation.
283: source has bad indentation.
./lib/lp/soyuz/adapters/tests/test_notification.py
217: Line exceeds 78 characters.
218: Line exceeds 78 characters.
222: Line exceeds 78 characters.
217: E501 line too long (85 characters)
218: E501 line too long (82 characters)
222: E501 line too long (82 characters)
--
https://code.launchpad.net/~jtv/launchpad/pre-876594/+merge/80537
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/pre-876594 into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/tests/nascentupload-announcements.txt'
--- lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2011-08-22 04:38:02 +0000
+++ lib/lp/archiveuploader/tests/nascentupload-announcements.txt 2011-10-27 07:07:29 +0000
@@ -46,8 +46,8 @@
... return cmp(a[1], b[1])
>>> def print_addrlist(field):
- ... for entry in field.split(','):
- ... print entry.strip()
+ ... for entry in sorted([addr.strip() for addr in field.split(',')]):
+ ... print entry
Import the test keys to use 'insecure' policy.
@@ -443,8 +443,8 @@
'Launchpad actually'
>>> print_addrlist(notification['To'])
+ Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
Foo Bar <foo.bar@xxxxxxxxxxxxx>
- Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
>>> notification['Subject']
'[ubuntu/hoary-updates] bar 1.0-2 (Waiting for approval)'
@@ -479,8 +479,8 @@
'Launchpad actually'
>>> print_addrlist(notification['To'])
+ Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
Foo Bar <foo.bar@xxxxxxxxxxxxx>
- Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
>>> notification['Subject']
'[ubuntu/hoary-backports] bar 1.0-3 (Waiting for approval)'
=== modified file 'lib/lp/soyuz/adapters/notification.py'
--- lib/lp/soyuz/adapters/notification.py 2011-08-31 05:06:54 +0000
+++ lib/lp/soyuz/adapters/notification.py 2011-10-27 07:07:29 +0000
@@ -6,7 +6,6 @@
__metaclass__ = type
__all__ = [
- 'get_recipients', # Available for testing only.
'notify',
]
@@ -68,7 +67,7 @@
}
template = get_template(archive, 'rejected')
body = template % information
- to_addrs = get_recipients(
+ to_addrs = get_upload_notification_recipients(
blamer, archive, distroseries, logger, changes=changes)
debug(logger, "Sending rejection email.")
if not to_addrs:
@@ -128,7 +127,7 @@
summary_text=None, changes=None, changesfile_content=None,
changesfile_object=None, action=None, dry_run=False,
logger=None, announce_from_person=None, previous_version=None):
- """Notify about
+ """Notify about an upload or package copy.
:param blamer: The `IPerson` who is to blame for this notification.
:param spr: The `ISourcePackageRelease` that was created.
@@ -187,7 +186,7 @@
if not files and action != 'rejected':
return
- recipients = get_recipients(
+ recipients = get_upload_notification_recipients(
blamer, archive, distroseries, logger, changes=changes, spr=spr,
bprs=bprs)
@@ -304,13 +303,12 @@
if distroseries.changeslist:
information['ANNOUNCE'] = "Announcing to %s" % (
distroseries.changeslist)
- try:
- changedby_person = email_to_person(info['changedby'])
- except ParseMaintError:
- # Some syncs (e.g. from Debian) will involve packages whose
- # changed-by person was auto-created in LP and hence does not
- # have a preferred email address set.
- changedby_person = None
+
+ # Some syncs (e.g. from Debian) will involve packages whose
+ # changed-by person was auto-created in LP and hence does not have a
+ # preferred email address set.
+ changedby_person = email_to_person(info['changedby'])
+
if blamer is not None and blamer != changedby_person:
signer_signature = person_to_email(blamer)
if signer_signature != info['changedby']:
@@ -448,73 +446,68 @@
return guess_encoding(s)
-def debug(logger, msg):
+def debug(logger, msg, *args, **kwargs):
"""Shorthand debug notation for publish() methods."""
if logger is not None:
- logger.debug(msg)
-
-
-def get_recipients(blamer, archive, distroseries, logger, changes=None,
- spr=None, bprs=None):
+ logger.debug(msg, *args, **kwargs)
+
+
+def is_valid_uploader(person, distribution):
+ """Is `person` an uploader for `distribution`?
+
+ A `None` person is not an uploader.
+ """
+ if person is None:
+ return None
+ else:
+ return person.isUploader(distribution)
+
+
+def get_upload_notification_recipients(blamer, archive, distroseries,
+ logger=None, changes=None, spr=None,
+ bprs=None):
"""Return a list of recipients for notification emails."""
- candidate_recipients = []
debug(logger, "Building recipients list.")
+ candidate_recipients = [
+ blamer,
+ ]
info = fetch_information(spr, bprs, changes)
- if info['changedby']:
- try:
- changer = email_to_person(info['changedby'])
- except ParseMaintError:
- changer = None
- else:
- changer = None
-
- if info['maintainer']:
- try:
- maintainer = email_to_person(info['maintainer'])
- except ParseMaintError:
- maintainer = None
- else:
- maintainer = None
-
- if blamer:
- # This is a signed upload.
- candidate_recipients.append(blamer)
- else:
- debug(logger,
- "Changes file is unsigned, adding changer as recipient")
+ 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.")
candidate_recipients.append(changer)
if archive.is_ppa:
# For PPAs, any person or team mentioned explicitly in the
# ArchivePermissions as uploaders for the archive will also
# get emailed.
- uploaders = [
- permission.person for permission in
- archive.getUploadersForComponent()]
- candidate_recipients.extend(uploaders)
-
- # If this is not a PPA, we also consider maintainer and changed-by.
- elif blamer is not None:
- if (maintainer and maintainer != blamer and
- maintainer.isUploader(distroseries.distribution)):
- debug(logger, "Adding maintainer to recipients")
- candidate_recipients.append(maintainer)
-
- if (changer and changer != blamer and
- changer.isUploader(distroseries.distribution)):
- debug(logger, "Adding changed-by to recipients")
- candidate_recipients.append(changer)
+ candidate_recipients.extend([
+ permission.person
+ for permission in archive.getUploadersForComponent()])
+ else:
+ # If this is not a PPA, we also consider maintainer and changed-by.
+ if blamer is not None:
+ if is_valid_uploader(maintainer, distroseries.distribution):
+ debug(logger, "Adding maintainer to recipients")
+ candidate_recipients.append(maintainer)
+
+ if is_valid_uploader(changer, distroseries.distribution):
+ debug(logger, "Adding changed-by to recipients")
+ candidate_recipients.append(changer)
# Now filter list of recipients for persons only registered in
# Launchpad to avoid spamming the innocent.
- recipients = []
- for person in candidate_recipients:
- if person is None or person.preferredemail is None:
- continue
- recipient = format_address_for_person(person)
- debug(logger, "Adding recipient: '%s'" % recipient)
- recipients.append(recipient)
+ recipients = [
+ format_address_for_person(person)
+ for person in filter(None, set(candidate_recipients))
+ if person.preferredemail is not None]
+
+ for recipient in recipients:
+ debug(logger, "Adding recipient: '%s'", recipient)
return recipients
@@ -572,12 +565,23 @@
def email_to_person(fullemail):
- """Return an IPerson given an RFC2047 email address."""
- # 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")
- return getUtility(IPersonSet).getByEmail(email)
+ """Return an `IPerson` given an RFC2047 email address.
+
+ :param fullemail: Potential email address.
+ :return: `IPerson` with the given email address. None if there
+ isn't one, or if `fullemail` isn't a proper email address.
+ """
+ if fullemail is None or fullemail == '':
+ return None
+
+ 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")
+ return getUtility(IPersonSet).getByEmail(email)
+ except ParseMaintError:
+ return None
def person_to_email(person):
@@ -595,12 +599,11 @@
user (archive@xxxxxxxxxx).
"""
katie = getUtility(ILaunchpadCelebrities).katie
- try:
- changed_by = email_to_person(changed_by_email)
- except ParseMaintError:
- return False
+ changed_by = email_to_person(changed_by_email)
return (
- spr and not bprs and changed_by == katie and
+ spr and
+ not bprs and
+ changed_by == katie and
pocket != PackagePublishingPocket.SECURITY)
=== modified file 'lib/lp/soyuz/adapters/tests/test_notification.py'
--- lib/lp/soyuz/adapters/tests/test_notification.py 2011-08-31 05:06:54 +0000
+++ lib/lp/soyuz/adapters/tests/test_notification.py 2011-10-27 07:07:29 +0000
@@ -6,8 +6,9 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
from email.utils import formataddr
+from textwrap import dedent
+
from storm.store import Store
-from textwrap import dedent
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -18,24 +19,24 @@
)
from lp.archivepublisher.utils import get_ppa_reference
from lp.registry.interfaces.pocket import PackagePublishingPocket
+from lp.services.log.logger import BufferLogger
from lp.services.mail.sendmail import format_address_for_person
-from lp.services.log.logger import BufferLogger
from lp.soyuz.adapters.notification import (
assemble_body,
calculate_subject,
- get_recipients,
fetch_information,
+ get_upload_notification_recipients,
is_auto_sync_upload,
+ notify,
+ person_to_email,
reject_changes_file,
- person_to_email,
- notify,
)
-from lp.soyuz.interfaces.component import IComponentSet
-from lp.soyuz.model.component import ComponentSelection
from lp.soyuz.enums import (
ArchivePurpose,
PackageUploadCustomFormat,
)
+from lp.soyuz.interfaces.component import IComponentSet
+from lp.soyuz.model.component import ComponentSelection
from lp.soyuz.model.distroseriessourcepackagerelease import (
DistroSeriesSourcePackageRelease,
)
@@ -231,8 +232,8 @@
def test_fetch_information_changes(self):
changes = {
'Date': '2001-01-01',
- 'Changed-By': 'Foo Bar <foo.bar@xxxxxxxxxxxxx>',
- 'Maintainer': 'Foo Bar <foo.bar@xxxxxxxxxxxxx>',
+ 'Changed-By': 'Foo Bar <foo.bar@xxxxxxxxxxx>',
+ 'Maintainer': 'Foo Bar <foo.bar@xxxxxxxxxxx>',
'Changes': ' * Foo!',
}
info = fetch_information(
@@ -246,7 +247,7 @@
info['maintainer_displayname'],
]
for field in fields:
- self.assertEqual('Foo Bar <foo.bar@xxxxxxxxxxxxx>', field)
+ self.assertEqual('Foo Bar <foo.bar@xxxxxxxxxxx>', field)
def test_fetch_information_spr(self):
creator = self.factory.makePerson(displayname=u"foø")
@@ -365,64 +366,67 @@
distroseries=distroseries, component=component))
archive.newComponentUploader(maintainer, component)
archive.newComponentUploader(changer, component)
- return get_recipients(
+ return get_upload_notification_recipients(
blamer, archive, distroseries, logger=None, changes=changes)
- def test_get_recipients_good_emails(self):
- # Test get_recipients with good email addresses..
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- 'maintainer@xxxxxxxxxxxxx', displayname='Maintainer')
- changer = self.factory.makePerson(
- 'changer@xxxxxxxxxxxxx', displayname='Changer')
- changes = {
- 'Date': '2001-01-01',
- 'Changed-By': 'Changer <changer@xxxxxxxxxxxxx>',
- 'Maintainer': 'Maintainer <maintainer@xxxxxxxxxxxxx>',
- 'Changes': ' * Foo!',
- }
- recipients = self._run_recipients_test(
- changes, blamer, maintainer, changer)
- expected = [format_address_for_person(p)
- for p in (blamer, maintainer, changer)]
- self.assertEqual(expected, recipients)
-
- def test_get_recipients_bad_maintainer_email(self):
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- 'maintainer@xxxxxxxxxxxxx', displayname='Maintainer')
- changer = self.factory.makePerson(
- 'changer@xxxxxxxxxxxxx', displayname='Changer')
- changes = {
- 'Date': '2001-01-01',
- 'Changed-By': 'Changer <changer@xxxxxxxxxxxxx>',
- 'Maintainer': 'Maintainer <maintainer at canonical.com>',
- 'Changes': ' * Foo!',
- }
- recipients = self._run_recipients_test(
- changes, blamer, maintainer, changer)
- expected = [format_address_for_person(p)
- for p in (blamer, changer)]
- self.assertEqual(expected, recipients)
-
- def test_get_recipients_bad_changedby_email(self):
- # Test get_recipients with invalid changedby email address.
- blamer = self.factory.makePerson()
- maintainer = self.factory.makePerson(
- 'maintainer@xxxxxxxxxxxxx', displayname='Maintainer')
- changer = self.factory.makePerson(
- 'changer@xxxxxxxxxxxxx', displayname='Changer')
- changes = {
- 'Date': '2001-01-01',
- 'Changed-By': 'Changer <changer at canonical.com>',
- 'Maintainer': 'Maintainer <maintainer@xxxxxxxxxxxxx>',
- 'Changes': ' * Foo!',
- }
- recipients = self._run_recipients_test(
- changes, blamer, maintainer, changer)
- expected = [format_address_for_person(p)
- for p in (blamer, maintainer)]
- self.assertEqual(expected, recipients)
+ 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(
+ 'maintainer@xxxxxxxxxxx', displayname='Maintainer')
+ changer = self.factory.makePerson(
+ 'changer@xxxxxxxxxxx', displayname='Changer')
+ changes = {
+ 'Date': '2001-01-01',
+ 'Changed-By': 'Changer <changer@xxxxxxxxxxx>',
+ 'Maintainer': 'Maintainer <maintainer@xxxxxxxxxxx>',
+ 'Changes': ' * Foo!',
+ }
+ recipients = self._run_recipients_test(
+ 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(
+ 'maintainer@xxxxxxxxxxx', displayname='Maintainer')
+ changer = self.factory.makePerson(
+ 'changer@xxxxxxxxxxx', displayname='Changer')
+ changes = {
+ 'Date': '2001-01-01',
+ 'Changed-By': 'Changer <changer@xxxxxxxxxxx>',
+ '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)
+
+ 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(
+ 'maintainer@xxxxxxxxxxx', displayname='Maintainer')
+ changer = self.factory.makePerson(
+ 'changer@xxxxxxxxxxx', displayname='Changer')
+ changes = {
+ 'Date': '2001-01-01',
+ 'Changed-By': 'Changer <changer at example.com>',
+ 'Maintainer': 'Maintainer <maintainer@xxxxxxxxxxx>',
+ '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)
def test_assemble_body_handles_no_preferred_email_for_changer(self):
# If changer has no preferred email address,
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2011-07-21 23:10:26 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-dist-upgrader.txt 2011-10-27 07:07:29 +0000
@@ -60,6 +60,7 @@
DEBUG Creating queue entry
DEBUG Setting it to ACCEPTED
DEBUG Building recipients list.
+ ...
DEBUG Adding recipient: 'Foo Bar <foo.bar@xxxxxxxxxxxxx>'
DEBUG Sent a mail:
...
@@ -235,6 +236,7 @@
DEBUG Creating queue entry
DEBUG Setting it to ACCEPTED
DEBUG Building recipients list.
+ ...
DEBUG Adding recipient: 'Foo Bar <foo.bar@xxxxxxxxxxxxx>'
DEBUG Sent a mail:
...
=== modified file 'lib/lp/soyuz/doc/distroseriesqueue-notify.txt'
--- lib/lp/soyuz/doc/distroseriesqueue-notify.txt 2011-06-01 04:57:27 +0000
+++ lib/lp/soyuz/doc/distroseriesqueue-notify.txt 2011-10-27 07:07:29 +0000
@@ -45,11 +45,11 @@
>>> netapplet_upload.notify(
... changes_file_object=changes_file, logger=FakeLogger())
DEBUG Building recipients list.
- DEBUG Changes file is unsigned, adding changer as recipient
+ DEBUG Changes file is unsigned; adding changer as recipient.
...
DEBUG Sent a mail:
...
- DEBUG Recipients: Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ DEBUG Recipients: ... Silverstone ...
...
DEBUG above if files already exist in other distroseries.
...
@@ -120,7 +120,7 @@
...
DEBUG Sent a mail:
...
- DEBUG Recipients: Foo Bar <foo.bar@xxxxxxxxxxxxx>, Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ DEBUG Recipients: ... Bar ...
...
DEBUG Announcing to autotest_changes@xxxxxxxxxx
...
@@ -137,22 +137,41 @@
The mail 'To:' addresses contain the signer and the changer's email. The
announcement email contains the serieses changeslist.
- >>> [msg['To'] for msg in msgs]
- ['Foo Bar <foo.bar@xxxxxxxxxxxxx>,\n\tDaniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>', 'autotest_changes@xxxxxxxxxx']
+ >>> def to_lower(address):
+ ... """Return lower-case version of email address."""
+ ... return address.lower()
+
+ >>> def extract_addresses(header_field):
+ ... """Extract and sort addresses from an email header field."""
+ ... return sorted(
+ ... [addr.strip() for addr in header_field.split(',')],
+ ... key=to_lower)
+
+ >>> for addr in extract_addresses(msgs[0]['To']):
+ ... print addr
+ Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ Foo Bar <foo.bar@xxxxxxxxxxxxx>
+
+ >>> print msgs[1]['To']
+ autotest_changes@xxxxxxxxxx
The mail 'Bcc:' address is the uploader. The announcement has the uploader
and the Debian derivatives address for the package uploaded.
- >>> [msg['Bcc'] for msg in msgs]
- ['Root <root@localhost>', 'Root <root@localhost>, netapplet_derivatives@xxxxxxxxxxxxxxxxxxxxxx']
+ >>> for msg in msgs:
+ ... print extract_addresses(msg['Bcc'])
+ ['Root <root@localhost>']
+ ['netapplet_derivatives@xxxxxxxxxxxxxxxxxxxxxx', 'Root <root@localhost>']
The mail 'From:' addresses are the uploader and the changer.
- >>> [msg['From'] for msg in msgs]
- ['Root <root@localhost>', 'Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>']
+ >>> for msg in msgs:
+ ... print msg['From']
+ Root <root@localhost>
+ Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
- >>> notification['Subject']
- '[ubuntu/breezy-autotest] netapplet 0.99.6-1 (New)'
+ >>> print notification['Subject']
+ [ubuntu/breezy-autotest] netapplet 0.99.6-1 (New)
The mail body contains the same list of files again:
@@ -188,7 +207,7 @@
...
DEBUG Sent a mail:
...
- DEBUG Recipients: Foo Bar <foo.bar@xxxxxxxxxxxxx>, Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ DEBUG Recipients: ... Silverstone ...
...
DEBUG above if files already exist in other distroseries.
...
@@ -200,13 +219,15 @@
The mail headers are the same as before:
- >>> notification['To']
- 'Foo Bar <foo.bar@xxxxxxxxxxxxx>,\n\tDaniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>'
- >>> notification['Bcc']
- 'Root <root@localhost>'
+ >>> for addr in extract_addresses(notification['To']):
+ ... print addr
+ Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ Foo Bar <foo.bar@xxxxxxxxxxxxx>
+ >>> print notification['Bcc']
+ Root <root@localhost>
- >>> notification['Subject']
- '[ubuntu/breezy-autotest] netapplet 0.99.6-1 (New)'
+ >>> print notification['Subject']
+ [ubuntu/breezy-autotest] netapplet 0.99.6-1 (New)
The mail body contains the same list of files again:
@@ -241,7 +262,7 @@
...
DEBUG Subject: [ubuntu/breezy-autotest] netapplet 0.99.6-1 (Rejected)
DEBUG Sender: Root <root@localhost>
- DEBUG Recipients: Foo Bar <foo.bar@xxxxxxxxxxxxx>, Daniel Silverstone <daniel.silverstone@xxxxxxxxxxxxx>
+ DEBUG Recipients: ... Foo Bar ...
DEBUG Bcc: Root <root@localhost>
DEBUG Body:
DEBUG Rejected:
=== modified file 'lib/lp/soyuz/model/queue.py'
--- lib/lp/soyuz/model/queue.py 2011-08-25 08:29:37 +0000
+++ lib/lp/soyuz/model/queue.py 2011-10-27 07:07:29 +0000
@@ -868,7 +868,7 @@
signer, self.sourcepackagerelease, self.builds, self.customfiles,
self.archive, self.distroseries, self.pocket, summary_text,
changes, changesfile_content, changes_file_object,
- status_action[self.status], dry_run, logger)
+ status_action[self.status], dry_run=dry_run, logger=logger)
def _isPersonUploader(self, person):
"""Return True if person is an uploader to the package's distro."""
=== modified file 'lib/lp/soyuz/scripts/packagecopier.py'
--- lib/lp/soyuz/scripts/packagecopier.py 2011-09-26 06:30:07 +0000
+++ lib/lp/soyuz/scripts/packagecopier.py 2011-10-27 07:07:29 +0000
@@ -533,8 +533,8 @@
Verifies if each copy can be performed using `CopyChecker` and
raises `CannotCopy` if one or more copies could not be performed.
- When `CannotCopy`is raised call sites are in charge to rollback the
- transaction or performed copies will be commited.
+ When `CannotCopy` is raised, call sites are responsible for rolling
+ back the transaction. Otherwise, performed copies will be commited.
Wrapper for `do_direct_copy`.
@@ -611,8 +611,7 @@
# In zopeless mode this email will be sent immediately.
notify(
person, source.sourcepackagerelease, [], [], archive,
- series, pocket, summary_text=error_text,
- action='rejected')
+ series, pocket, summary_text=error_text, action='rejected')
raise CannotCopy(error_text)
overrides_index = 0
@@ -648,8 +647,7 @@
if send_email:
notify(
person, source.sourcepackagerelease, [], [], archive,
- destination_series, pocket, changes=None,
- action='accepted',
+ destination_series, pocket, action='accepted',
announce_from_person=announce_from_person,
previous_version=old_version)
