launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-export-request-auto-builds into lp:launchpad.

Commit message:
Break out the per-snap part of SnapSet.makeAutoBuilds into Snap.requestAutoBuilds, and export it.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-export-request-auto-builds/+merge/300367

Break out the per-snap part of SnapSet.makeAutoBuilds into Snap.requestAutoBuilds, and export it.

William requested this last week, and it seems reasonable; one might wish to trigger a build with the usual parameters for an automatic build even when it happens that Launchpad can't detect that the package is stale.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-export-request-auto-builds into lp:launchpad.

=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py	2016-07-15 17:11:09 +0000
+++ lib/lp/snappy/interfaces/snap.py	2016-07-18 17:19:30 +0000
@@ -8,8 +8,10 @@
 __all__ = [
     'BadSnapSearchContext',
     'CannotModifySnapProcessor',
+    'CannotRequestAutoBuilds',
     'DuplicateSnapName',
     'ISnap',
+    'ISnapEdit',
     'ISnapSet',
     'ISnapView',
     'NoSourceForSnap',
@@ -41,6 +43,7 @@
     exported,
     operation_for_version,
     operation_parameters,
+    operation_returns_collection_of,
     operation_returns_entry,
     REQUEST_USER,
     )
@@ -68,8 +71,8 @@
     )
 
 from lp import _
+from lp.app.errors import NameLookupFailed
 from lp.app.interfaces.launchpad import IPrivacy
-from lp.app.errors import NameLookupFailed
 from lp.app.validators.name import name_validator
 from lp.buildmaster.interfaces.processor import IProcessor
 from lp.code.interfaces.branch import IBranch
@@ -203,6 +206,16 @@
             self._fmt % {'processor': processor.name})
 
 
+@error_status(httplib.BAD_REQUEST)
+class CannotRequestAutoBuilds(Exception):
+    """Snap package is not configured for automatic builds."""
+
+    def __init__(self, field):
+        super(CannotRequestAutoBuilds, self).__init__(
+            "This snap package cannot have automatic builds created for it "
+            "because %s is not set." % field)
+
+
 class ISnapView(Interface):
     """`ISnap` attributes that require launchpad.View permission."""
 
@@ -247,7 +260,7 @@
         archive=Reference(schema=IArchive),
         distro_arch_series=Reference(schema=IDistroArchSeries),
         pocket=Choice(vocabulary=PackagePublishingPocket))
-    # Really ISnapBuild, patched in _schema_circular_imports.py.
+    # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
     @export_factory_operation(Interface, [])
     @operation_for_version("devel")
     def requestBuild(requester, archive, distro_arch_series, pocket):
@@ -280,7 +293,7 @@
         description=_(
             "All builds of this snap package, sorted in descending order "
             "of finishing (or starting if not completed successfully)."),
-        # Really ISnapBuild, patched in _schema_circular_imports.py.
+        # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
         value_type=Reference(schema=Interface), readonly=True)))
 
     completed_builds = exported(doNotSnapshot(CollectionField(
@@ -288,7 +301,7 @@
         description=_(
             "Completed builds of this snap package, sorted in descending "
             "order of finishing."),
-        # Really ISnapBuild, patched in _schema_circular_imports.py.
+        # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
         value_type=Reference(schema=Interface), readonly=True)))
 
     pending_builds = exported(doNotSnapshot(CollectionField(
@@ -296,13 +309,26 @@
         description=_(
             "Pending builds of this snap package, sorted in descending "
             "order of creation."),
-        # Really ISnapBuild, patched in _schema_circular_imports.py.
+        # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
         value_type=Reference(schema=Interface), readonly=True)))
 
 
 class ISnapEdit(IWebhookTarget):
     """`ISnap` methods that require launchpad.Edit permission."""
 
+    # Really ISnapBuild, patched in lp.snappy.interfaces.webservice.
+    @operation_returns_collection_of(Interface)
+    @export_write_operation()
+    @operation_for_version("devel")
+    def requestAutoBuilds(logger=None):
+        """Create and return automatic builds for this snap package.
+
+        :param logger: An optional logger.
+        :raises CannotRequestAutoBuilds: if no auto_build_archive or
+            auto_build_pocket is set.
+        :return: A sequence of `ISnapBuild` instances.
+        """
+
     @export_destructor_operation()
     @operation_for_version("devel")
     def destroySelf():
@@ -468,16 +494,21 @@
     export_as_webservice_collection(ISnap)
 
     @call_with(registrant=REQUEST_USER)
+    @operation_parameters(
+        processors=List(
+            value_type=Reference(schema=IProcessor), required=False))
     @export_factory_operation(
         ISnap, [
             "owner", "distro_series", "name", "description", "branch",
-            "git_ref", "private"])
+            "git_ref", "auto_build", "auto_build_archive", "auto_build_pocket",
+            "private"])
     @operation_for_version("devel")
     def new(registrant, owner, distro_series, name, description=None,
-            branch=None, git_ref=None, require_virtualized=True,
-            processors=None, date_created=None, private=False,
-            store_upload=False, store_series=None, store_name=None,
-            store_secrets=None):
+            branch=None, git_ref=None, auto_build=False,
+            auto_build_archive=None, auto_build_pocket=None,
+            require_virtualized=True, processors=None, date_created=None,
+            private=False, store_upload=False, store_series=None,
+            store_name=None, store_secrets=None):
         """Create an `ISnap`."""
 
     def exists(owner, name):

=== modified file 'lib/lp/snappy/interfaces/webservice.py'
--- lib/lp/snappy/interfaces/webservice.py	2016-05-05 20:02:01 +0000
+++ lib/lp/snappy/interfaces/webservice.py	2016-07-18 17:19:30 +0000
@@ -19,11 +19,13 @@
 
 from lp.services.webservice.apihelpers import (
     patch_collection_property,
+    patch_collection_return_type,
     patch_entry_return_type,
     patch_reference_property,
     )
 from lp.snappy.interfaces.snap import (
     ISnap,
+    ISnapEdit,
     ISnapSet,
     ISnapView,
     )
@@ -45,3 +47,6 @@
 patch_collection_property(ISnapView, 'builds', ISnapBuild)
 patch_collection_property(ISnapView, 'completed_builds', ISnapBuild)
 patch_collection_property(ISnapView, 'pending_builds', ISnapBuild)
+
+# ISnapEdit
+patch_collection_return_type(ISnapEdit, 'requestAutoBuilds', ISnapBuild)

=== 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-18 17:19:30 +0000
@@ -10,6 +10,7 @@
     datetime,
     timedelta,
     )
+
 import pytz
 from storm.expr import (
     And,
@@ -96,6 +97,7 @@
 from lp.snappy.interfaces.snap import (
     BadSnapSearchContext,
     CannotModifySnapProcessor,
+    CannotRequestAutoBuilds,
     DuplicateSnapName,
     ISnap,
     ISnapSet,
@@ -380,6 +382,40 @@
         result.order_by(order_by)
         return result
 
+    def requestAutoBuilds(self, logger=None):
+        """See `ISnapSet`."""
+        builds = []
+        if self.auto_build_archive is None:
+            raise CannotRequestAutoBuilds("auto_build_archive")
+        if self.auto_build_pocket is None:
+            raise CannotRequestAutoBuilds("auto_build_pocket")
+        self.is_stale = False
+        if logger is not None:
+            logger.debug(
+                "Scheduling builds of snap package %s/%s",
+                self.owner.name, self.name)
+        for arch in self.getAllowedArchitectures():
+            try:
+                build = self.requestBuild(
+                    self.owner, self.auto_build_archive, arch,
+                    self.auto_build_pocket)
+                if logger is not None:
+                    logger.debug(
+                        " - %s/%s/%s: Build requested.",
+                        self.owner.name, self.name, arch.architecturetag)
+                builds.append(build)
+            except SnapBuildAlreadyPending as e:
+                if logger is not None:
+                    logger.warning(
+                        " - %s/%s/%s: %s",
+                        self.owner.name, self.name, arch.architecturetag, e)
+            except Exception as e:
+                if logger is not None:
+                    logger.exception(
+                        " - %s/%s/%s: %s",
+                        self.owner.name, self.name, arch.architecturetag, e)
+        return builds
+
     def getBuildSummariesForSnapBuildIds(self, snap_build_ids):
         """See `ISnap`."""
         result = {}
@@ -720,33 +756,7 @@
         snaps = cls._findStaleSnaps()
         builds = []
         for snap in snaps:
-            snap.is_stale = False
-            if logger is not None:
-                logger.debug(
-                    "Scheduling builds of snap package %s/%s",
-                    snap.owner.name, snap.name)
-            for arch in snap.getAllowedArchitectures():
-                try:
-                    build = snap.requestBuild(
-                        snap.owner, snap.auto_build_archive, arch,
-                        snap.auto_build_pocket)
-                    if logger is not None:
-                        logger.debug(
-                            " - %s/%s/%s: Build requested.",
-                            snap.owner.name, snap.name, arch.architecturetag)
-                    builds.append(build)
-                except SnapBuildAlreadyPending as e:
-                    if logger is not None:
-                        logger.warning(
-                            " - %s/%s/%s: %s",
-                            snap.owner.name, snap.name, arch.architecturetag,
-                            e)
-                except Exception as e:
-                    if logger is not None:
-                        logger.exception(
-                            " - %s/%s/%s: %s",
-                            snap.owner.name, snap.name, arch.architecturetag,
-                            e)
+            builds.extend(snap.requestAutoBuilds(logger=logger))
         return builds
 
     def detachFromBranch(self, branch):

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2016-07-01 11:53:31 +0000
+++ lib/lp/snappy/tests/test_snap.py	2016-07-18 17:19:30 +0000
@@ -16,6 +16,7 @@
 from storm.locals import Store
 from testtools.matchers import (
     Equals,
+    MatchesSetwise,
     MatchesStructure,
     )
 import transaction
@@ -305,6 +306,30 @@
             snap.owner, snap.distro_series.main_archive, distroarchseries,
             PackagePublishingPocket.UPDATES)
 
+    def test_requestAutoBuilds(self):
+        # requestAutoBuilds creates new builds for all configured
+        # architectures with appropriate parameters.
+        distroseries = self.factory.makeDistroSeries()
+        dases = []
+        for _ in range(3):
+            processor = self.factory.makeProcessor(supports_virtualized=True)
+            dases.append(self.makeBuildableDistroArchSeries(
+                distroseries=distroseries, processor=processor))
+        archive = self.factory.makeArchive()
+        snap = self.factory.makeSnap(
+            distroseries=distroseries,
+            processors=[das.processor for das in dases[:2]],
+            auto_build_archive=archive,
+            auto_build_pocket=PackagePublishingPocket.PROPOSED)
+        with person_logged_in(snap.owner):
+            builds = snap.requestAutoBuilds()
+        self.assertThat(builds, MatchesSetwise(
+            *(MatchesStructure.byEquality(
+                requester=snap.owner, snap=snap, archive=archive,
+                distro_arch_series=das,
+                pocket=PackagePublishingPocket.PROPOSED)
+              for das in dases[:2])))
+
     def test_getBuilds(self):
         # Test the various getBuilds methods.
         snap = self.factory.makeSnap()
@@ -1063,7 +1088,8 @@
 
     def makeSnap(self, owner=None, distroseries=None, branch=None,
                  git_ref=None, processors=None, webservice=None,
-                 private=False):
+                 private=False, auto_build_archive=None,
+                 auto_build_pocket=None):
         if owner is None:
             owner = self.person
         if distroseries is None:
@@ -1083,6 +1109,10 @@
         if processors is not None:
             kwargs["processors"] = [
                 api_url(processor) for processor in processors]
+        if auto_build_archive is not None:
+            kwargs["auto_build_archive"] = api_url(auto_build_archive)
+        if auto_build_pocket is not None:
+            kwargs["auto_build_pocket"] = auto_build_pocket.title
         logout()
         response = webservice.named_post(
             "/+snaps", "new", owner=owner_url, distro_series=distroseries_url,
@@ -1425,6 +1455,50 @@
             "if the snap package owner and the archive owner are equal.",
             response.body)
 
+    def test_requestAutoBuilds(self):
+        # requestAutoBuilds can be performed over the webservice.
+        distroseries = self.factory.makeDistroSeries()
+        dases = []
+        for _ in range(3):
+            processor = self.factory.makeProcessor(supports_virtualized=True)
+            dases.append(self.makeBuildableDistroArchSeries(
+                distroseries=distroseries, processor=processor))
+        archive = self.factory.makeArchive()
+        snap = self.makeSnap(
+            distroseries=distroseries,
+            processors=[das.processor for das in dases[:2]],
+            auto_build_archive=archive,
+            auto_build_pocket=PackagePublishingPocket.PROPOSED)
+        response = self.webservice.named_post(
+            snap["self_link"], "requestAutoBuilds")
+        self.assertEqual(200, response.status)
+        builds = response.jsonBody()
+        self.assertContentEqual(
+            [self.getURL(das) for das in dases[:2]],
+            [build["distro_arch_series_link"] for build in builds])
+
+    def test_requestAutoBuilds_requires_auto_build_archive(self):
+        # requestAutoBuilds fails if auto_build_archive is not set.
+        snap = self.makeSnap()
+        response = self.webservice.named_post(
+            snap["self_link"], "requestAutoBuilds")
+        self.assertEqual(400, response.status)
+        self.assertEqual(
+            "This snap package cannot have automatic builds created for it "
+            "because auto_build_archive is not set.",
+            response.body)
+
+    def test_requestAutoBuilds_requires_auto_build_pocket(self):
+        # requestAutoBuilds fails if auto_build_pocket is not set.
+        snap = self.makeSnap(auto_build_archive=self.factory.makeArchive())
+        response = self.webservice.named_post(
+            snap["self_link"], "requestAutoBuilds")
+        self.assertEqual(400, response.status)
+        self.assertEqual(
+            "This snap package cannot have automatic builds created for it "
+            "because auto_build_pocket is not set.",
+            response.body)
+
     def test_getBuilds(self):
         # The builds, completed_builds, and pending_builds properties are as
         # expected.