launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

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

Commit message:
Add a job to upload completed snap builds to the store.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1572605 in Launchpad itself: "Automatically upload snap builds to store"
  https://bugs.launchpad.net/launchpad/+bug/1572605

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

Add a job to upload completed snap builds to the store.

We'll need to create a snap-build-job DB user before landing this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-store-upload-job into lp:launchpad.

=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg	2016-04-15 17:31:36 +0000
+++ database/schema/security.cfg	2016-05-06 16:40:11 +0000
@@ -2519,3 +2519,19 @@
 public.sourcepackagename                = SELECT
 public.webhook                          = SELECT
 public.webhookjob                       = SELECT, UPDATE
+
+[snap-build-job]
+type=user
+groups=script
+public.distribution                     = SELECT
+public.distroarchseries                 = SELECT
+public.distroseries                     = SELECT
+public.job                              = SELECT, UPDATE
+public.libraryfilealias                 = SELECT
+public.libraryfilecontent               = SELECT
+public.person                           = SELECT
+public.snap                             = SELECT
+public.snapbuild                        = SELECT, UPDATE
+public.snapbuildjob                     = SELECT, UPDATE
+public.snapfile                         = SELECT
+public.snappyseries                     = SELECT

=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf	2016-05-06 16:40:10 +0000
+++ lib/lp/services/config/schema-lazr.conf	2016-05-06 16:40:11 +0000
@@ -1807,6 +1807,7 @@
     IRemoveArtifactSubscriptionsJobSource,
     ISelfRenewalNotificationJobSource,
     ISevenDayCommercialExpirationJobSource,
+    ISnapStoreUploadJobSource,
     ITeamInvitationNotificationJobSource,
     ITeamJoinNotificationJobSource,
     IThirtyDayCommercialExpirationJobSource
@@ -1948,6 +1949,10 @@
 dbuser: product-job
 crontab_group: MAIN
 
+[ISnapStoreUploadJobSource]
+module: lp.snappy.interfaces.snapbuildjob
+dbuser: snap-build-job
+
 [ITeamInvitationNotificationJobSource]
 module: lp.registry.interfaces.persontransferjob
 dbuser: person-transfer-job

=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml	2016-05-06 16:40:10 +0000
+++ lib/lp/snappy/configure.zcml	2016-05-06 16:40:11 +0000
@@ -124,6 +124,20 @@
         <allow interface="lp.snappy.interfaces.snapstoreclient.ISnapStoreClient" />
     </securedutility>
 
+    <!-- Snap-related jobs -->
+    <class class="lp.snappy.model.snapbuildjob.SnapBuildJob">
+        <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapBuildJob" />
+    </class>
+    <securedutility
+        component="lp.snappy.model.snapbuildjob.SnapStoreUploadJob"
+        provides="lp.snappy.interfaces.snapbuildjob.ISnapStoreUploadJobSource">
+        <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapStoreUploadJobSource" />
+    </securedutility>
+    <class class="lp.snappy.model.snapbuildjob.SnapStoreUploadJob">
+        <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapBuildJob" />
+        <allow interface="lp.snappy.interfaces.snapbuildjob.ISnapStoreUploadJob" />
+    </class>
+
     <webservice:register module="lp.snappy.interfaces.webservice" />
 
 </configure>

=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py	2016-01-19 17:41:11 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py	2016-05-06 16:40:11 +0000
@@ -20,7 +20,10 @@
     operation_for_version,
     operation_parameters,
     )
-from lazr.restful.fields import Reference
+from lazr.restful.fields import (
+    CollectionField,
+    Reference,
+    )
 from zope.component.interfaces import IObjectEvent
 from zope.interface import Interface
 from zope.schema import (
@@ -103,6 +106,12 @@
         required=True, readonly=True,
         description=_("Whether this build record can be cancelled.")))
 
+    store_upload_jobs = CollectionField(
+        title=_("Store upload jobs for this build."),
+        # Really ISnapStoreUploadJob.
+        value_type=Reference(schema=Interface),
+        readonly=True)
+
     def getFiles():
         """Retrieve the build's `ISnapFile` records.
 

=== added file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py	2016-05-06 16:40:11 +0000
@@ -0,0 +1,58 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Snap build job interfaces."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'ISnapBuildJob',
+    'ISnapStoreUploadJob',
+    'ISnapStoreUploadJobSource',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import (
+    Attribute,
+    Interface,
+    )
+from zope.schema import TextLine
+
+from lp import _
+from lp.services.job.interfaces.job import (
+    IJob,
+    IJobSource,
+    IRunnableJob,
+    )
+from lp.snappy.interfaces.snapbuild import ISnapBuild
+
+
+class ISnapBuildJob(Interface):
+    """A job related to a snap package."""
+
+    job = Reference(
+        title=_("The common Job attributes."), schema=IJob,
+        required=True, readonly=True)
+
+    snapbuild = Reference(
+        title=_("The snap build to use for this job."),
+        schema=ISnapBuild, required=True, readonly=True)
+
+    metadata = Attribute(_("A dict of data about the job."))
+
+
+class ISnapStoreUploadJob(IRunnableJob):
+    """A Job that uploads a snap build to the store."""
+
+    error_message = TextLine(
+        title=_("Error message"), required=False, readonly=True)
+
+
+class ISnapStoreUploadJobSource(IJobSource):
+
+    def create(snapbuild):
+        """Upload a snap build to the store.
+
+        :param snapbuild: The snap build to upload.
+        """

=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py	2016-02-19 14:18:22 +0000
+++ lib/lp/snappy/model/snapbuild.py	2016-05-06 16:40:11 +0000
@@ -48,6 +48,7 @@
     IStore,
     )
 from lp.services.features import getFeatureFlag
+from lp.services.job.model.job import Job
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.librarian.model import (
     LibraryFileAlias,
@@ -65,6 +66,10 @@
     ISnapFile,
     )
 from lp.snappy.mail.snapbuild import SnapBuildMailer
+from lp.snappy.model.snapbuildjob import (
+    SnapBuildJob,
+    SnapBuildJobType,
+    )
 from lp.soyuz.interfaces.component import IComponentSet
 from lp.soyuz.model.archive import Archive
 from lp.soyuz.model.distroarchseries import DistroArchSeries
@@ -346,6 +351,20 @@
     def getFileUrls(self):
         return [self.lfaUrl(lfa) for _, lfa, _ in self.getFiles()]
 
+    @property
+    def store_upload_jobs(self):
+        jobs = Store.of(self).find(
+            SnapBuildJob,
+            SnapBuildJob.snapbuild == self,
+            SnapBuildJob.job_type == SnapBuildJobType.STORE_UPLOAD)
+        jobs.order_by(Desc(SnapBuildJob.job_id))
+
+        def preload_jobs(rows):
+            load_related(Job, rows, ["job_id"])
+
+        return DecoratedResultSet(
+            jobs, lambda job: job.makeDerived(), pre_iter_hook=preload_jobs)
+
 
 @implementer(ISnapBuildSet)
 class SnapBuildSet(SpecificBuildFarmJobSourceMixin):

=== added file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/model/snapbuildjob.py	2016-05-06 16:40:11 +0000
@@ -0,0 +1,191 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Snap build jobs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'SnapBuildJob',
+    'SnapBuildJobType',
+    'SnapStoreUploadJob',
+    ]
+
+from lazr.delegates import delegate_to
+from lazr.enum import (
+    DBEnumeratedType,
+    DBItem,
+    )
+from storm.locals import (
+    Int,
+    JSON,
+    Reference,
+    )
+import transaction
+from zope.component import getUtility
+from zope.interface import (
+    implementer,
+    provider,
+    )
+
+from lp.app.errors import NotFoundError
+from lp.services.config import config
+from lp.services.database.enumcol import EnumCol
+from lp.services.database.interfaces import (
+    IMasterStore,
+    IStore,
+    )
+from lp.services.database.stormbase import StormBase
+from lp.services.job.model.job import (
+    EnumeratedSubclass,
+    Job,
+    )
+from lp.services.job.runner import BaseRunnableJob
+from lp.snappy.interfaces.snapbuildjob import (
+    ISnapBuildJob,
+    ISnapStoreUploadJob,
+    ISnapStoreUploadJobSource,
+    )
+from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
+
+
+class SnapBuildJobType(DBEnumeratedType):
+    """Values that `ISnapBuildJob.job_type` can take."""
+
+    STORE_UPLOAD = DBItem(0, """
+        Store upload
+
+        This job uploads a snap build to the store.
+        """)
+
+
+@implementer(ISnapBuildJob)
+class SnapBuildJob(StormBase):
+    """See `ISnapBuildJob`."""
+
+    __storm_table__ = 'SnapBuildJob'
+
+    job_id = Int(name='job', primary=True, allow_none=False)
+    job = Reference(job_id, 'Job.id')
+
+    snapbuild_id = Int(name='snapbuild', allow_none=False)
+    snapbuild = Reference(snapbuild_id, 'SnapBuild.id')
+
+    job_type = EnumCol(enum=SnapBuildJobType, notNull=True)
+
+    metadata = JSON('json_data', allow_none=False)
+
+    def __init__(self, snapbuild, job_type, metadata, **job_args):
+        """Constructor.
+
+        Extra keyword arguments are used to construct the underlying Job
+        object.
+
+        :param snapbuild: The `ISnapBuild` this job relates to.
+        :param job_type: The `SnapBuildJobType` of this job.
+        :param metadata: The type-specific variables, as a JSON-compatible
+            dict.
+        """
+        super(SnapBuildJob, self).__init__()
+        self.job = Job(**job_args)
+        self.snapbuild = snapbuild
+        self.job_type = job_type
+        self.metadata = metadata
+
+    def makeDerived(self):
+        return SnapBuildJobDerived.makeSubclass(self)
+
+
+@delegate_to(ISnapBuildJob)
+class SnapBuildJobDerived(BaseRunnableJob):
+
+    __metaclass__ = EnumeratedSubclass
+
+    def __init__(self, snap_build_job):
+        self.context = snap_build_job
+
+    def __repr__(self):
+        """An informative representation of the job."""
+        return "<%s for %s>" % (self.__class__.__name__, self.snapbuild.title)
+
+    @classmethod
+    def get(cls, job_id):
+        """Get a job by id.
+
+        :return: The `SnapBuildJob` with the specified id, as the current
+            `SnapBuildJobDerived` subclass.
+        :raises: `NotFoundError` if there is no job with the specified id,
+            or its `job_type` does not match the desired subclass.
+        """
+        snap_build_job = IStore(SnapBuildJob).get(SnapBuildJob, job_id)
+        if snap_build_job.job_type != cls.class_job_type:
+            raise NotFoundError(
+                "No object found with id %d and type %s" %
+                (job_id, cls.class_job_type.title))
+        return cls(snap_build_job)
+
+    @classmethod
+    def iterReady(cls):
+        """See `IJobSource`."""
+        jobs = IMasterStore(SnapBuildJob).find(
+            SnapBuildJob,
+            SnapBuildJob.job_type == cls.class_job_type,
+            SnapBuildJob.job == Job.id,
+            Job.id.is_in(Job.ready_jobs))
+        return (cls(job) for job in jobs)
+
+    def getOopsVars(self):
+        """See `IRunnableJob`."""
+        oops_vars = super(SnapBuildJobDerived, self).getOopsVars()
+        oops_vars.extend([
+            ('job_id', self.context.job.id),
+            ('job_type', self.context.job_type.title),
+            ('snapbuild_id', self.context.snapbuild.id),
+            ('snap_owner_name', self.context.snapbuild.snap.owner.name),
+            ('snap_name', self.context.snapbuild.snap.name),
+            ])
+        return oops_vars
+
+
+@implementer(ISnapStoreUploadJob)
+@provider(ISnapStoreUploadJobSource)
+class SnapStoreUploadJob(SnapBuildJobDerived):
+    """A Job that uploads a snap build to the store."""
+
+    class_job_type = SnapBuildJobType.STORE_UPLOAD
+
+    # XXX cjwatson 2016-05-04: identify transient upload failures and retry
+
+    config = config.ISnapStoreUploadJobSource
+
+    @classmethod
+    def create(cls, snapbuild):
+        """See `ISnapStoreUploadJobSource`."""
+        snap_build_job = SnapBuildJob(snapbuild, cls.class_job_type, {})
+        job = cls(snap_build_job)
+        job.celeryRunOnCommit()
+        return job
+
+    @property
+    def error_message(self):
+        """See `ISnapStoreUploadJob`."""
+        return self.metadata.get("error_message")
+
+    @error_message.setter
+    def error_message(self, message):
+        """See `ISnapStoreUploadJob`."""
+        self.metadata["error_message"] = message
+
+    def run(self):
+        """See `IRunnableJob`."""
+        try:
+            getUtility(ISnapStoreClient).upload(self.snapbuild)
+            self.error_message = None
+        except Exception as e:
+            # Abort work done so far, but make sure that we commit the error
+            # message.
+            transaction.abort()
+            self.error_message = str(e)
+            transaction.commit()
+            raise

=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py	2016-01-19 17:41:11 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py	2016-05-06 16:40:11 +0000
@@ -9,16 +9,18 @@
 
 from zope.component import getUtility
 
+from lp.buildmaster.enums import BuildStatus
 from lp.services.features import getFeatureFlag
 from lp.services.webapp.publisher import canonical_url
 from lp.services.webhooks.interfaces import IWebhookSet
 from lp.services.webhooks.payload import compose_webhook_payload
 from lp.snappy.interfaces.snap import SNAP_WEBHOOKS_FEATURE_FLAG
 from lp.snappy.interfaces.snapbuild import ISnapBuild
+from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
 
 
 def snap_build_status_changed(snapbuild, event):
-    """Trigger webhooks when snap package build statuses change."""
+    """Trigger events when snap package build statuses change."""
     if getFeatureFlag(SNAP_WEBHOOKS_FEATURE_FLAG):
         payload = {
             "snap_build": canonical_url(snapbuild, force_local_path=True),
@@ -28,3 +30,8 @@
             ISnapBuild, snapbuild, ["snap", "status"]))
         getUtility(IWebhookSet).trigger(
             snapbuild.snap, "snap:build:0.1", payload)
+
+    if (snapbuild.snap.store_upload and
+            snapbuild.snap.store_secrets is not None and
+            snapbuild.status == BuildStatus.FULLYBUILT):
+        getUtility(ISnapStoreUploadJobSource).create(snapbuild)

=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py	2016-03-02 21:21:26 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py	2016-05-06 16:40:11 +0000
@@ -250,6 +250,26 @@
                     hook.id, hook.target),
                 repr(delivery))
 
+    def test_updateStatus_failure_does_not_trigger_store_uploads(self):
+        # A failed SnapBuild does not trigger store uploads.
+        self.build.snap.store_series = self.factory.makeSnappySeries()
+        self.build.snap.store_name = self.factory.getUniqueUnicode()
+        self.build.snap.store_upload = True
+        self.build.snap.store_secrets = {
+            "root": "dummy-root", "discharge": "dummy-discharge"}
+        self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
+        self.assertContentEqual([], self.build.store_upload_jobs)
+
+    def test_updateStatus_fullybuilt_triggers_store_uploads(self):
+        # A completed SnapBuild triggers store uploads.
+        self.build.snap.store_series = self.factory.makeSnappySeries()
+        self.build.snap.store_name = self.factory.getUniqueUnicode()
+        self.build.snap.store_upload = True
+        self.build.snap.store_secrets = {
+            "root": "dummy-root", "discharge": "dummy-discharge"}
+        self.build.updateStatus(BuildStatus.FULLYBUILT)
+        self.assertEqual(1, len(list(self.build.store_upload_jobs)))
+
     def test_notify_fullybuilt(self):
         # notify does not send mail when a SnapBuild completes normally.
         person = self.factory.makePerson(name="person")

=== added file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py	1970-01-01 00:00:00 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py	2016-05-06 16:40:11 +0000
@@ -0,0 +1,105 @@
+# Copyright 2016 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for snap build jobs."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from zope.interface import implementer
+
+from lp.services.config import config
+from lp.services.features.testing import FeatureFixture
+from lp.services.job.runner import JobRunner
+from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
+from lp.snappy.interfaces.snapbuildjob import (
+    ISnapBuildJob,
+    ISnapStoreUploadJob,
+    )
+from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
+from lp.snappy.model.snapbuildjob import (
+    SnapBuildJob,
+    SnapBuildJobType,
+    SnapStoreUploadJob,
+    )
+from lp.testing import TestCaseWithFactory
+from lp.testing.dbuser import dbuser
+from lp.testing.fakemethod import FakeMethod
+from lp.testing.fixture import ZopeUtilityFixture
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadZopelessLayer,
+    )
+
+
+@implementer(ISnapStoreClient)
+class FakeSnapStoreClient:
+
+    def __init__(self):
+        self.upload = FakeMethod()
+
+
+class TestSnapBuildJob(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestSnapBuildJob, self).setUp()
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+
+    def test_provides_interface(self):
+        # `SnapBuildJob` objects provide `ISnapBuildJob`.
+        snapbuild = self.factory.makeSnapBuild()
+        self.assertProvides(
+            SnapBuildJob(snapbuild, SnapBuildJobType.STORE_UPLOAD, {}),
+            ISnapBuildJob)
+
+
+class TestSnapStoreUploadJob(TestCaseWithFactory):
+
+    layer = LaunchpadZopelessLayer
+
+    def setUp(self):
+        super(TestSnapStoreUploadJob, self).setUp()
+        self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS))
+
+    def test_provides_interface(self):
+        # `SnapStoreUploadJob` objects provide `ISnapStoreUploadJob`.
+        snapbuild = self.factory.makeSnapBuild()
+        job = SnapStoreUploadJob.create(snapbuild)
+        self.assertProvides(job, ISnapStoreUploadJob)
+
+    def test___repr__(self):
+        # `SnapStoreUploadJob` objects have an informative __repr__.
+        snapbuild = self.factory.makeSnapBuild()
+        job = SnapStoreUploadJob.create(snapbuild)
+        self.assertEqual(
+            "<SnapStoreUploadJob for %s>" % snapbuild.title, repr(job))
+
+    def test_run(self):
+        # The job uploads the build to the store.
+        snapbuild = self.factory.makeSnapBuild()
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertIsNone(job.error_message)
+
+    def test_run_failed(self):
+        # A failed run sets the store upload status to FAILED.
+        snapbuild = self.factory.makeSnapBuild()
+        self.assertContentEqual([], snapbuild.store_upload_jobs)
+        job = SnapStoreUploadJob.create(snapbuild)
+        client = FakeSnapStoreClient()
+        client.upload.failure = ValueError("An upload failure")
+        self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+        with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+            JobRunner([job]).runAll()
+        self.assertEqual([((snapbuild,), {})], client.upload.calls)
+        self.assertContentEqual([job], snapbuild.store_upload_jobs)
+        self.assertEqual("An upload failure", job.error_message)