launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #21410
[Merge] lp:~cjwatson/launchpad/snap-webhook-store-status into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-webhook-store-status into lp:launchpad.
Commit message:
Extend snap webhooks to include store_upload_status.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-webhook-store-status/+merge/320295
This is needed for build.snapcraft.io, so that it can increment metrics when snaps are released to edge; but in general it seems reasonable to include store upload status as well as build status in the webhook payload and trigger deliveries when it changes.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-webhook-store-status into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2016-11-23 03:42:51 +0000
+++ database/schema/security.cfg 2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
#
# Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -2565,7 +2565,7 @@
public.distroarchseries = SELECT
public.distroseries = SELECT
public.emailaddress = SELECT
-public.job = SELECT, UPDATE
+public.job = SELECT, INSERT, UPDATE
public.libraryfilealias = SELECT
public.libraryfilecontent = SELECT
public.person = SELECT
@@ -2576,3 +2576,5 @@
public.snapfile = SELECT
public.snappyseries = SELECT
public.teammembership = SELECT
+public.webhook = SELECT
+public.webhookjob = SELECT, INSERT
=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml 2016-06-20 21:17:58 +0000
+++ lib/lp/snappy/configure.zcml 2017-03-20 00:43:16 +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).
-->
@@ -139,6 +139,10 @@
<allow interface="lp.snappy.interfaces.snapbuildjob.ISnapBuildJob" />
<allow interface="lp.snappy.interfaces.snapbuildjob.ISnapStoreUploadJob" />
</class>
+ <subscriber
+ for="lp.snappy.interfaces.snapbuild.ISnapBuild
+ lp.snappy.interfaces.snapbuildjob.ISnapBuildStoreUploadStatusChangedEvent"
+ handler="lp.snappy.subscribers.snapbuild.snap_build_store_upload_status_changed" />
<webservice:register module="lp.snappy.interfaces.webservice" />
=== modified file 'lib/lp/snappy/interfaces/snapbuildjob.py'
--- lib/lp/snappy/interfaces/snapbuildjob.py 2016-06-21 14:51:06 +0000
+++ lib/lp/snappy/interfaces/snapbuildjob.py 2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Snap build job interfaces."""
@@ -8,11 +8,13 @@
__metaclass__ = type
__all__ = [
'ISnapBuildJob',
+ 'ISnapBuildStoreUploadStatusChangedEvent',
'ISnapStoreUploadJob',
'ISnapStoreUploadJobSource',
]
from lazr.restful.fields import Reference
+from zope.component.interfaces import IObjectEvent
from zope.interface import (
Attribute,
Interface,
@@ -42,6 +44,10 @@
metadata = Attribute(_("A dict of data about the job."))
+class ISnapBuildStoreUploadStatusChangedEvent(IObjectEvent):
+ """The store upload status of a snap package build changed."""
+
+
class ISnapStoreUploadJob(IRunnableJob):
"""A Job that uploads a snap build to the store."""
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2016-10-16 22:04:39 +0000
+++ lib/lp/snappy/model/snapbuildjob.py 2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Snap build jobs."""
@@ -9,6 +9,7 @@
__all__ = [
'SnapBuildJob',
'SnapBuildJobType',
+ 'SnapBuildStoreUploadStatusChangedEvent',
'SnapStoreUploadJob',
]
@@ -26,6 +27,8 @@
)
import transaction
from zope.component import getUtility
+from zope.component.interfaces import ObjectEvent
+from zope.event import notify
from zope.interface import (
implementer,
provider,
@@ -44,8 +47,10 @@
Job,
)
from lp.services.job.runner import BaseRunnableJob
+from lp.services.propertycache import get_property_cache
from lp.snappy.interfaces.snapbuildjob import (
ISnapBuildJob,
+ ISnapBuildStoreUploadStatusChangedEvent,
ISnapStoreUploadJob,
ISnapStoreUploadJobSource,
)
@@ -164,6 +169,11 @@
pass
+@implementer(ISnapBuildStoreUploadStatusChangedEvent)
+class SnapBuildStoreUploadStatusChangedEvent(ObjectEvent):
+ """See `ISnapBuildStoreUploadStatusChangedEvent`."""
+
+
@implementer(ISnapStoreUploadJob)
@provider(ISnapStoreUploadJobSource)
class SnapStoreUploadJob(SnapBuildJobDerived):
@@ -191,6 +201,8 @@
snap_build_job = SnapBuildJob(snapbuild, cls.class_job_type, {})
job = cls(snap_build_job)
job.celeryRunOnCommit()
+ del get_property_cache(snapbuild).last_store_upload_job
+ notify(SnapBuildStoreUploadStatusChangedEvent(snapbuild))
return job
@property
@@ -213,6 +225,33 @@
"""See `ISnapStoreUploadJob`."""
self.metadata["store_url"] = url
+ # Ideally we'd just override Job._set_status or similar, but
+ # lazr.delegates makes that difficult, so we use this to override all
+ # the individual Job lifecycle methods instead.
+ def _do_lifecycle(self, method, *args, **kwargs):
+ old_store_upload_status = self.snapbuild.store_upload_status
+ method(*args, **kwargs)
+ if self.snapbuild.store_upload_status != old_store_upload_status:
+ notify(SnapBuildStoreUploadStatusChangedEvent(self.snapbuild))
+
+ def start(self, *args, **kwargs):
+ self._do_lifecycle(self.job.start, *args, **kwargs)
+
+ def complete(self, *args, **kwargs):
+ self._do_lifecycle(self.job.complete, *args, **kwargs)
+
+ def fail(self, *args, **kwargs):
+ self._do_lifecycle(self.job.fail, *args, **kwargs)
+
+ def queue(self, *args, **kwargs):
+ self._do_lifecycle(self.job.queue, *args, **kwargs)
+
+ def suspend(self, *args, **kwargs):
+ self._do_lifecycle(self.job.suspend, *args, **kwargs)
+
+ def resume(self, *args, **kwargs):
+ self._do_lifecycle(self.job.resume, *args, **kwargs)
+
def run(self):
"""See `IRunnableJob`."""
client = getUtility(ISnapStoreClient)
=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py 2016-07-19 16:32:46 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py 2017-03-20 00:43:16 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Event subscribers for snap builds."""
@@ -19,18 +19,27 @@
from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
-def snap_build_status_changed(snapbuild, event):
- """Trigger events when snap package build statuses change."""
+def _trigger_snap_build_webhook(snapbuild):
if getFeatureFlag(SNAP_WEBHOOKS_FEATURE_FLAG):
payload = {
"snap_build": canonical_url(snapbuild, force_local_path=True),
"action": "status-changed",
}
payload.update(compose_webhook_payload(
- ISnapBuild, snapbuild, ["snap", "status"]))
+ ISnapBuild, snapbuild, ["snap", "status", "store_upload_status"]))
getUtility(IWebhookSet).trigger(
snapbuild.snap, "snap:build:0.1", payload)
+
+def snap_build_status_changed(snapbuild, event):
+ """Trigger events when snap package build statuses change."""
+ _trigger_snap_build_webhook(snapbuild)
+
if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
snapbuild.status == BuildStatus.FULLYBUILT):
getUtility(ISnapStoreUploadJobSource).create(snapbuild)
+
+
+def snap_build_store_upload_status_changed(snapbuild, event):
+ """Trigger events when snap package build store upload statuses change."""
+ _trigger_snap_build_webhook(snapbuild)
=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py 2017-02-27 18:46:38 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py 2017-03-20 00:43:16 +0000
@@ -231,6 +231,7 @@
"snap": Equals(
canonical_url(self.build.snap, force_local_path=True)),
"status": Equals("Successfully built"),
+ "store_upload_status": Equals("Unscheduled"),
}
delivery = hook.deliveries.one()
self.assertThat(
@@ -471,6 +472,37 @@
self.assertEqual(
[], list(getUtility(ISnapStoreUploadJobSource).iterReady()))
+ def test_scheduleStoreUpload_triggers_webhooks(self):
+ # Scheduling a store upload triggers webhooks on the corresponding
+ # snap.
+ self.setUpStoreUpload()
+ self.build.updateStatus(BuildStatus.FULLYBUILT)
+ self.factory.makeSnapFile(
+ snapbuild=self.build,
+ libraryfile=self.factory.makeLibraryFileAlias(db_only=True))
+ hook = self.factory.makeWebhook(
+ target=self.build.snap, event_types=["snap:build:0.1"])
+ self.build.scheduleStoreUpload()
+ expected_payload = {
+ "snap_build": Equals(
+ canonical_url(self.build, force_local_path=True)),
+ "action": Equals("status-changed"),
+ "snap": Equals(
+ canonical_url(self.build.snap, force_local_path=True)),
+ "status": Equals("Successfully built"),
+ "store_upload_status": Equals("Pending"),
+ }
+ delivery = hook.deliveries.one()
+ self.assertThat(
+ delivery, MatchesStructure(
+ event_type=Equals("snap:build:0.1"),
+ payload=MatchesDict(expected_payload)))
+ with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+ self.assertEqual(
+ "<WebhookDeliveryJob for webhook %d on %r>" % (
+ hook.id, hook.target),
+ repr(delivery))
+
class TestSnapBuildSet(TestCaseWithFactory):
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2017-01-04 20:52:12 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2017-03-20 00:43:16 +0000
@@ -8,12 +8,20 @@
__metaclass__ = type
from fixtures import FakeLogger
+from testtools.matchers import (
+ Equals,
+ MatchesDict,
+ MatchesListwise,
+ MatchesStructure,
+ )
from zope.interface import implementer
+from lp.buildmaster.enums import BuildStatus
from lp.services.config import config
from lp.services.features.testing import FeatureFixture
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.runner import JobRunner
+from lp.services.webapp.publisher import canonical_url
from lp.snappy.interfaces.snap import SNAP_TESTING_FLAGS
from lp.snappy.interfaces.snapbuildjob import (
ISnapBuildJob,
@@ -92,10 +100,45 @@
self.assertEqual(
"<SnapStoreUploadJob for %s>" % snapbuild.title, repr(job))
+ def makeSnapBuild(self, **kwargs):
+ # Make a build with a builder and a webhook.
+ snapbuild = self.factory.makeSnapBuild(
+ builder=self.factory.makeBuilder(), **kwargs)
+ snapbuild.updateStatus(BuildStatus.FULLYBUILT)
+ self.factory.makeWebhook(
+ target=snapbuild.snap, event_types=["snap:build:0.1"])
+ return snapbuild
+
+ def assertWebhookDeliveries(self, snapbuild,
+ expected_store_upload_statuses):
+ hook = snapbuild.snap.webhooks.one()
+ deliveries = list(hook.deliveries)
+ deliveries.reverse()
+ expected_payloads = [{
+ "snap_build": Equals(
+ canonical_url(snapbuild, force_local_path=True)),
+ "action": Equals("status-changed"),
+ "snap": Equals(
+ canonical_url(snapbuild.snap, force_local_path=True)),
+ "status": Equals("Successfully built"),
+ "store_upload_status": Equals(expected),
+ } for expected in expected_store_upload_statuses]
+ matchers = [
+ MatchesStructure(
+ event_type=Equals("snap:build:0.1"),
+ payload=MatchesDict(expected_payload))
+ for expected_payload in expected_payloads]
+ self.assertThat(deliveries, MatchesListwise(matchers))
+ with dbuser(config.IWebhookDeliveryJobSource.dbuser):
+ for delivery in deliveries:
+ self.assertEqual(
+ "<WebhookDeliveryJob for webhook %d on %r>" % (
+ hook.id, hook.target),
+ repr(delivery))
+
def test_run(self):
# The job uploads the build to the store and records the store URL.
- snapbuild = self.factory.makeSnapBuild(
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild()
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -111,11 +154,11 @@
self.assertEqual(self.store_url, job.store_url)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
+ self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
def test_run_failed(self):
# A failed run sets the store upload status to FAILED.
- snapbuild = self.factory.makeSnapBuild(
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild()
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -130,15 +173,16 @@
self.assertIsNone(job.store_url)
self.assertEqual("An upload failure", job.error_message)
self.assertEqual([], pop_notifications())
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to upload"])
def test_run_unauthorized_notifies(self):
# A run that gets 401 from the store sends mail.
requester = self.factory.makePerson(name="requester")
requester_team = self.factory.makeTeam(
owner=requester, name="requester-team", members=[requester])
- snapbuild = self.factory.makeSnapBuild(
- requester=requester_team, name="test-snap", owner=requester_team,
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild(
+ requester=requester_team, name="test-snap", owner=requester_team)
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -177,6 +221,8 @@
"http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n";
"Your team Requester Team is the requester of the build.\n" %
snapbuild.id, footer)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to upload"])
def test_run_upload_failure_notifies(self):
# A run that gets some other upload failure from the store sends
@@ -184,9 +230,8 @@
requester = self.factory.makePerson(name="requester")
requester_team = self.factory.makeTeam(
owner=requester, name="requester-team", members=[requester])
- snapbuild = self.factory.makeSnapBuild(
- requester=requester_team, name="test-snap", owner=requester_team,
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild(
+ requester=requester_team, name="test-snap", owner=requester_team)
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -225,13 +270,14 @@
self.assertEqual(
"%s\nYour team Requester Team is the requester of the build.\n" %
build_url, footer)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to upload"])
def test_run_scan_pending_retries(self):
# A run that finds that the store has not yet finished scanning the
# package schedules itself to be retried.
self.useFixture(FakeLogger())
- snapbuild = self.factory.makeSnapBuild(
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild()
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -248,6 +294,7 @@
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.WAITING, job.job.status)
+ self.assertWebhookDeliveries(snapbuild, ["Pending"])
# Try again. The upload part of the job is not retried, and this
# time the scan completes.
job.lease_expires = None
@@ -266,15 +313,15 @@
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
self.assertEqual(JobStatus.COMPLETED, job.job.status)
+ self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
def test_run_scan_failure_notifies(self):
# A run that gets a scan failure from the store sends mail.
requester = self.factory.makePerson(name="requester")
requester_team = self.factory.makeTeam(
owner=requester, name="requester-team", members=[requester])
- snapbuild = self.factory.makeSnapBuild(
- requester=requester_team, name="test-snap", owner=requester_team,
- builder=self.factory.makeBuilder())
+ snapbuild = self.makeSnapBuild(
+ requester=requester_team, name="test-snap", owner=requester_team)
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -311,12 +358,13 @@
"http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n";
"Your team Requester Team is the requester of the build.\n" %
snapbuild.id, footer)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to upload"])
def test_run_release(self):
# A run configured to automatically release the package to certain
# channels does so.
- snapbuild = self.factory.makeSnapBuild(
- store_channels=["stable", "edge"])
+ snapbuild = self.makeSnapBuild(store_channels=["stable", "edge"])
self.assertContentEqual([], snapbuild.store_upload_jobs)
job = SnapStoreUploadJob.create(snapbuild)
client = FakeSnapStoreClient()
@@ -332,6 +380,7 @@
self.assertEqual(self.store_url, job.store_url)
self.assertIsNone(job.error_message)
self.assertEqual([], pop_notifications())
+ self.assertWebhookDeliveries(snapbuild, ["Pending", "Uploaded"])
def test_run_release_manual_review_notifies(self):
# A run configured to automatically release the package to certain
@@ -340,7 +389,7 @@
requester = self.factory.makePerson(name="requester")
requester_team = self.factory.makeTeam(
owner=requester, name="requester-team", members=[requester])
- snapbuild = self.factory.makeSnapBuild(
+ snapbuild = self.makeSnapBuild(
requester=requester_team, name="test-snap", owner=requester_team,
store_channels=["stable", "edge"])
self.assertContentEqual([], snapbuild.store_upload_jobs)
@@ -382,6 +431,8 @@
"http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n";
"Your team Requester Team is the requester of the build.\n" %
snapbuild.id, footer)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to release to channels"])
def test_run_release_failure_notifies(self):
# A run configured to automatically release the package to certain
@@ -389,7 +440,7 @@
requester = self.factory.makePerson(name="requester")
requester_team = self.factory.makeTeam(
owner=requester, name="requester-team", members=[requester])
- snapbuild = self.factory.makeSnapBuild(
+ snapbuild = self.makeSnapBuild(
requester=requester_team, name="test-snap", owner=requester_team,
store_channels=["stable", "edge"])
self.assertContentEqual([], snapbuild.store_upload_jobs)
@@ -430,3 +481,5 @@
"http://launchpad.dev/~requester-team/+snap/test-snap/+build/%d\n";
"Your team Requester Team is the requester of the build.\n" %
snapbuild.id, footer)
+ self.assertWebhookDeliveries(
+ snapbuild, ["Pending", "Failed to release to channels"])
Follow ups
