launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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 +0000
+++ lib/lp/snappy/mail/snapbuild.py	2016-10-16 22:03:41 +0000
@@ -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 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2016-10-16 22:03:41 +0000
@@ -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 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2016-10-16 22:03:41 +0000
@@ -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(
+            "snap-build-upload-failed",
+            notification["X-Launchpad-Notification-Type"])
+        body, footer = notification.get_payload(decode=True).split("\n-- \n")
+        self.assertIn("Failed to upload", body)
+        build_url = (
+            "http://launchpad.dev/~requester/+snap/test-snap/+build/%d"; %
+            snapbuild.id)
+        self.assertIn(build_url, body)
+        self.assertEqual(
+            "%s\nYou are the requester of the build.\n" % build_url, footer)
+
     def test_run_scan_pending_retries(self):
         # A run that finds that the store has not yet finished scanning the
         # package schedules itself to be retried.