launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-request-builds into lp:launchpad with lp:~cjwatson/launchpad/snap-request-builds-job as a prerequisite.

Commit message:
Add Snap.requestBuilds and associated webservice workflow.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1770400 in Launchpad itself: "Support snapcraft architectures keyword"
  https://bugs.launchpad.net/launchpad/+bug/1770400

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

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2018-02-20 11:17:28 +0000
+++ lib/lp/security.py	2018-06-15 16:59:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Security policies for using content objects."""
@@ -192,7 +192,10 @@
     ILanguage,
     ILanguageSet,
     )
-from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snap import (
+    ISnap,
+    ISnapBuildRequest,
+    )
 from lp.snappy.interfaces.snapbuild import ISnapBuild
 from lp.snappy.interfaces.snappyseries import (
     ISnappySeries,
@@ -3222,6 +3225,15 @@
             and EditSnap(self.obj).checkAuthenticated(user))
 
 
+class ViewSnapBuildRequest(DelegatedAuthorization):
+    permission = 'launchpad.View'
+    usedfor = ISnapBuildRequest
+
+    def __init__(self, obj):
+        super(ViewSnapBuildRequest, self).__init__(
+            obj, obj.snap, 'launchpad.View')
+
+
 class ViewSnapBuild(DelegatedAuthorization):
     permission = 'launchpad.View'
     usedfor = ISnapBuild

=== modified file 'lib/lp/snappy/browser/configure.zcml'
--- lib/lp/snappy/browser/configure.zcml	2016-05-19 02:02:39 +0000
+++ lib/lp/snappy/browser/configure.zcml	2018-06-15 16:59:33 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -88,6 +88,10 @@
             path_expression="string:+snaps"
             parent_utility="lp.services.webapp.interfaces.ILaunchpadRoot" />
         <browser:url
+            for="lp.snappy.interfaces.snap.ISnapBuildRequest"
+            path_expression="string:+build-request/${id}"
+            attribute_to_parent="snap" />
+        <browser:url
             for="lp.snappy.interfaces.snapbuild.ISnapBuild"
             path_expression="string:+build/${id}"
             attribute_to_parent="snap" />

=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/browser/snap.py	2018-06-15 16:59:33 +0000
@@ -104,6 +104,14 @@
 class SnapNavigation(WebhookTargetNavigationMixin, Navigation):
     usedfor = ISnap
 
+    @stepthrough('+build-request')
+    def traverse_build_request(self, name):
+        try:
+            job_id = int(name)
+        except ValueError:
+            return None
+        return self.context.getBuildRequest(job_id)
+
     @stepthrough('+build')
     def traverse_build(self, name):
         build = get_build_by_id_str(ISnapBuildSet, name)

=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/configure.zcml	2018-06-15 16:59:33 +0000
@@ -42,6 +42,13 @@
         <allow interface="lp.snappy.interfaces.snap.ISnapSet" />
     </securedutility>
 
+    <!-- SnapBuildRequest -->
+    <class class="lp.snappy.model.snap.SnapBuildRequest">
+        <require
+            permission="launchpad.View"
+            interface="lp.snappy.interfaces.snap.ISnapBuildRequest" />
+    </class>
+
     <!-- SnapBuild -->
     <class class="lp.snappy.model.snapbuild.SnapBuild">
         <require

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/interfaces/snap.py	2018-06-15 16:59:33 +0000
@@ -15,6 +15,7 @@
     'CannotRequestAutoBuilds',
     'DuplicateSnapName',
     'ISnap',
+    'ISnapBuildRequest',
     'ISnapEdit',
     'ISnapSet',
     'ISnapView',
@@ -27,6 +28,7 @@
     'SnapBuildAlreadyPending',
     'SnapBuildArchiveOwnerMismatch',
     'SnapBuildDisallowedArchitecture',
+    'SnapBuildRequestStatus',
     'SnapNotOwner',
     'SnapPrivacyMismatch',
     'SnapPrivateFeatureDisabled',
@@ -34,6 +36,10 @@
 
 import httplib
 
+from lazr.enum import (
+    EnumeratedType,
+    Item,
+    )
 from lazr.lifecycle.snapshot import doNotSnapshot
 from lazr.restful.declarations import (
     call_with,
@@ -247,6 +253,57 @@
     """Launchpad cannot parse this snap package's snapcraft.yaml."""
 
 
+class SnapBuildRequestStatus(EnumeratedType):
+    """The status of a request to build a snap package."""
+
+    PENDING = Item("""
+        Pending
+
+        This snap build request is pending.
+        """)
+
+    FAILED = Item("""
+        Failed
+
+        This snap build request failed.
+        """)
+
+    COMPLETED = Item("""
+        Completed
+
+        This snap build request completed successfully.
+        """)
+
+
+class ISnapBuildRequest(Interface):
+    """A request to build a snap package."""
+
+    # XXX cjwatson 2018-06-14 bug=760849: "beta" is a lie to get WADL
+    # generation working.  Individual attributes must set their version to
+    # "devel".
+    export_as_webservice_entry(as_of="beta")
+
+    id = Int(title=_("ID"), required=True, readonly=True)
+
+    snap = exported(Reference(
+        # Really ISnap, patched in lp.snappy.interfaces.webservice.
+        Interface,
+        title=_("Snap package"), required=True, readonly=True))
+
+    status = exported(Choice(
+        title=_("Status"), vocabulary=SnapBuildRequestStatus,
+        required=True, readonly=True))
+
+    error_message = exported(TextLine(
+        title=_("Error message"), required=True, readonly=True))
+
+    builds = exported(CollectionField(
+        title=_("Builds produced by this request"),
+        # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
+        value_type=Reference(schema=Interface),
+        required=True, readonly=True))
+
+
 class ISnapView(Interface):
     """`ISnap` attributes that require launchpad.View permission."""
 
@@ -316,6 +373,34 @@
         :return: `ISnapBuild`.
         """
 
+    @call_with(requester=REQUEST_USER)
+    @operation_parameters(
+        archive=Reference(schema=IArchive),
+        pocket=Choice(vocabulary=PackagePublishingPocket),
+        channels=Dict(
+            title=_("Source snap channels to use for this build."),
+            description=_(
+                "A dictionary mapping snap names to channels to use for this "
+                "build.  Currently only 'core' and 'snapcraft' keys are "
+                "supported."),
+            key_type=TextLine(), required=False))
+    @export_factory_operation(ISnapBuildRequest, [])
+    @operation_for_version("devel")
+    def requestBuilds(requester, archive, pocket, channels=None):
+        """Request that the snap package be built for relevant architectures.
+
+        This is an asynchronous operation, and returns a job ID which can be
+        passed to `snap.getRequestedBuilds`; once the operation has
+        finished, that method will return the resulting builds.
+
+        :param requester: The person requesting the builds.
+        :param archive: The IArchive to associate the builds with.
+        :param pocket: The pocket that should be targeted.
+        :param channels: A dictionary mapping snap names to channels to use
+            for these builds.
+        :return: An `ISnapBuildRequest`.
+        """
+
     def requestBuildsFromJob(requester, archive, pocket, channels=None,
                              logger=None):
         """Synchronous part of `Snap.requestBuilds`.
@@ -331,6 +416,13 @@
         :return: A sequence of `ISnapBuild` instances.
         """
 
+    def getBuildRequest(job_id):
+        """Get an asynchronous build request by ID.
+
+        :param job_id: The ID of the build request.
+        :return: `ISnapBuildRequest`.
+        """
+
     @operation_parameters(
         snap_build_ids=List(
             title=_("A list of snap build ids."),

=== modified file 'lib/lp/snappy/interfaces/snapjob.py'
--- lib/lp/snappy/interfaces/snapjob.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/interfaces/snapjob.py	2018-06-15 16:59:33 +0000
@@ -95,3 +95,11 @@
         :param channels: A dictionary mapping snap names to channels to use
             for these builds.
         """
+
+    def getBySnapAndID(snap, job_id):
+        """Get a job by snap and job ID.
+
+        :return: The `SnapRequestBuildsJob` with the specified snap and ID.
+        :raises: `NotFoundError` if there is no job with the specified snap
+            and ID, or its `job_type` is not `SnapJobType.REQUEST_BUILDS`.
+        """

=== modified file 'lib/lp/snappy/interfaces/webservice.py'
--- lib/lp/snappy/interfaces/webservice.py	2016-07-18 17:16:30 +0000
+++ lib/lp/snappy/interfaces/webservice.py	2018-06-15 16:59:33 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2015-2018 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """All the interfaces that are exposed through the webservice.
@@ -12,6 +12,7 @@
 __all__ = [
     'ISnap',
     'ISnapBuild',
+    'ISnapBuildRequest',
     'ISnappySeries',
     'ISnappySeriesSet',
     'ISnapSet',
@@ -25,6 +26,7 @@
     )
 from lp.snappy.interfaces.snap import (
     ISnap,
+    ISnapBuildRequest,
     ISnapEdit,
     ISnapSet,
     ISnapView,
@@ -42,6 +44,10 @@
 # ISnapFile
 patch_reference_property(ISnapFile, 'snapbuild', ISnapBuild)
 
+# ISnapBuildRequest
+patch_reference_property(ISnapBuildRequest, 'snap', ISnap)
+patch_collection_property(ISnapBuildRequest, 'builds', ISnapBuild)
+
 # ISnapView
 patch_entry_return_type(ISnapView, 'requestBuild', ISnapBuild)
 patch_collection_property(ISnapView, 'builds', ISnapBuild)

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/model/snap.py	2018-06-15 16:59:33 +0000
@@ -110,6 +110,7 @@
     NullsLast,
     )
 from lp.services.features import getFeatureFlag
+from lp.services.job.interfaces.job import JobStatus
 from lp.services.librarian.model import (
     LibraryFileAlias,
     LibraryFileContent,
@@ -129,6 +130,7 @@
     CannotRequestAutoBuilds,
     DuplicateSnapName,
     ISnap,
+    ISnapBuildRequest,
     ISnapSet,
     NoSourceForSnap,
     NoSuchSnap,
@@ -137,11 +139,13 @@
     SnapBuildAlreadyPending,
     SnapBuildArchiveOwnerMismatch,
     SnapBuildDisallowedArchitecture,
+    SnapBuildRequestStatus,
     SnapNotOwner,
     SnapPrivacyMismatch,
     SnapPrivateFeatureDisabled,
     )
 from lp.snappy.interfaces.snapbuild import ISnapBuildSet
+from lp.snappy.interfaces.snapjob import ISnapRequestBuildsJobSource
 from lp.snappy.interfaces.snappyseries import ISnappyDistroSeriesSet
 from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
 from lp.snappy.model.snapbuild import SnapBuild
@@ -162,6 +166,43 @@
     removeSecurityProxy(snap).date_last_modified = UTC_NOW
 
 
+@implementer(ISnapBuildRequest)
+class SnapBuildRequest:
+    """See `ISnapBuildRequest`.
+
+    This is not directly backed by a database table; instead, it is a
+    webservice-friendly view of an asynchronous build request.
+    """
+
+    def __init__(self, snap, id):
+        self._job = getUtility(ISnapRequestBuildsJobSource).getBySnapAndID(
+            snap, id)
+        self.snap = snap
+        self.id = id
+
+    @property
+    def status(self):
+        """See `ISnapBuildRequest`."""
+        status_map = {
+            JobStatus.WAITING: SnapBuildRequestStatus.PENDING,
+            JobStatus.RUNNING: SnapBuildRequestStatus.PENDING,
+            JobStatus.COMPLETED: SnapBuildRequestStatus.COMPLETED,
+            JobStatus.FAILED: SnapBuildRequestStatus.FAILED,
+            JobStatus.SUSPENDED: SnapBuildRequestStatus.PENDING,
+            }
+        return status_map[self._job.job.status]
+
+    @property
+    def error_message(self):
+        """See `ISnapBuildRequest`."""
+        return self._job.error_message
+
+    @property
+    def builds(self):
+        """See `ISnapBuildRequest`."""
+        return self._job.builds
+
+
 @implementer(ISnap, IHasOwner)
 class Snap(Storm, WebhookTargetMixin):
     """See `ISnap`."""
@@ -502,6 +543,13 @@
         build.queueBuild()
         return build
 
+    def requestBuilds(self, requester, archive, pocket, channels=None):
+        """See `ISnap`."""
+        self._checkRequestBuild(requester, archive)
+        job = getUtility(ISnapRequestBuildsJobSource).create(
+            self, requester, archive, pocket, channels)
+        return self.getBuildRequest(job.job_id)
+
     def requestBuildsFromJob(self, requester, archive, pocket, channels=None,
                              logger=None):
         """See `ISnap`."""
@@ -572,6 +620,10 @@
                         self.owner.name, self.name, arch.architecturetag, e)
         return builds
 
+    def getBuildRequest(self, job_id):
+        """See `ISnap`."""
+        return SnapBuildRequest(self, job_id)
+
     def _getBuilds(self, filter_term, order_by):
         """The actual query to get the builds."""
         query_args = [

=== modified file 'lib/lp/snappy/model/snapjob.py'
--- lib/lp/snappy/model/snapjob.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/model/snapjob.py	2018-06-15 16:59:33 +0000
@@ -187,6 +187,20 @@
         job.celeryRunOnCommit()
         return job
 
+    @classmethod
+    def getBySnapAndID(cls, snap, job_id):
+        """See `ISnapRequestBuildsJobSource`."""
+        snap_job = IStore(SnapJob).find(
+            SnapJob,
+            SnapJob.job_id == job_id,
+            SnapJob.snap == snap,
+            SnapJob.job_type == cls.class_job_type).one()
+        if snap_job is None:
+            raise NotFoundError(
+                "No REQUEST_BUILDS job with ID %d found for %r" %
+                (job_id, snap))
+        return cls(snap_job)
+
     def getOperationDescription(self):
         return "requesting builds of %s" % self.snap.name
 

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2018-06-15 16:59:32 +0000
+++ lib/lp/snappy/tests/test_snap.py	2018-06-15 16:59:33 +0000
@@ -23,6 +23,7 @@
 from storm.exceptions import LostObjectError
 from storm.locals import Store
 from testtools.matchers import (
+    AfterPreprocessing,
     ContainsDict,
     Equals,
     Is,
@@ -69,6 +70,8 @@
     FeatureFixture,
     MemoryFeatureFixture,
     )
+from lp.services.job.interfaces.job import JobStatus
+from lp.services.job.runner import JobRunner
 from lp.services.log.logger import BufferLogger
 from lp.services.propertycache import clear_property_cache
 from lp.services.webapp.interfaces import OAuthPermission
@@ -84,6 +87,7 @@
     SNAP_TESTING_FLAGS,
     SnapBuildAlreadyPending,
     SnapBuildDisallowedArchitecture,
+    SnapBuildRequestStatus,
     SnapPrivacyMismatch,
     SnapPrivateFeatureDisabled,
     )
@@ -363,6 +367,30 @@
             snap.owner, snap.distro_series.main_archive, distroarchseries,
             PackagePublishingPocket.UPDATES)
 
+    def test_requestBuilds(self):
+        # requestBuilds schedules a job and returns a corresponding
+        # SnapBuildRequest.
+        snap = self.factory.makeSnap()
+        with person_logged_in(snap.owner.teamowner):
+            request = snap.requestBuilds(
+                snap.owner.teamowner, snap.distro_series.main_archive,
+                PackagePublishingPocket.UPDATES,
+                channels={"snapcraft": "edge"})
+        self.assertThat(request, MatchesStructure(
+            snap=Equals(snap),
+            status=Equals(SnapBuildRequestStatus.PENDING),
+            error_message=Is(None),
+            builds=AfterPreprocessing(set, MatchesSetwise())))
+        [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
+        self.assertThat(job, MatchesStructure(
+            job_id=Equals(request.id),
+            job=MatchesStructure.byEquality(status=JobStatus.WAITING),
+            snap=Equals(snap),
+            requester=Equals(snap.owner.teamowner),
+            archive=Equals(snap.distro_series.main_archive),
+            pocket=Equals(PackagePublishingPocket.UPDATES),
+            channels=Equals({"snapcraft": "edge"})))
+
     def makeRequestBuildsJob(self, arch_tags):
         distro = self.factory.makeDistribution()
         distroseries = self.factory.makeDistroSeries(distribution=distro)
@@ -2408,6 +2436,128 @@
             "if the snap package owner and the archive owner are equal.",
             response.body)
 
+    def test_requestBuilds(self):
+        # Requests for builds for all relevant architectures can be
+        # performed over the webservice, and the returned entry indicates
+        # the status of the asynchronous job.
+        distroseries = self.factory.makeDistroSeries(registrant=self.person)
+        processors = [
+            self.factory.makeProcessor(supports_virtualized=True)
+            for _ in range(3)]
+        for processor in processors:
+            self.makeBuildableDistroArchSeries(
+                distroseries=distroseries, architecturetag=processor.name,
+                processor=processor, owner=self.person)
+        archive_url = api_url(distroseries.main_archive)
+        [git_ref] = self.factory.makeGitRefs()
+        snap = self.makeSnap(
+            git_ref=git_ref, distroseries=distroseries, processors=processors)
+        response = self.webservice.named_post(
+            snap["self_link"], "requestBuilds", archive=archive_url,
+            pocket="Updates", channels={"snapcraft": "edge"})
+        self.assertEqual(201, response.status)
+        build_request_url = response.getHeader("Location")
+        build_request = self.webservice.get(build_request_url).jsonBody()
+        self.assertThat(build_request, ContainsDict({
+            "snap_link": Equals(snap["self_link"]),
+            "status": Equals("Pending"),
+            "error_message": Is(None),
+            "builds_collection_link": Equals(build_request_url + "/builds"),
+            }))
+        self.assertEqual([], self.getCollectionLinks(build_request, "builds"))
+        with person_logged_in(self.person):
+            snapcraft_yaml = "architectures:\n"
+            for processor in processors:
+                snapcraft_yaml += "  - build-on: %s\n" % processor.name
+            self.useFixture(GitHostingFixture(blob=snapcraft_yaml))
+            [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
+            with dbuser(config.ISnapRequestBuildsJobSource.dbuser):
+                JobRunner([job]).runAll()
+        build_request = self.webservice.get(
+            build_request["self_link"]).jsonBody()
+        self.assertThat(build_request, ContainsDict({
+            "snap_link": Equals(snap["self_link"]),
+            "status": Equals("Completed"),
+            "error_message": Is(None),
+            "builds_collection_link": Equals(build_request_url + "/builds"),
+            }))
+        builds = self.webservice.get(
+            build_request["builds_collection_link"]).jsonBody()["entries"]
+        with person_logged_in(self.person):
+            self.assertThat(builds, MatchesSetwise(*(
+                ContainsDict({
+                    "snap_link": Equals(snap["self_link"]),
+                    "archive_link": Equals(
+                        self.getURL(distroseries.main_archive)),
+                    "arch_tag": Equals(processor.name),
+                    "pocket": Equals("Updates"),
+                    "channels": Equals({"snapcraft": "edge"}),
+                    })
+                for processor in processors)))
+
+    def test_requestBuilds_failure(self):
+        # If the asynchronous build request job fails, this is reflected in
+        # the build request entry.
+        distroseries = self.factory.makeDistroSeries(registrant=self.person)
+        processor = self.factory.makeProcessor(supports_virtualized=True)
+        self.makeBuildableDistroArchSeries(
+            distroseries=distroseries, architecturetag=processor.name,
+            processor=processor, owner=self.person)
+        archive_url = api_url(distroseries.main_archive)
+        [git_ref] = self.factory.makeGitRefs()
+        snap = self.makeSnap(
+            git_ref=git_ref, distroseries=distroseries,
+            processors=[processor])
+        response = self.webservice.named_post(
+            snap["self_link"], "requestBuilds", archive=archive_url,
+            pocket="Updates")
+        self.assertEqual(201, response.status)
+        build_request_url = response.getHeader("Location")
+        build_request = self.webservice.get(build_request_url).jsonBody()
+        self.assertThat(build_request, ContainsDict({
+            "snap_link": Equals(snap["self_link"]),
+            "status": Equals("Pending"),
+            "error_message": Is(None),
+            "builds_collection_link": Equals(build_request_url + "/builds"),
+            }))
+        self.assertEqual([], self.getCollectionLinks(build_request, "builds"))
+        with person_logged_in(self.person):
+            self.useFixture(GitHostingFixture()).getBlob.failure = Exception(
+                "Something went wrong")
+            [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
+            with dbuser(config.ISnapRequestBuildsJobSource.dbuser):
+                JobRunner([job]).runAll()
+        build_request = self.webservice.get(
+            build_request["self_link"]).jsonBody()
+        self.assertThat(build_request, ContainsDict({
+            "snap_link": Equals(snap["self_link"]),
+            "status": Equals("Failed"),
+            "error_message": Equals("Something went wrong"),
+            "builds_collection_link": Equals(build_request_url + "/builds"),
+            }))
+        self.assertEqual([], self.getCollectionLinks(build_request, "builds"))
+
+    def test_requestBuilds_not_owner(self):
+        # If the requester is not the owner or a member of the owner team,
+        # build requests are rejected.
+        other_team = self.factory.makeTeam(displayname="Other Team")
+        distroseries = self.factory.makeDistroSeries(registrant=self.person)
+        archive_url = api_url(distroseries.main_archive)
+        other_webservice = webservice_for_person(
+            other_team.teamowner, permission=OAuthPermission.WRITE_PUBLIC)
+        other_webservice.default_api_version = "devel"
+        login(ANONYMOUS)
+        snap = self.makeSnap(
+            owner=other_team, distroseries=distroseries,
+            webservice=other_webservice)
+        response = self.webservice.named_post(
+            snap["self_link"], "requestBuilds", archive=archive_url,
+            pocket="Updates")
+        self.assertEqual(401, response.status)
+        self.assertEqual(
+            "Test Person cannot create snap package builds owned by Other "
+            "Team.", response.body)
+
     def test_requestAutoBuilds(self):
         # requestAutoBuilds can be performed over the webservice.
         distroseries = self.factory.makeDistroSeries()