Merge lp:~cjwatson/launchpad/snap-channels-job into lp:launchpad

[Colin Watson]

Automatically release snap packages that have store_channels set.

Automatically release snap packages that have store_channels set.

[Thomi Richards (thomir-deactivatedaccount)]

+1

review: Approve

=== added file 'lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/emailtemplates/snapbuild-manualreview.txt	2016-06-30 17:00:36 +0000
@@ -0,0 +1,5 @@
+This snap package could not be released automatically because it was held
+for manual review.  Once it has been approved, you will need to release it
+manually from here:
+
+  %(store_url)s

=== added file 'lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt'
--- lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/emailtemplates/snapbuild-releasefailed.txt	2016-06-30 17:00:36 +0000
@@ -0,0 +1,7 @@
+Launchpad asked the store to release this snap package, but it failed:
+
+  %(store_error_message)s
+
+You can try to release it manually here:
+
+  %(store_url)s

=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/mail/snapbuild.py	2016-06-30 17:00:36 +0000
@@ -60,6 +60,34 @@
             config.canonical.noreply_from_address,
             "snap-build-upload-scan-failed", build)
 
+    @classmethod
+    def forManualReview(cls, build):
+        """Create a mailer for notifying about manual review.
+
+        :param build: The relevant build.
+        """
+        requester = build.requester
+        recipients = {requester: RecipientReason.forBuildRequester(requester)}
+        return cls(
+            "%(snap_name)s held for manual review",
+            "snapbuild-manualreview.txt", recipients,
+            config.canonical.noreply_from_address,
+            "snap-build-release-manual-review", build)
+
+    @classmethod
+    def forReleaseFailure(cls, build):
+        """Create a mailer for notifying about store release failures.
+
+        :param build: The relevant build.
+        """
+        requester = build.requester
+        recipients = {requester: RecipientReason.forBuildRequester(requester)}
+        return cls(
+            "Store release failed for %(snap_name)s",
+            "snapbuild-releasefailed.txt", recipients,
+            config.canonical.noreply_from_address,
+            "snap-build-release-failed", build)
+
     def __init__(self, subject, template_name, recipients, from_address,
                  notification_type, build):
         super(SnapBuildMailer, self).__init__(
@@ -77,10 +105,12 @@
         """See `BaseMailer`."""
         build = self.build
         upload_job = build.store_upload_jobs.first()
-        if upload_job is None or upload_job.error_message is None:
+        if upload_job is None:
             error_message = ""
+            store_url = ""
         else:
-            error_message = upload_job.error_message
+            error_message = upload_job.error_message or ""
+            store_url = upload_job.store_url or ""
         params = super(SnapBuildMailer, self)._getTemplateParams(
             email, recipient)
         params.update({
@@ -100,6 +130,7 @@
             "snap_authorize_url": canonical_url(
                 build.snap, view_name="+authorize"),
             "store_error_message": error_message,
+            "store_url": store_url,
             })
         if build.duration is not None:
             duration_formatter = DurationFormatterAPI(build.duration)

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2016-06-30 17:00:36 +0000
@@ -50,8 +50,10 @@
     ISnapStoreUploadJobSource,
     )
 from lp.snappy.interfaces.snapstoreclient import (
+    BadReleaseResponse,
     BadScanStatusResponse,
     ISnapStoreClient,
+    ReleaseFailedResponse,
     ScanFailedResponse,
     UnauthorizedUploadResponse,
     UploadNotScannedYetResponse,
@@ -157,6 +159,10 @@
         return oops_vars
 
 
+class ManualReview(Exception):
+    pass
+
+
 @implementer(ISnapStoreUploadJob)
 @provider(ISnapStoreUploadJobSource)
 class SnapStoreUploadJob(SnapBuildJobDerived):
@@ -164,7 +170,12 @@
 
     class_job_type = SnapBuildJobType.STORE_UPLOAD
 
-    user_error_types = (UnauthorizedUploadResponse, ScanFailedResponse)
+    user_error_types = (
+        UnauthorizedUploadResponse,
+        ScanFailedResponse,
+        ManualReview,
+        ReleaseFailedResponse,
+        )
 
     # XXX cjwatson 2016-05-04: identify transient upload failures and retry
     retry_error_types = (UploadNotScannedYetResponse,)
@@ -207,14 +218,19 @@
         try:
             if "status_url" not in self.metadata:
                 self.metadata["status_url"] = client.upload(self.snapbuild)
-            self.store_url = client.checkStatus(self.metadata["status_url"])
+            if self.store_url is None:
+                self.store_url, self.metadata["store_revision"] = (
+                    client.checkStatus(self.metadata["status_url"]))
+            if self.snapbuild.snap.store_channels:
+                if self.metadata["store_revision"] is None:
+                    raise ManualReview(
+                        "Package held for manual review on the store; "
+                        "cannot release it automatically.")
+                client.release(self.snapbuild, self.metadata["store_revision"])
             self.error_message = None
         except self.retry_error_types:
             raise
         except Exception as e:
-            # Abort work done so far, but make sure that we commit the error
-            # message.
-            transaction.abort()
             self.error_message = str(e)
             if isinstance(e, UnauthorizedUploadResponse):
                 mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
@@ -222,5 +238,14 @@
             elif isinstance(e, (BadScanStatusResponse, ScanFailedResponse)):
                 mailer = SnapBuildMailer.forUploadScanFailure(self.snapbuild)
                 mailer.sendAll()
+            elif isinstance(e, ManualReview):
+                mailer = SnapBuildMailer.forManualReview(self.snapbuild)
+                mailer.sendAll()
+            elif isinstance(e, (BadReleaseResponse, ReleaseFailedResponse)):
+                mailer = SnapBuildMailer.forReleaseFailure(self.snapbuild)
+                mailer.sendAll()
+            # The normal job infrastructure will abort the transaction, but
+            # we want to commit instead: the only database changes we make
+            # are to this job's metadata and should be preserved.
             transaction.commit()
             raise

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2016-06-30 17:00:36 +0000
@@ -20,6 +20,7 @@
     ISnapStoreUploadJob,
     )
 from lp.snappy.interfaces.snapstoreclient import (
+    BadReleaseResponse,
     BadScanStatusResponse,
     ISnapStoreClient,
     UnauthorizedUploadResponse,
@@ -47,6 +48,7 @@
     def __init__(self):
         self.upload = FakeMethod()
         self.checkStatus = FakeMethod()
+        self.release = FakeMethod()
 
 
 class TestSnapBuildJob(TestCaseWithFactory):
@@ -95,12 +97,13 @@
         job = SnapStoreUploadJob.create(snapbuild)
         client = FakeSnapStoreClient()
         client.upload.result = self.status_url
-        client.checkStatus.result = self.store_url
+        client.checkStatus.result = (self.store_url, 1)
         self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
         self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
         self.assertIsNone(job.error_message)
@@ -118,6 +121,7 @@
             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("An upload failure", job.error_message)
@@ -138,6 +142,7 @@
             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("Authorization failed.", job.error_message)
@@ -178,6 +183,7 @@
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
         self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
         self.assertIsNone(job.error_message)
@@ -190,11 +196,12 @@
         client.upload.calls = []
         client.checkStatus.calls = []
         client.checkStatus.failure = None
-        client.checkStatus.result = self.store_url
+        client.checkStatus.result = (self.store_url, 1)
         with dbuser(config.ISnapStoreUploadJobSource.dbuser):
             JobRunner([job]).runAll()
         self.assertEqual([], client.upload.calls)
         self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
         self.assertIsNone(job.error_message)
@@ -216,6 +223,7 @@
             JobRunner([job]).runAll()
         self.assertEqual([((snapbuild,), {})], client.upload.calls)
         self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
         self.assertEqual("Scan failed.", job.error_message)
@@ -239,3 +247,114 @@
         self.assertEqual(
             "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n"
             "You are the requester of the build.\n" % snapbuild.id, footer)
+
+    def test_run_release(self):
+        # A run configured to automatically release the package to certain
+        # channels does so.
+        snapbuild = self.factory.makeSnapBuild(
+            store_channels=["stable", "edge"])
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.result = (self.store_url, 1)
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([((snapbuild, 1), {})], client.release.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual(self.store_url, job.store_url)
+        self.assertIsNone(job.error_message)
+        self.assertEqual([], pop_notifications())
+
+    def test_run_release_manual_review_notifies(self):
+        # A run configured to automatically release the package to certain
+        # channels but that encounters the manual review state on upload
+        # sends mail.
+        requester = self.factory.makePerson(name="requester")
+        snapbuild = self.factory.makeSnapBuild(
+            requester=requester, name="test-snap", owner=requester,
+            store_channels=["stable", "edge"])
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.result = (self.store_url, None)
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([], client.release.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual(
+            "Package held for manual review on the store; "
+            "cannot release it automatically.",
+            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("test-snap held for manual review", subject)
+        self.assertEqual(
+            "Requester", notification["X-Launchpad-Message-Rationale"])
+        self.assertEqual(
+            requester.name, notification["X-Launchpad-Message-For"])
+        self.assertEqual(
+            "snap-build-release-manual-review",
+            notification["X-Launchpad-Notification-Type"])
+        body, footer = notification.get_payload(decode=True).split("\n-- \n")
+        self.assertIn(self.store_url, body)
+        self.assertEqual(
+            "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n"
+            "You are the requester of the build.\n" % snapbuild.id, footer)
+
+    def test_run_release_failure_notifies(self):
+        # A run configured to automatically release the package to certain
+        # channels but that fails to do so sends mail.
+        requester = self.factory.makePerson(name="requester")
+        snapbuild = self.factory.makeSnapBuild(
+            requester=requester, name="test-snap", owner=requester,
+            store_channels=["stable", "edge"])
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.result = self.status_url
+        client.checkStatus.result = (self.store_url, 1)
+        client.release.failure = BadReleaseResponse("Failed to publish")
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertEqual([((self.status_url,), {})], client.checkStatus.calls)
+        self.assertEqual([((snapbuild, 1), {})], client.release.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual("Failed to publish", 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 release 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-release-failed",
+            notification["X-Launchpad-Notification-Type"])
+        body, footer = notification.get_payload(decode=True).split("\n-- \n")
+        self.assertIn("Failed to publish", body)
+        self.assertIn(self.store_url, body)
+        self.assertEqual(
+            "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n"
+            "You are the requester of the build.\n" % snapbuild.id, footer)