launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-store-revision into lp:launchpad.

Commit message:
Export store-assigned revision as SnapBuild.store_upload_revision.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1679197 in Launchpad itself: "Export store-assigned revision of snaps"
  https://bugs.launchpad.net/launchpad/+bug/1679197

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-store-revision/+merge/321712
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-store-revision into lp:launchpad.

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2017-02-27 18:46:38 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2017-04-03 14:48:19 +0000
@@ -195,6 +195,11 @@
             "The URL to use for managing this package in the store."),
         required=False, readonly=True))
 
+    store_upload_revision = exported(Int(
+        title=_("Store revision"),
+        description=_("The revision assigned to this package by the store."),
+        required=False, readonly=True))
+
     store_upload_error_message = exported(TextLine(
         title=_("Store upload error message"),
         description=_(

=== modified file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py	2017-03-20 00:03:52 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py	2017-04-03 14:48:19 +0000
@@ -19,7 +19,10 @@
     Attribute,
     Interface,
     )
-from zope.schema import TextLine
+from zope.schema import (
+    Int,
+    TextLine,
+    )
 
 from lp import _
 from lp.services.job.interfaces.job import (
@@ -58,6 +61,10 @@
         title=_("The URL on the store corresponding to this build"),
         required=False, readonly=True)
 
+    store_revision = Int(
+        title=_("The revision assigned to this build by the store"),
+        required=False, readonly=True)
+
 
 class ISnapStoreUploadJobSource(IJobSource):
 

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2017-02-27 18:46:38 +0000
+++ lib/lp/snappy/model/snapbuild.py	2017-04-03 14:48:19 +0000
@@ -439,6 +439,11 @@
         return job and job.store_url
 
     @property
+    def store_upload_revision(self):
+        job = self.last_store_upload_job
+        return job and job.store_revision
+
+    @property
     def store_upload_error_message(self):
         job = self.last_store_upload_job
         return job and job.error_message

=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	2017-03-20 00:03:52 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2017-04-03 14:48:19 +0000
@@ -225,6 +225,16 @@
         """See `ISnapStoreUploadJob`."""
         self.metadata["store_url"] = url
 
+    @property
+    def store_revision(self):
+        """See `ISnapStoreUploadJob`."""
+        return self.metadata.get("store_revision")
+
+    @store_revision.setter
+    def store_revision(self, revision):
+        """See `ISnapStoreUploadJob`."""
+        self.metadata["store_revision"] = revision
+
     # Ideally we'd just override Job._set_status or similar, but
     # lazr.delegates makes that difficult, so we use this to override all
     # the individual Job lifecycle methods instead.
@@ -259,14 +269,14 @@
             if "status_url" not in self.metadata:
                 self.metadata["status_url"] = client.upload(self.snapbuild)
             if self.store_url is None:
-                self.store_url, self.metadata["store_revision"] = (
+                self.store_url, self.store_revision = (
                     client.checkStatus(self.metadata["status_url"]))
             if self.snapbuild.snap.store_channels:
-                if self.metadata["store_revision"] is None:
+                if self.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"])
+                client.release(self.snapbuild, self.store_revision)
             self.error_message = None
         except self.retry_error_types:
             raise

=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	2017-03-20 00:03:52 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2017-04-03 14:48:19 +0000
@@ -137,7 +137,8 @@
                     repr(delivery))
 
     def test_run(self):
-        # The job uploads the build to the store and records the store URL.
+        # The job uploads the build to the store and records the store URL
+        # and revision.
         snapbuild = self.makeSnapBuild()
         self.assertContentEqual([], snapbuild.store_upload_jobs)
         job = SnapStoreUploadJob.create(snapbuild)
@@ -152,6 +153,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual(1, job.store_revision)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
@@ -171,6 +173,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertEqual("An upload failure", job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertWebhookDeliveries(
@@ -196,6 +199,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertEqual("Authorization failed.", job.error_message)
         [notification] = pop_notifications()
         self.assertEqual(
@@ -244,6 +248,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertEqual("Failed to upload", job.error_message)
         [notification] = pop_notifications()
         self.assertEqual(
@@ -291,6 +296,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertEqual(JobStatus.WAITING, job.job.status)
@@ -310,6 +316,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual(1, job.store_revision)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertEqual(JobStatus.COMPLETED, job.job.status)
@@ -335,6 +342,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertIsNone(job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertEqual("Scan failed.", job.error_message)
         [notification] = pop_notifications()
         self.assertEqual(
@@ -378,6 +386,7 @@
         self.assertEqual([((snapbuild, 1), {})], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual(1, job.store_revision)
         self.assertIsNone(job.error_message)
         self.assertEqual([], pop_notifications())
         self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
@@ -405,6 +414,7 @@
         self.assertEqual([], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
+        self.assertIsNone(job.store_revision)
         self.assertEqual(
             "Package held for manual review on the store; "
             "cannot release it automatically.",
@@ -457,6 +467,7 @@
         self.assertEqual([((snapbuild, 1), {})], client.release.calls)
         self.assertContentEqual([job], snapbuild.store_upload_jobs)
         self.assertEqual(self.store_url, job.store_url)
+        self.assertEqual(1, job.store_revision)
         self.assertEqual("Failed to publish", job.error_message)
         [notification] = pop_notifications()
         self.assertEqual(