← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~cjwatson/launchpad/snap-store-upload-state-api into lp:launchpad

 

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

Commit message:
Export snap build store upload states on the webservice.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-store-upload-state-api/+merge/318396

build.snapcraft.io needs this so that it can display its full range of specified states for snap builds.

The main complexity here is making sure that query counts scale properly, since we might now need to work out the store upload states for more than one build.  Figuring out a bulk equivalent of SnapBuild.last_store_upload_job was somewhat fiddly, but I think I managed it.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-store-upload-state-api into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snapbuild.py'
--- lib/lp/snappy/browser/snapbuild.py	2016-07-27 01:43:45 +0000
+++ lib/lp/snappy/browser/snapbuild.py	2017-02-27 18:59:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """SnapBuild views."""
@@ -88,10 +88,6 @@
         return bool(self.files)
 
     @property
-    def last_upload_job(self):
-        return self.context.store_upload_jobs.first()
-
-    @property
     def next_url(self):
         return canonical_url(self.context)
 

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2016-12-12 13:41:17 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2017-02-27 18:59:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Snap package build interfaces."""
@@ -11,10 +11,15 @@
     'ISnapBuildSet',
     'ISnapBuildStatusChangedEvent',
     'ISnapFile',
+    'SnapBuildStoreUploadStatus',
     ]
 
 import httplib
 
+from lazr.enum import (
+    EnumeratedType,
+    Item,
+    )
 from lazr.restful.declarations import (
     error_status,
     export_as_webservice_entry,
@@ -73,6 +78,45 @@
         required=True, readonly=True)
 
 
+class SnapBuildStoreUploadStatus(EnumeratedType):
+    """Snap build store upload status type
+
+    Snap builds may be uploaded to the store. This represents the state of
+    that process.
+    """
+
+    UNSCHEDULED = Item("""
+        Unscheduled
+
+        No upload of this snap build to the store is scheduled.
+        """)
+
+    PENDING = Item("""
+        Pending
+
+        This snap build is queued for upload to the store.
+        """)
+
+    FAILEDTOUPLOAD = Item("""
+        Failed to upload
+
+        The last attempt to upload this snap build to the store failed.
+        """)
+
+    FAILEDTORELEASE = Item("""
+        Failed to release to channels
+
+        The last attempt to release this snap build to its intended set of
+        channels failed.
+        """)
+
+    UPLOADED = Item("""
+        Uploaded
+
+        This snap build was successfully uploaded to the store.
+        """)
+
+
 class ISnapBuildView(IPackageBuild):
     """`ISnapBuild` attributes that require launchpad.View permission."""
 
@@ -137,6 +181,27 @@
         value_type=Reference(schema=Interface),
         readonly=True)
 
+    # Really ISnapStoreUploadJob.
+    last_store_upload_job = Reference(
+        title=_("Last store upload job for this build."), schema=Interface)
+
+    store_upload_status = exported(Choice(
+        title=_("Store upload status"),
+        vocabulary=SnapBuildStoreUploadStatus, required=True, readonly=False))
+
+    store_upload_url = exported(TextLine(
+        title=_("Store URL"),
+        description=_(
+            "The URL to use for managing this package in the store."),
+        required=False, readonly=True))
+
+    store_upload_error_message = exported(TextLine(
+        title=_("Store upload error message"),
+        description=_(
+            "The error message, if any, from the last attempt to upload "
+            "this snap build to the store."),
+        required=False, readonly=True))
+
     def getFiles():
         """Retrieve the build's `ISnapFile` records.
 
@@ -229,3 +294,6 @@
     def new(requester, snap, archive, distro_arch_series, pocket,
             date_created=DEFAULT):
         """Create an `ISnapBuild`."""
+
+    def preloadBuildsData(builds):
+        """Load the data related to a list of snap builds."""

=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py	2016-10-16 22:04:39 +0000
+++ lib/lp/snappy/mail/snapbuild.py	2017-02-27 18:59:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -118,7 +118,7 @@
     def _getTemplateParams(self, email, recipient):
         """See `BaseMailer`."""
         build = self.build
-        upload_job = build.store_upload_jobs.first()
+        upload_job = build.last_store_upload_job
         if upload_job is None:
             error_message = ""
             store_url = ""

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2017-02-06 14:34:35 +0000
+++ lib/lp/snappy/model/snap.py	2017-02-27 18:59:10 +0000
@@ -85,6 +85,7 @@
     DEFAULT,
     UTC_NOW,
     )
+from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
 from lp.services.database.interfaces import (
     IMasterStore,
@@ -500,7 +501,12 @@
             query_args.append(filter_term)
         result = Store.of(self).find(SnapBuild, *query_args)
         result.order_by(order_by)
-        return result
+
+        def eager_load(rows):
+            getUtility(ISnapBuildSet).preloadBuildsData(rows)
+            getUtility(IBuildQueueSet).preloadForBuildFarmJobs(result)
+
+        return DecoratedResultSet(result, pre_iter_hook=eager_load)
 
     def getBuildSummariesForSnapBuildIds(self, snap_build_ids):
         """See `ISnap`."""
@@ -512,7 +518,6 @@
         builds = self._getBuilds(filter_term, order_by)
 
         # Prefetch data to keep DB query count constant
-        getUtility(IBuildQueueSet).preloadForBuildFarmJobs(builds)
         lfas = load_related(LibraryFileAlias, builds, ["log_id"])
         load_related(LibraryFileContent, lfas, ["contentID"])
 

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2016-12-12 13:41:17 +0000
+++ lib/lp/snappy/model/snapbuild.py	2017-02-27 18:59:10 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -8,14 +8,23 @@
     ]
 
 from datetime import timedelta
+from operator import attrgetter
 
 import pytz
+from storm.expr import (
+    Column,
+    Table,
+    With,
+    )
 from storm.locals import (
+    And,
     Bool,
     DateTime,
     Desc,
     Int,
     Reference,
+    Select,
+    SQL,
     Store,
     Storm,
     Unicode,
@@ -55,7 +64,10 @@
     LibraryFileAlias,
     LibraryFileContent,
     )
-from lp.services.propertycache import cachedproperty
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 from lp.snappy.interfaces.snap import ISnapSet
 from lp.snappy.interfaces.snapbuild import (
     CannotScheduleStoreUpload,
@@ -63,6 +75,7 @@
     ISnapBuildSet,
     ISnapBuildStatusChangedEvent,
     ISnapFile,
+    SnapBuildStoreUploadStatus,
     )
 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.snappy.mail.snapbuild import SnapBuildMailer
@@ -401,6 +414,35 @@
         return DecoratedResultSet(
             jobs, lambda job: job.makeDerived(), pre_iter_hook=preload_jobs)
 
+    @cachedproperty
+    def last_store_upload_job(self):
+        return self.store_upload_jobs.first()
+
+    @property
+    def store_upload_status(self):
+        job = self.last_store_upload_job
+        if job is None or job.job.status == JobStatus.SUSPENDED:
+            return SnapBuildStoreUploadStatus.UNSCHEDULED
+        elif job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
+            return SnapBuildStoreUploadStatus.PENDING
+        elif job.job.status == JobStatus.COMPLETED:
+            return SnapBuildStoreUploadStatus.UPLOADED
+        else:
+            if job.store_url:
+                return SnapBuildStoreUploadStatus.FAILEDTORELEASE
+            else:
+                return SnapBuildStoreUploadStatus.FAILEDTOUPLOAD
+
+    @property
+    def store_upload_url(self):
+        job = self.last_store_upload_job
+        return job and job.store_url
+
+    @property
+    def store_upload_error_message(self):
+        job = self.last_store_upload_job
+        return job and job.error_message
+
     def scheduleStoreUpload(self):
         """See `ISnapBuild`."""
         if not self.snap.can_upload_to_store:
@@ -410,15 +452,13 @@
         if not self.was_built or self.getFiles().is_empty():
             raise CannotScheduleStoreUpload(
                 "Cannot upload this package because it has no files.")
-        job = self.store_upload_jobs.first()
-        if job is not None:
-            if job.job.status in (JobStatus.WAITING, JobStatus.RUNNING):
-                raise CannotScheduleStoreUpload(
-                    "An upload of this package is already in progress.")
-            if job.job.status == JobStatus.COMPLETED:
-                raise CannotScheduleStoreUpload(
-                    "Cannot upload this package because it has already "
-                    "been uploaded.")
+        if self.store_upload_status == SnapBuildStoreUploadStatus.PENDING:
+            raise CannotScheduleStoreUpload(
+                "An upload of this package is already in progress.")
+        elif self.store_upload_status == SnapBuildStoreUploadStatus.UPLOADED:
+            raise CannotScheduleStoreUpload(
+                "Cannot upload this package because it has already been "
+                "uploaded.")
         getUtility(ISnapStoreUploadJobSource).create(self)
 
 
@@ -455,7 +495,8 @@
         # Circular import.
         from lp.snappy.model.snap import Snap
         load_related(Person, builds, ["requester_id"])
-        load_related(LibraryFileAlias, builds, ["log_id"])
+        lfas = load_related(LibraryFileAlias, builds, ["log_id"])
+        load_related(LibraryFileContent, lfas, ["contentID"])
         archives = load_related(Archive, builds, ["archive_id"])
         load_related(Person, archives, ["ownerID"])
         distroarchseries = load_related(
@@ -465,6 +506,29 @@
         load_related(Distribution, distroseries, ['distributionID'])
         snaps = load_related(Snap, builds, ["snap_id"])
         getUtility(ISnapSet).preloadDataForSnaps(snaps)
+        snapbuild_ids = set(map(attrgetter("id"), builds))
+        latest_jobs_cte = With("LatestJobs", Select(
+            (SnapBuildJob.job_id,
+             SQL(
+                 "rank() OVER "
+                 "(PARTITION BY snapbuild ORDER BY job DESC) AS rank")),
+            tables=SnapBuildJob,
+            where=And(
+                SnapBuildJob.snapbuild_id.is_in(snapbuild_ids),
+                SnapBuildJob.job_type == SnapBuildJobType.STORE_UPLOAD)))
+        LatestJobs = Table("LatestJobs")
+        sbjs = list(IStore(SnapBuildJob).with_(latest_jobs_cte).using(
+            SnapBuildJob, LatestJobs).find(
+                SnapBuildJob,
+                SnapBuildJob.job_id == Column("job", LatestJobs),
+                Column("rank", LatestJobs) == 1))
+        sbj_map = {}
+        for sbj in sbjs:
+            sbj_map[sbj.snapbuild] = sbj.makeDerived()
+        for build in builds:
+            get_property_cache(build).last_store_upload_job = (
+                sbj_map.get(build))
+        load_related(Job, sbjs, ["job_id"])
 
     def getByBuildFarmJobs(self, build_farm_jobs):
         """See `ISpecificBuildFarmJobSource`."""

=== modified file 'lib/lp/snappy/templates/snapbuild-index.pt'
--- lib/lp/snappy/templates/snapbuild-index.pt	2016-07-19 16:32:46 +0000
+++ lib/lp/snappy/templates/snapbuild-index.pt	2017-02-27 18:59:10 +0000
@@ -160,37 +160,40 @@
         (<span tal:replace="file/content/filesize/fmt:bytes" />)
       </li>
       <li id="store-upload-status"
-          tal:define="job view/last_upload_job"
+          tal:define="job context/last_store_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>
+        <a tal:condition="context/store_upload_url"
+           tal:attributes="href context/store_upload_url">
+          Manage this package in the store
+        </a>
+        <tal:pending
+            condition="context/store_upload_status/enumvalue:PENDING">
+          <br tal:condition="context/store_upload_url" />
+          Store upload in progress
+        </tal:pending>
+        <tal:failed-upload
+            condition="context/store_upload_status/enumvalue:FAILEDTOUPLOAD">
+          Store upload failed:
+          <tal:error-message replace="context/store_upload_error_message" />
+          <form action="" method="POST">
+            <input type="submit" name="field.actions.upload" value="Retry" />
+          </form>
+        </tal:failed-upload>
+        <tal:failed-release
+            condition="context/store_upload_status/enumvalue:FAILEDTORELEASE">
+          <br />
+          Releasing package to channels failed:
+          <tal:error-message replace="context/store_upload_error_message" />
+          <form action="" method="POST">
+            <input type="submit" name="field.actions.upload" value="Retry" />
+          </form>
+        </tal:failed-release>
       </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">
+            context.last_store_upload_job is None">
         <form action="" method="POST">
           <input type="submit" name="field.actions.upload"
                  value="Upload this package to the store" />

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2017-02-06 14:34:35 +0000
+++ lib/lp/snappy/tests/test_snap.py	2017-02-27 18:59:10 +0000
@@ -87,6 +87,7 @@
     StormStatementRecorder,
     TestCaseWithFactory,
     )
+from lp.testing.dbuser import dbuser
 from lp.testing.fakemethod import FakeMethod
 from lp.testing.fixture import ZopeUtilityFixture
 from lp.testing.layers import (
@@ -2029,3 +2030,50 @@
         with StormStatementRecorder() as recorder:
             self.webservice.get(url)
         self.assertThat(recorder, HasQueryCount(Equals(15)))
+
+    def test_builds_query_count(self):
+        # The query count of Snap.builds is constant in the number of
+        # builds, even if they have store upload jobs.
+        self.pushConfig(
+            "snappy", store_url="http://sca.example/";,
+            store_upload_url="http://updown.example/";)
+        with admin_logged_in():
+            snappyseries = self.factory.makeSnappySeries()
+        distroseries = self.factory.makeDistroSeries(
+            distribution=getUtility(IDistributionSet)['ubuntu'],
+            registrant=self.person)
+        processor = self.factory.makeProcessor(supports_virtualized=True)
+        distroarchseries = self.makeBuildableDistroArchSeries(
+            distroseries=distroseries, processor=processor, owner=self.person)
+        with person_logged_in(self.person):
+            snap = self.factory.makeSnap(
+                registrant=self.person, owner=self.person,
+                distroseries=distroseries, processors=[processor])
+            snap.store_series = snappyseries
+            snap.store_name = self.factory.getUniqueUnicode()
+            snap.store_upload = True
+            snap.store_secrets = {"root": Macaroon().serialize()}
+        builds_url = "%s/builds" % api_url(snap)
+        logout()
+
+        def make_build():
+            with person_logged_in(self.person):
+                build = snap.requestBuild(
+                    self.person, distroseries.main_archive, distroarchseries,
+                    PackagePublishingPocket.PROPOSED)
+                with dbuser(config.builddmaster.dbuser):
+                    build.updateStatus(
+                        BuildStatus.BUILDING, date_started=snap.date_created)
+                    build.updateStatus(
+                        BuildStatus.FULLYBUILT,
+                        date_finished=(
+                            snap.date_created + timedelta(minutes=10)))
+                return build
+
+        def get_builds():
+            response = self.webservice.get(builds_url)
+            self.assertEqual(200, response.status)
+            return response
+
+        recorder1, recorder2 = record_two_runs(get_builds, make_build, 2)
+        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2017-01-27 12:44:41 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2017-02-27 18:59:10 +0000
@@ -43,6 +43,7 @@
     CannotScheduleStoreUpload,
     ISnapBuild,
     ISnapBuildSet,
+    SnapBuildStoreUploadStatus,
     )
 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 from lp.soyuz.enums import ArchivePurpose
@@ -361,6 +362,44 @@
         self.build.snap.store_name = self.factory.getUniqueUnicode()
         self.build.snap.store_secrets = {"root": Macaroon().serialize()}
 
+    def test_store_upload_status_unscheduled(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        self.assertEqual(
+            SnapBuildStoreUploadStatus.UNSCHEDULED, build.store_upload_status)
+
+    def test_store_upload_status_pending(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        getUtility(ISnapStoreUploadJobSource).create(build)
+        self.assertEqual(
+            SnapBuildStoreUploadStatus.PENDING, build.store_upload_status)
+
+    def test_store_upload_status_uploaded(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.COMPLETED
+        self.assertEqual(
+            SnapBuildStoreUploadStatus.UPLOADED, build.store_upload_status)
+
+    def test_store_upload_status_failed_to_upload(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.FAILED
+        self.assertEqual(
+            SnapBuildStoreUploadStatus.FAILEDTOUPLOAD,
+            build.store_upload_status)
+
+    def test_store_upload_status_failed_to_release(self):
+        build = self.factory.makeSnapBuild(status=BuildStatus.FULLYBUILT)
+        job = getUtility(ISnapStoreUploadJobSource).create(build)
+        naked_job = removeSecurityProxy(job)
+        naked_job.job._status = JobStatus.FAILED
+        naked_job.store_url = "http://sca.example/dev/click-apps/1/rev/1/";
+        self.assertEqual(
+            SnapBuildStoreUploadStatus.FAILEDTORELEASE,
+            build.store_upload_status)
+
     def test_scheduleStoreUpload(self):
         # A build not previously uploaded to the store can be uploaded
         # manually.


Follow ups