[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/snap-notify-bad-upload-response into lp:launchpad
Colin Watson has proposed merging
lp:~cjwatson/launchpad/snap-notify-bad-upload-response into lp:launchpad.
Commit message:
Send an email notification for general snap store upload failures.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1632299 in Launchpad itself: "BadUploadResponse store reply in snap
uploads should trigger email notification"
https://bugs.launchpad.net/launchpad/+bug/1632299
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-notify-bad-upload-response/+merge/308593
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/snap-notify-bad-upload-response into lp:launchpad.
=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py 2016-06-30 16:44:14 +
+++ lib/lp/snappy/mail/snapbuild.py 2016-10-16 22:03:41 +
@@ -47,6 +47,20 @@
"snap-build-upload-unauthorized", build)
@classmethod
+def forUploadFailure(cls, build):
+"""Create a mailer for notifying about store upload failures.
+
+:param build: The relevant build.
+"""
+requester = build.requester
+recipients = {requester: RecipientReason.forBuildRequester(requester)}
+return cls(
+"Store upload failed for %(snap_name)s",
+"snapbuild-uploadfailed.txt", recipients,
+config.canonical.noreply_from_address,
+"snap-build-upload-failed", build)
+
+@classmethod
def forUploadScanFailure(cls, build):
"""Create a mailer for notifying about store upload scan failures.
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2016-06-30 16:44:14 +
+++ lib/lp/snappy/model/snapbuildjob.py 2016-10-16 22:03:41 +
@@ -52,6 +52,7 @@
from lp.snappy.interfaces.snapstoreclient import (
BadReleaseResponse,
BadScanStatusResponse,
+BadUploadResponse,
ISnapStoreClient,
ReleaseFailedResponse,
ScanFailedResponse,
@@ -235,6 +236,9 @@
if isinstance(e, UnauthorizedUploadResponse):
mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
mailer.sendAll()
+elif isinstance(e, BadUploadResponse):
+mailer = SnapBuildMailer.forUploadFailure(self.snapbuild)
+mailer.sendAll()
elif isinstance(e, (BadScanStatusResponse, ScanFailedResponse)):
mailer = SnapBuildMailer.forUploadScanFailure(self.snapbuild)
mailer.sendAll()
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2016-07-15 17:11:50 +
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2016-10-16 22:03:41 +
@@ -22,6 +22,7 @@
from lp.snappy.interfaces.snapstoreclient import (
BadReleaseResponse,
BadScanStatusResponse,
+BadUploadResponse,
ISnapStoreClient,
UnauthorizedUploadResponse,
UploadNotScannedYetResponse,
@@ -171,6 +172,50 @@
"http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n;
"You are the requester of the build.\n" % snapbuild.id, footer)
+def test_run_upload_failure_notifies(self):
+# A run that gets some other upload failure from the store sends
+# mail.
+requester = self.factory.makePerson(name="requester")
+snapbuild = self.factory.makeSnapBuild(
+requester=requester, name="test-snap", owner=requester,
+builder=self.factory.makeBuilder())
+self.assertContentEqual([], snapbuild.store_upload_jobs)
+job = SnapStoreUploadJob.create(snapbuild)
+client = FakeSnapStoreClient()
+client.upload.failure = BadUploadResponse("Failed to upload")
+self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+JobRunner([job]).runAll()
+self.assertEqual([((snapbuild,), {})], client.upload.calls)
+self.assertEqual([], client.checkStatus.calls)
+self.assertEqual([], client.release.calls)
+self.assertContentEqual([job], snapbuild.store_upload_jobs)
+self.assertIsNone(job.store_url)
+self.assertEqual("Failed to upload", job.error_message)
+[notification] = pop_notifications()
+self.assertEqual(
+config.canonical.noreply_from_address, notification["From"])
+self.assertEqual(
+"Requester <%s>" % requester.preferredemail.email,
+notification["To"])
+subject = notification["Subject"].replace("\n ", " ")
+self.assertEqual("Store upload failed for test-snap", subject)
+self.assertEqual(
+"Requester", notification["X-Launchpad-Message-Rationale"])
+self.assertEqual(
+requester.name, notification["X-Launchpad-Message-For"])
+self.assertEqual(
+
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/notify-changed-by-field into lp:launchpad
Colin Watson has proposed merging
lp:~cjwatson/launchpad/notify-changed-by-field into lp:launchpad.
Commit message:
Notify the Changed-By address for PPA uploads if the .changes contains
"Launchpad-Notify-Changed-By: yes".
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1633608 in Launchpad itself: "Bileto PPA upload rejections are lost to
the ether"
https://bugs.launchpad.net/launchpad/+bug/1633608
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/notify-changed-by-field/+merge/308587
--
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~cjwatson/launchpad/notify-changed-by-field into lp:launchpad.
=== modified file 'lib/lp/soyuz/mail/packageupload.py'
--- lib/lp/soyuz/mail/packageupload.py 2015-09-02 12:05:15 +
+++ lib/lp/soyuz/mail/packageupload.py 2016-10-16 13:55:46 +
@@ -1,4 +1,4 @@
-# Copyright 2011-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -133,6 +133,7 @@
def fetch_information(spr, bprs, changes, previous_version=None):
changelog = date = changedby = maintainer = None
+notify_changed_by = False
if changes:
changelog = ChangesFile.formatChangesComment(
@@ -148,6 +149,8 @@
changes.get('Maintainer'), 'Maintainer')
except ParseMaintError:
pass
+notify_changed_by = changes.get(
+'Launchpad-Notify-Changed-By', '') == 'yes'
elif spr or bprs:
if not spr and bprs:
spr = bprs[0].build.source_package_release
@@ -166,6 +169,7 @@
'date': date,
'changedby': changedby,
'maintainer': maintainer,
+'notify_changed_by': notify_changed_by,
}
@@ -338,6 +342,17 @@
add_recipient(
recipients, permission.person,
PackageUploadRecipientReason.forPPAUploader, logger=logger)
+
+# If the "Launchpad-Notify-Changed-By: yes" field is set in the
+# .changes file, we also consider changed-by. The latter is
+# intended for use by systems that automatically sign uploads on
+# behalf of developers, in which case we want to make sure to
+# notify the developer in question.
+if info['notify_changed_by']:
+debug(logger, "Adding changed-by to recipients")
+add_recipient(
+recipients, changer,
+PackageUploadRecipientReason.forChangedBy, logger=logger)
elif archive.is_copy:
# For copy archives, notifying anyone else will probably only
# confuse them.
=== modified file 'lib/lp/soyuz/mail/tests/test_packageupload.py'
--- lib/lp/soyuz/mail/tests/test_packageupload.py 2015-09-11 12:20:23 +
+++ lib/lp/soyuz/mail/tests/test_packageupload.py 2016-10-16 13:55:46 +
@@ -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-2015 Canonical Ltd. This software is licensed under the
+# Copyright 2011-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
from textwrap import dedent
@@ -267,6 +267,26 @@
]
for field in fields:
self.assertEqual((u'Foo Bar', u'foo@example.com'), field)
+self.assertFalse(info['notify_changed_by'])
+
+def test_fetch_information_changes_notify_changed_by(self):
+changes = {
+'Date': '2001-01-01',
+'Changed-By': 'Foo Bar ',
+'Maintainer': 'Foo Bar ',
+'Changes': ' * Foo!',
+'Launchpad-Notify-Changed-By': 'yes',
+}
+info = fetch_information(None, None, changes)
+self.assertEqual('2001-01-01', info['date'])
+self.assertEqual(' * Foo!', info['changelog'])
+fields = [
+info['changedby'],
+info['maintainer'],
+]
+for field in fields:
+self.assertEqual((u'Foo Bar', u'foo@example.com'), field)
+self.assertTrue(info['notify_changed_by'])
def test_fetch_information_spr(self):
creator = self.factory.makePerson(displayname=u"foø")
@@ -280,6 +300,7 @@
(u"foø", spr.creator.preferredemail.email), info['changedby'])
self.assertEqual(
(u"bær", spr.maintainer.preferredemail.email), info['maintainer'])
+self.assertFalse(info['notify_changed_by'])
def test_fetch_information_bprs(self):
bpr = self.factory.makeBinaryPackageRelease()
@@ -293,6 +314,7 @@
self.assertEqual(
[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/remove-branch-commitsForDays into lp:launchpad
The proposal to merge lp:~cjwatson/launchpad/remove-branch-commitsForDays into lp:launchpad has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~cjwatson/launchpad/remove-branch-commitsForDays/+merge/308572 -- Your team Launchpad code reviewers is subscribed to branch lp:launchpad. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
