← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/snap-build-manual-upload into lp:launchpad

 

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-build-manual-upload into lp:launchpad.

Commit message:
Allow manually uploading a completed snap package build to the store.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-build-manual-upload/+merge/300503

Allow manually uploading a completed snap package build to the store.

This is often useful for various reasons (upload failed, package wasn't previously configured for store upload, etc.).  Previously the only option was to download it and upload it to the store yourself, which isn't really very satisfactory given that Launchpad could do it for you.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-build-manual-upload into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snapbuild.py'
--- lib/lp/snappy/browser/snapbuild.py	2016-07-16 07:46:23 +0000
+++ lib/lp/snappy/browser/snapbuild.py	2016-07-19 17:18:24 +0000
@@ -10,6 +10,7 @@
     'SnapBuildView',
     ]
 
+from zope.component import getUtility
 from zope.interface import Interface
 
 from lp.app.browser.launchpadform import (
@@ -26,12 +27,11 @@
     canonical_url,
     ContextMenu,
     enabled_with_permission,
-    LaunchpadView,
     Link,
     Navigation,
-    structured,
     )
 from lp.snappy.interfaces.snapbuild import ISnapBuild
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.soyuz.interfaces.binarypackagebuild import IBuildRescoreForm
 
 
@@ -61,9 +61,12 @@
             enabled=self.context.can_be_rescored)
 
 
-class SnapBuildView(LaunchpadView):
+class SnapBuildView(LaunchpadFormView):
     """Default view of a SnapBuild."""
 
+    class schema(Interface):
+        """Schema for uploading a build."""
+
     @property
     def label(self):
         return self.context.title
@@ -84,25 +87,40 @@
     def has_files(self):
         return bool(self.files)
 
-    @cachedproperty
-    def store_upload_status(self):
-        job = self.context.store_upload_jobs.first()
-        if job is None:
-            return None
-        elif job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
-            return "Store upload in progress"
-        elif job.job.status == JobStatus.COMPLETED:
-            return structured(
-                '<a href="%s">Manage this package in the store</a>',
-                job.store_url).escapedtext
-        elif job.store_url:
-            return structured(
-                '<a href="%s">Manage this package in the store</a><br />'
-                'Releasing package to channels failed: %s',
-                job.store_url, job.error_message).escapedtext
-        else:
-            return structured(
-                "Store upload failed: %s", job.error_message).escapedtext
+    @property
+    def last_upload_job(self):
+        return self.context.store_upload_jobs.first()
+
+    @property
+    def next_url(self):
+        return canonical_url(self.context)
+
+    @action('Upload build to store', name='upload')
+    def upload_action(self, action, data):
+        """Schedule an upload of this build to the store."""
+        if not self.context.snap.can_upload_to_store:
+            self.request.response.addWarningNotification(
+                "Cannot upload this package to the store because it is not "
+                "properly configured.")
+            return
+        if not self.has_files:
+            self.request.response.addWarningNotification(
+                "Cannot upload this package because it has no files.")
+            return
+        job = self.last_upload_job
+        if job is not None:
+            if job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
+                self.request.response.addWarningNotification(
+                    "An upload of this package is already in progress.")
+                return
+            if job.job.status == JobStatus.COMPLETED:
+                self.request.response.addWarningNotification(
+                    "Cannot upload this package because it has already "
+                    "been uploaded.")
+                return
+        getUtility(ISnapStoreUploadJobSource).create(self.context)
+        self.request.response.addInfoNotification(
+            "An upload has been scheduled and will run as soon as possible.")
 
 
 class SnapBuildCancelView(LaunchpadFormView):

=== modified file 'lib/lp/snappy/browser/tests/test_snapbuild.py'
--- lib/lp/snappy/browser/tests/test_snapbuild.py	2016-07-16 07:46:23 +0000
+++ lib/lp/snappy/browser/tests/test_snapbuild.py	2016-07-19 17:18:24 +0000
@@ -5,6 +5,8 @@
 
 __metaclass__ = type
 
+import re
+
 from fixtures import FakeLogger
 from mechanize import LinkNotFoundError
 import soupmatchers
@@ -82,7 +84,7 @@
             soupmatchers.Tag(
                 "store upload status", "li",
                 attrs={"id": "store-upload-status"},
-                text="Store upload in progress")))
+                text=re.compile(r"^\s*Store upload in progress\s*$"))))
 
     def test_store_upload_status_completed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
@@ -98,7 +100,8 @@
                     attrs={"id": "store-upload-status"}),
                 soupmatchers.Tag(
                     "store link", "a", attrs={"href": job.store_url},
-                    text="Manage this package in the store"))))
+                    text=re.compile(
+                        r"^\s*Manage this package in the store\s*$")))))
 
     def test_store_upload_status_failed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
@@ -111,7 +114,8 @@
             soupmatchers.Tag(
                 "store upload status", "li",
                 attrs={"id": "store-upload-status"},
-                text="Store upload failed: Scan failed.")))
+                text=re.compile(
+                    r"^\s*Store upload failed:\s+Scan failed.\s*$"))))
 
     def test_store_upload_status_release_failed(self):
         build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
@@ -126,9 +130,9 @@
                 soupmatchers.Tag(
                     "store upload status", "li",
                     attrs={"id": "store-upload-status"},
-                    text=(
-                        "Releasing package to channels failed: "
-                        "Failed to publish")),
+                    text=re.compile(
+                        r"^\s*Releasing package to channels failed:\s+"
+                        r"Failed to publish\s*$")),
                 soupmatchers.Tag(
                     "store link", "a", attrs={"href": job.store_url}))))
 
@@ -232,6 +236,130 @@
             "Cannot rescore this build because it is not queued.",
             browser.contents)
 
+    def setUpStoreUpload(self):
+        self.pushConfig(
+            "snappy", store_url="http://sca.example/";,
+            store_upload_url="http://updown.example/";)
+        with admin_logged_in():
+            snappyseries = self.factory.makeSnappySeries(
+                usable_distro_series=[self.build.snap.distro_series])
+        with person_logged_in(self.requester):
+            self.build.snap.store_series = snappyseries
+            self.build.snap.store_name = self.factory.getUniqueUnicode()
+            self.build.snap.store_secrets = {
+                "root": "dummy-root", "discharge": "dummy-discharge"}
+
+    def test_store_upload(self):
+        # A build not previously uploaded to the store can be uploaded
+        # manually.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeSnapFile(
+            snapbuild=self.build,
+            libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        browser.getControl("Upload this package to the store").click()
+        self.assertEqual(self.build_url, browser.url)
+        login(ANONYMOUS)
+        [job] = getUtility(ISnapStoreUploadJobSource).iterReady()
+        self.assertEqual(JobStatus.WAITING, job.job.status)
+        self.assertEqual(self.build, job.snapbuild)
+        self.assertEqual(
+            "An upload has been scheduled and will run as soon as possible.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
+    def test_store_upload_retry(self):
+        # A build with a previously-failed store upload can have the upload
+        # retried.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeSnapFile(
+            snapbuild=self.build,
+            libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+        old_job = getUtility(ISnapStoreUploadJobSource).create(self.build)
+        removeSecurityProxy(old_job).job._status = JobStatus.FAILED
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        browser.getControl("Retry").click()
+        self.assertEqual(self.build_url, browser.url)
+        login(ANONYMOUS)
+        [job] = getUtility(ISnapStoreUploadJobSource).iterReady()
+        self.assertEqual(JobStatus.WAITING, job.job.status)
+        self.assertEqual(self.build, job.snapbuild)
+        self.assertEqual(
+            "An upload has been scheduled and will run as soon as possible.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
+    def test_store_upload_not_configured(self):
+        # A build that is not properly configured cannot be uploaded to the
+        # store.  (This requires changing configuration between requests.)
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        with person_logged_in(self.requester):
+            self.build.snap.store_name = None
+        browser.getControl("Upload this package to the store").click()
+        self.assertEqual(self.build_url, browser.url)
+        login(ANONYMOUS)
+        self.assertEqual(
+            [], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
+        self.assertEqual(
+            "Cannot upload this package to the store because it is not "
+            "properly configured.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
+    def test_store_upload_no_files(self):
+        # A build with no files cannot be uploaded to the store.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        browser.getControl("Upload this package to the store").click()
+        self.assertEqual(self.build_url, browser.url)
+        login(ANONYMOUS)
+        self.assertEqual(
+            [], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
+        self.assertEqual(
+            "Cannot upload this package because it has no files.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
+    def test_store_upload_already_in_progress(self):
+        # A build with an upload already in progress will not have another
+        # one created.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeSnapFile(
+            snapbuild=self.build,
+            libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        with person_logged_in(self.requester):
+            old_job = getUtility(ISnapStoreUploadJobSource).create(self.build)
+        browser.getControl("Upload this package to the store").click()
+        login(ANONYMOUS)
+        self.assertEqual(
+            [old_job], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
+        self.assertEqual(
+            "An upload of this package is already in progress.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
+    def test_store_upload_already_uploaded(self):
+        # A build with an upload that has already completed will not have
+        # another one created.
+        self.setUpStoreUpload()
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.factory.makeSnapFile(
+            snapbuild=self.build,
+            libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+        browser = self.getViewBrowser(self.build, user=self.requester)
+        with person_logged_in(self.requester):
+            old_job = getUtility(ISnapStoreUploadJobSource).create(self.build)
+            removeSecurityProxy(old_job).job._status = JobStatus.COMPLETED
+        browser.getControl("Upload this package to the store").click()
+        login(ANONYMOUS)
+        self.assertEqual(
+            [], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
+        self.assertEqual(
+            "Cannot upload this package because it has already been uploaded.",
+            extract_text(find_tags_by_class(browser.contents, "message")[0]))
+
     def test_builder_history(self):
         Store.of(self.build).flush()
         self.build.updateStatus(

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2016-07-15 17:11:09 +0000
+++ lib/lp/snappy/model/snap.py	2016-07-19 17:18:24 +0000
@@ -10,6 +10,7 @@
     datetime,
     timedelta,
     )
+
 import pytz
 from storm.expr import (
     And,
@@ -329,7 +330,6 @@
         return (
             config.snappy.store_upload_url is not None and
             config.snappy.store_url is not None and
-            self.store_upload and
             self.store_series is not None and
             self.store_name is not None and
             self.store_secrets is not None and

=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py	2016-05-11 00:06:17 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py	2016-07-19 17:18:24 +0000
@@ -31,6 +31,6 @@
         getUtility(IWebhookSet).trigger(
             snapbuild.snap, "snap:build:0.1", payload)
 
-    if (snapbuild.snap.can_upload_to_store and
+    if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
             snapbuild.status == BuildStatus.FULLYBUILT):
         getUtility(ISnapStoreUploadJobSource).create(snapbuild)

=== modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
--- lib/lp/snappy/templates/snapbuild-index.pt	2016-07-14 15:19:03 +0000
+++ lib/lp/snappy/templates/snapbuild-index.pt	2016-07-19 17:18:24 +0000
@@ -160,9 +160,42 @@
         (<span tal:replace="file/content/filesize/fmt:bytes" />)
       </li>
       <li id="store-upload-status"
-          tal:define="store_upload_status view/store_upload_status"
-          tal:condition="store_upload_status"
-          tal:content="structure store_upload_status" />
+          tal:define="job view/last_upload_job"
+          tal:condition="job">
+        <tal:job-status define="job_status job/job/status/title">
+          <a tal:condition="job/store_url" tal:attributes="href job/store_url">
+            Manage this package in the store
+          </a>
+          <tal:in-progress
+              condition="python: job_status in ('Waiting', 'Running')">
+            <br tal:condition="job/store_url" />
+            Store upload in progress
+          </tal:in-progress>
+          <tal:failed condition="python: job_status == 'Failed'">
+            <tal:failed-upload condition="not: job/store_url">
+              Store upload failed:
+            </tal:failed-upload>
+            <tal:failed-release condition="job/store_url">
+              <br />
+              Releasing package to channels failed:
+            </tal:failed-release>
+            <tal:error-message replace="job/error_message" />
+            <form action="" method="POST">
+              <input type="submit" name="field.actions.upload" value="Retry" />
+            </form>
+          </tal:failed>
+        </tal:job-status>
+      </li>
+      <li id="store-upload-status"
+          tal:condition="python:
+            context.status.title == 'Successfully built' and
+            context.snap.can_upload_to_store and
+            view.last_upload_job is None">
+        <form action="" method="POST">
+          <input type="submit" name="field.actions.upload"
+                 value="Upload this package to the store" />
+        </form>
+      </li>
     </ul>
 
     <div


Follow ups