launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Flesh out Snap.requestBuild, Snap.*builds, and Snap.destroySelf methods.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1476405 in Launchpad itself: "Add support for building snaps"
  https://bugs.launchpad.net/launchpad/+bug/1476405

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-builds/+merge/265698

Flesh out Snap.requestBuild, Snap.*builds, and Snap.destroySelf methods.

Now that both Snap and SnapBuild are modelled, we can link them together.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-builds into lp:launchpad.

=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py	2015-07-23 16:23:50 +0000
+++ lib/lp/snappy/model/snap.py	2015-07-23 16:23:50 +0000
@@ -11,13 +11,18 @@
 from storm.locals import (
     Bool,
     DateTime,
+    Desc,
     Int,
+    Not,
     Reference,
+    Store,
     Storm,
     Unicode,
     )
+from zope.component import getUtility
 from zope.interface import implementer
 
+from lp.buildmaster.enums import BuildStatus
 from lp.registry.interfaces.role import IHasOwner
 from lp.services.database.constants import (
     DEFAULT,
@@ -27,16 +32,31 @@
     IMasterStore,
     IStore,
     )
+from lp.services.database.stormexpr import (
+    Greatest,
+    NullsLast,
+    )
 from lp.services.features import getFeatureFlag
+from lp.services.webapp.interfaces import ILaunchBag
 from lp.snappy.interfaces.snap import (
+    CannotDeleteSnap,
     DuplicateSnapName,
     ISnap,
     ISnapSet,
     SNAP_FEATURE_FLAG,
+    SnapBuildAlreadyPending,
+    SnapBuildArchiveOwnerMismatch,
     SnapFeatureDisabled,
     SnapNotOwner,
     NoSuchSnap,
     )
+from lp.snappy.interfaces.snapbuild import ISnapBuildSet
+from lp.snappy.model.snapbuild import SnapBuild
+from lp.soyuz.interfaces.archive import ArchiveDisabled
+from lp.soyuz.model.archive import (
+    Archive,
+    get_enabled_archive_filter,
+    )
 
 
 def snap_modified(snap, event):
@@ -106,31 +126,93 @@
 
     def requestBuild(self, requester, archive, distro_arch_series, pocket):
         """See `ISnap`."""
-        raise NotImplementedError
+        if not requester.inTeam(self.owner):
+            raise SnapNotOwner(
+                "%s cannot create snap package builds owned by %s." %
+                (requester.displayname, self.owner.displayname))
+        if not archive.enabled:
+            raise ArchiveDisabled(archive.displayname)
+        if archive.private and self.owner != archive.owner:
+            # See rationale in `SnapBuildArchiveOwnerMismatch` docstring.
+            raise SnapBuildArchiveOwnerMismatch()
+
+        pending = IStore(self).find(
+            SnapBuild,
+            SnapBuild.snap_id == self.id,
+            SnapBuild.archive_id == archive.id,
+            SnapBuild.distro_arch_series_id == distro_arch_series.id,
+            SnapBuild.pocket == pocket,
+            SnapBuild.status == BuildStatus.NEEDSBUILD)
+        if pending.any() is not None:
+            raise SnapBuildAlreadyPending
+
+        build = getUtility(ISnapBuildSet).new(
+            requester, self, archive, distro_arch_series, pocket)
+        build.queueBuild()
+        return build
+
+    def _getBuilds(self, filter_term, order_by):
+        """The actual query to get the builds."""
+        query_args = [
+            SnapBuild.snap == self,
+            SnapBuild.archive_id == Archive.id,
+            Archive._enabled == True,
+            get_enabled_archive_filter(
+                getUtility(ILaunchBag).user, include_public=True,
+                include_subscribed=True)
+            ]
+        if filter_term is not None:
+            query_args.append(filter_term)
+        result = Store.of(self).find(SnapBuild, *query_args)
+        result.order_by(order_by)
+        return result
 
     @property
     def builds(self):
         """See `ISnap`."""
-        return []
+        order_by = (
+            NullsLast(Desc(Greatest(
+                SnapBuild.date_started,
+                SnapBuild.date_finished))),
+            Desc(SnapBuild.date_created),
+            Desc(SnapBuild.id))
+        return self._getBuilds(None, order_by)
 
     @property
     def _pending_states(self):
         """All the build states we consider pending (non-final)."""
-        raise NotImplementedError
+        return [
+            BuildStatus.NEEDSBUILD,
+            BuildStatus.BUILDING,
+            BuildStatus.UPLOADING,
+            BuildStatus.CANCELLING,
+            ]
 
     @property
     def completed_builds(self):
         """See `ISnap`."""
-        return []
+        filter_term = (Not(SnapBuild.status.is_in(self._pending_states)))
+        order_by = (
+            NullsLast(Desc(Greatest(
+                SnapBuild.date_started,
+                SnapBuild.date_finished))),
+            Desc(SnapBuild.id))
+        return self._getBuilds(filter_term, order_by)
 
     @property
     def pending_builds(self):
         """See `ISnap`."""
-        return []
+        filter_term = (SnapBuild.status.is_in(self._pending_states))
+        # We want to order by date_created but this is the same as ordering
+        # by id (since id increases monotonically) and is less expensive.
+        order_by = Desc(SnapBuild.id)
+        return self._getBuilds(filter_term, order_by)
 
     def destroySelf(self):
         """See `ISnap`."""
-        raise NotImplementedError
+        if not self.builds.is_empty():
+            raise CannotDeleteSnap("Cannot delete a snap package with builds.")
+        IStore(Snap).remove(self)
 
 
 @implementer(ISnapSet)

=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py	2015-07-23 16:23:50 +0000
+++ lib/lp/snappy/tests/test_snap.py	2015-07-23 16:23:50 +0000
@@ -32,6 +32,7 @@
     SnapBuildAlreadyPending,
     SnapFeatureDisabled,
     )
+from lp.snappy.interfaces.snapbuild import ISnapBuild
 from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
@@ -83,6 +84,183 @@
             removeSecurityProxy(snap), snap, [ISnap["name"]]))
         self.assertSqlAttributeEqualsDate(snap, "date_last_modified", UTC_NOW)
 
+    def test_requestBuild(self):
+        # requestBuild creates a new SnapBuild.
+        snap = self.factory.makeSnap()
+        distroarchseries = self.factory.makeDistroArchSeries(
+            distroseries=snap.distro_series)
+        build = snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+        self.assertTrue(ISnapBuild.providedBy(build))
+        self.assertEqual(snap.owner, build.requester)
+        self.assertEqual(snap.distro_series.main_archive, build.archive)
+        self.assertEqual(distroarchseries, build.distro_arch_series)
+        self.assertEqual(PackagePublishingPocket.RELEASE, build.pocket)
+        self.assertEqual(BuildStatus.NEEDSBUILD, build.status)
+        store = Store.of(build)
+        store.flush()
+        build_queue = store.find(
+            BuildQueue,
+            BuildQueue._build_farm_job_id ==
+                removeSecurityProxy(build).build_farm_job_id).one()
+        self.assertProvides(build_queue, IBuildQueue)
+        self.assertEqual(
+            snap.distro_series.main_archive.require_virtualized,
+            build_queue.virtualized)
+        self.assertEqual(BuildQueueStatus.WAITING, build_queue.status)
+
+    def test_requestBuild_score(self):
+        # Build requests have a relatively low queue score (2505).
+        snap = self.factory.makeSnap()
+        distroarchseries = self.factory.makeDistroArchSeries(
+            distroseries=snap.distro_series)
+        build = snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+        queue_record = build.buildqueue_record
+        queue_record.score()
+        self.assertEqual(2505, queue_record.lastscore)
+
+    def test_requestBuild_relative_build_score(self):
+        # Offsets for archives are respected.
+        snap = self.factory.makeSnap()
+        archive = self.factory.makeArchive(owner=snap.owner)
+        removeSecurityProxy(archive).relative_build_score = 100
+        distroarchseries = self.factory.makeDistroArchSeries(
+            distroseries=snap.distro_series)
+        build = snap.requestBuild(
+            snap.owner, archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+        queue_record = build.buildqueue_record
+        queue_record.score()
+        self.assertEqual(2605, queue_record.lastscore)
+
+    def test_requestBuild_rejects_repeats(self):
+        # requestBuild refuses if there is already a pending build.
+        snap = self.factory.makeSnap()
+        distroarchseries = self.factory.makeDistroArchSeries(
+            distroseries=snap.distro_series)
+        old_build = snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+        self.assertRaises(
+            SnapBuildAlreadyPending, snap.requestBuild,
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+        # We can build for a different archive.
+        snap.requestBuild(
+            snap.owner, self.factory.makeArchive(owner=snap.owner),
+            distroarchseries, PackagePublishingPocket.RELEASE)
+        # We can build for a different distroarchseries.
+        snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive,
+            self.factory.makeDistroArchSeries(distroseries=snap.distro_series),
+            PackagePublishingPocket.RELEASE)
+        # Changing the status of the old build allows a new build.
+        old_build.updateStatus(BuildStatus.BUILDING)
+        old_build.updateStatus(BuildStatus.FULLYBUILT)
+        snap.requestBuild(
+            snap.owner, snap.distro_series.main_archive, distroarchseries,
+            PackagePublishingPocket.RELEASE)
+
+    def test_requestBuild_virtualization(self):
+        # New builds are virtualized if any of the processor, snap or
+        # archive require it.
+        for proc_nonvirt, snap_virt, archive_virt, build_virt in (
+                (True, False, False, False),
+                (True, False, True, True),
+                (True, True, False, True),
+                (True, True, True, True),
+                (False, False, False, True),
+                (False, False, True, True),
+                (False, True, False, True),
+                (False, True, True, True),
+                ):
+            distroarchseries = self.factory.makeDistroArchSeries(
+                processor=self.factory.makeProcessor(
+                    supports_nonvirtualized=proc_nonvirt))
+            snap = self.factory.makeSnap(
+                distroseries=distroarchseries.distroseries,
+                require_virtualized=snap_virt)
+            archive = self.factory.makeArchive(
+                distribution=distroarchseries.distroseries.distribution,
+                owner=snap.owner, virtualized=archive_virt)
+            build = snap.requestBuild(
+                snap.owner, archive, distroarchseries,
+                PackagePublishingPocket.RELEASE)
+            self.assertEqual(build_virt, build.virtualized)
+
+    def test_getBuilds(self):
+        # Test the various getBuilds methods.
+        snap = self.factory.makeSnap()
+        builds = [self.factory.makeSnapBuild(snap=snap) for x in range(3)]
+        # We want the latest builds first.
+        builds.reverse()
+
+        self.assertEqual(builds, list(snap.builds))
+        self.assertEqual([], list(snap.completed_builds))
+        self.assertEqual(builds, list(snap.pending_builds))
+
+        # Change the status of one of the builds and retest.
+        builds[0].updateStatus(BuildStatus.BUILDING)
+        builds[0].updateStatus(BuildStatus.FULLYBUILT)
+        self.assertEqual(builds, list(snap.builds))
+        self.assertEqual(builds[:1], list(snap.completed_builds))
+        self.assertEqual(builds[1:], list(snap.pending_builds))
+
+    def test_getBuilds_cancelled_never_started_last(self):
+        # A cancelled build that was never even started sorts to the end.
+        snap = self.factory.makeSnap()
+        fullybuilt = self.factory.makeSnapBuild(snap=snap)
+        instacancelled = self.factory.makeSnapBuild(snap=snap)
+        fullybuilt.updateStatus(BuildStatus.BUILDING)
+        fullybuilt.updateStatus(BuildStatus.FULLYBUILT)
+        instacancelled.updateStatus(BuildStatus.CANCELLED)
+        self.assertEqual([fullybuilt, instacancelled], list(snap.builds))
+        self.assertEqual(
+            [fullybuilt, instacancelled], list(snap.completed_builds))
+        self.assertEqual([], list(snap.pending_builds))
+
+    def test_getBuilds_privacy(self):
+        # The various getBuilds methods exclude builds against invisible
+        # archives.
+        snap = self.factory.makeSnap()
+        archive = self.factory.makeArchive(
+            distribution=snap.distro_series.distribution, owner=snap.owner,
+            private=True)
+        with person_logged_in(snap.owner):
+            build = self.factory.makeSnapBuild(snap=snap, archive=archive)
+            self.assertEqual([build], list(snap.builds))
+            self.assertEqual([build], list(snap.pending_builds))
+        self.assertEqual([], list(snap.builds))
+        self.assertEqual([], list(snap.pending_builds))
+
+    def test_delete_without_builds(self):
+        # A snap package with no builds can be deleted.
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner, owner=owner, distroseries=distroseries,
+            name=u"condemned")
+        self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
+        with person_logged_in(snap.owner):
+            snap.destroySelf()
+        self.assertFalse(getUtility(ISnapSet).exists(owner, u"condemned"))
+
+    def test_delete_with_builds(self):
+        # A snap package with builds cannot be deleted.
+        owner = self.factory.makePerson()
+        distroseries = self.factory.makeDistroSeries()
+        snap = self.factory.makeSnap(
+            registrant=owner, owner=owner, distroseries=distroseries,
+            name=u"condemned")
+        self.factory.makeSnapBuild(snap=snap)
+        self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
+        with person_logged_in(snap.owner):
+            self.assertRaises(CannotDeleteSnap, snap.destroySelf)
+        self.assertTrue(getUtility(ISnapSet).exists(owner, u"condemned"))
+
 
 class TestSnapSet(TestCaseWithFactory):