launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22794
[Merge] lp:~cjwatson/launchpad/snap-build-created-webhook into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-build-created-webhook into lp:launchpad.
Commit message:
Issue snap:build:0.1 webhook delivery when creating a snap build, and include a link to the build request in the payload.
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-build-created-webhook/+merge/352306
I need this as part of converting build.snapcraft.io over to the new snap.requestBuilds interface; it needs to be able to issue a build request, keep a note of the reason it did so, and then retrieve that when it receives a webhook delivery indicating that the build has been created. (The alternatives are to have BSI's request-builds endpoint poll until the builds are created, which is obviously horrible, or to lose the build annotations.)
This requires https://code.launchpad.net/~cjwatson/launchpad/db-snap-build-build-request/+merge/352303.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-build-created-webhook into lp:launchpad.
=== modified file 'lib/lp/snappy/configure.zcml'
--- lib/lp/snappy/configure.zcml 2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/configure.zcml 2018-08-03 14:22:02 +0000
@@ -63,6 +63,10 @@
</class>
<subscriber
for="lp.snappy.interfaces.snapbuild.ISnapBuild
+ lazr.lifecycle.interfaces.IObjectCreatedEvent"
+ handler="lp.snappy.subscribers.snapbuild.snap_build_created" />
+ <subscriber
+ for="lp.snappy.interfaces.snapbuild.ISnapBuild
lp.snappy.interfaces.snapbuild.ISnapBuildStatusChangedEvent"
handler="lp.snappy.subscribers.snapbuild.snap_build_status_changed" />
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2018-06-18 22:08:58 +0000
+++ lib/lp/snappy/interfaces/snap.py 2018-08-03 14:22:02 +0000
@@ -365,7 +365,7 @@
@export_factory_operation(Interface, [])
@operation_for_version("devel")
def requestBuild(requester, archive, distro_arch_series, pocket,
- channels=None):
+ channels=None, build_request=None):
"""Request that the snap package be built.
:param requester: The person requesting the build.
@@ -374,6 +374,8 @@
:param pocket: The pocket that should be targeted.
:param channels: A dictionary mapping snap names to channels to use
for this build.
+ :param build_request: The `ISnapBuildRequest` job being processed,
+ if any.
:return: `ISnapBuild`.
"""
@@ -407,7 +409,7 @@
def requestBuildsFromJob(requester, archive, pocket, channels=None,
allow_failures=False, fetch_snapcraft_yaml=True,
- logger=None):
+ build_request=None, logger=None):
"""Synchronous part of `Snap.requestBuilds`.
Request that the snap package be built for relevant architectures.
@@ -424,6 +426,8 @@
appropriate branch or repository and use it to decide which
builds to request; if False, fall back to building for all
supported architectures.
+ :param build_request: The `ISnapBuildRequest` job being processed,
+ if any.
:param logger: An optional logger.
:return: A sequence of `ISnapBuild` instances.
"""
=== modified file 'lib/lp/snappy/interfaces/snapbuild.py'
--- lib/lp/snappy/interfaces/snapbuild.py 2018-03-22 16:48:16 +0000
+++ lib/lp/snappy/interfaces/snapbuild.py 2018-08-03 14:22:02 +0000
@@ -52,7 +52,10 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.database.constants import DEFAULT
from lp.services.librarian.interfaces import ILibraryFileAlias
-from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snap import (
+ ISnap,
+ ISnapBuildRequest,
+ )
from lp.soyuz.interfaces.archive import IArchive
from lp.soyuz.interfaces.distroarchseries import IDistroArchSeries
@@ -122,6 +125,11 @@
class ISnapBuildView(IPackageBuild):
"""`ISnapBuild` attributes that require launchpad.View permission."""
+ build_request = Reference(
+ ISnapBuildRequest,
+ title=_("The build request that caused this build to be created."),
+ required=False, readonly=True)
+
requester = exported(Reference(
IPerson,
title=_("The person who requested this build."),
=== modified file 'lib/lp/snappy/interfaces/snapjob.py'
--- lib/lp/snappy/interfaces/snapjob.py 2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/interfaces/snapjob.py 2018-08-03 14:22:02 +0000
@@ -32,7 +32,10 @@
IJobSource,
IRunnableJob,
)
-from lp.snappy.interfaces.snap import ISnap
+from lp.snappy.interfaces.snap import (
+ ISnap,
+ ISnapBuildRequest,
+ )
from lp.snappy.interfaces.snapbuild import ISnapBuild
from lp.soyuz.interfaces.archive import IArchive
@@ -78,6 +81,10 @@
title=_("Error message resulting from running this job."),
required=False, readonly=True)
+ build_request = Reference(
+ title=_("The build request corresponding to this job."),
+ schema=ISnapBuildRequest, required=True, readonly=True)
+
builds = List(
title=_("The builds created by this request."),
value_type=Reference(schema=ISnapBuild), required=True, readonly=True)
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2018-07-30 09:07:30 +0000
+++ lib/lp/snappy/model/snap.py 2018-08-03 14:22:02 +0000
@@ -14,6 +14,7 @@
from operator import attrgetter
from urlparse import urlsplit
+from lazr.lifecycle.event import ObjectCreatedEvent
from pymacaroons import Macaroon
import pytz
from storm.expr import (
@@ -39,6 +40,7 @@
getAdapter,
getUtility,
)
+from zope.event import notify
from zope.interface import implementer
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
@@ -521,7 +523,7 @@
raise SnapBuildArchiveOwnerMismatch()
def requestBuild(self, requester, archive, distro_arch_series, pocket,
- channels=None):
+ channels=None, build_request=None):
"""See `ISnap`."""
self._checkRequestBuild(requester, archive)
if distro_arch_series not in self.getAllowedArchitectures():
@@ -540,8 +542,9 @@
build = getUtility(ISnapBuildSet).new(
requester, self, archive, distro_arch_series, pocket,
- channels=channels)
+ channels=channels, build_request=build_request)
build.queueBuild()
+ notify(ObjectCreatedEvent(build, user=requester))
return build
def requestBuilds(self, requester, archive, pocket, channels=None):
@@ -553,7 +556,7 @@
def requestBuildsFromJob(self, requester, archive, pocket, channels=None,
allow_failures=False, fetch_snapcraft_yaml=True,
- logger=None):
+ build_request=None, logger=None):
"""See `ISnap`."""
if fetch_snapcraft_yaml:
try:
@@ -585,7 +588,7 @@
try:
build = self.requestBuild(
requester, archive, supported_arches[arch], pocket,
- channels)
+ channels, build_request=build_request)
if logger is not None:
logger.debug(
" - %s/%s/%s: Build requested.",
=== modified file 'lib/lp/snappy/model/snapbuild.py'
--- lib/lp/snappy/model/snapbuild.py 2018-03-22 16:48:16 +0000
+++ lib/lp/snappy/model/snapbuild.py 2018-08-03 14:22:02 +0000
@@ -128,6 +128,8 @@
build_farm_job_id = Int(name='build_farm_job', allow_none=False)
build_farm_job = Reference(build_farm_job_id, 'BuildFarmJob.id')
+ build_request_id = Int(name='build_request', allow_none=True)
+
requester_id = Int(name='requester', allow_none=False)
requester = Reference(requester_id, 'Person.id')
@@ -175,7 +177,7 @@
def __init__(self, build_farm_job, requester, snap, archive,
distro_arch_series, pocket, channels, processor, virtualized,
- date_created):
+ date_created, build_request=None):
"""Construct a `SnapBuild`."""
super(SnapBuild, self).__init__()
self.build_farm_job = build_farm_job
@@ -188,9 +190,17 @@
self.processor = processor
self.virtualized = virtualized
self.date_created = date_created
+ if build_request is not None:
+ self.build_request_id = build_request.id
self.status = BuildStatus.NEEDSBUILD
@property
+ def build_request(self):
+ """See `ISnapBuild`."""
+ if self.build_request_id is not None:
+ return self.snap.getBuildRequest(self.build_request_id)
+
+ @property
def is_private(self):
"""See `IBuildFarmJob`."""
return (
@@ -488,7 +498,7 @@
class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
def new(self, requester, snap, archive, distro_arch_series, pocket,
- channels=None, date_created=DEFAULT):
+ channels=None, date_created=DEFAULT, build_request=None):
"""See `ISnapBuildSet`."""
store = IMasterStore(SnapBuild)
build_farm_job = getUtility(IBuildFarmJobSource).new(
@@ -499,7 +509,7 @@
pocket, channels, distro_arch_series.processor,
not distro_arch_series.processor.supports_nonvirtualized
or snap.require_virtualized or archive.require_virtualized,
- date_created)
+ date_created, build_request=build_request)
store.add(snapbuild)
return snapbuild
=== modified file 'lib/lp/snappy/model/snapjob.py'
--- lib/lp/snappy/model/snapjob.py 2018-06-15 13:21:14 +0000
+++ lib/lp/snappy/model/snapjob.py 2018-08-03 14:22:02 +0000
@@ -243,6 +243,11 @@
self.metadata["error_message"] = message
@property
+ def build_request(self):
+ """See `ISnapRequestBuildsJob`."""
+ return self.snap.getBuildRequest(self.job.id)
+
+ @property
def builds(self):
"""See `ISnapRequestBuildsJob`."""
build_ids = self.metadata.get("builds")
@@ -272,7 +277,7 @@
try:
self.builds = self.snap.requestBuildsFromJob(
requester, archive, self.pocket, channels=self.channels,
- logger=log)
+ build_request=self.build_request, logger=log)
self.error_message = None
except self.retry_error_types:
raise
=== modified file 'lib/lp/snappy/subscribers/snapbuild.py'
--- lib/lp/snappy/subscribers/snapbuild.py 2017-03-20 00:03:52 +0000
+++ lib/lp/snappy/subscribers/snapbuild.py 2018-08-03 14:22:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2018 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,21 +19,27 @@
from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
-def _trigger_snap_build_webhook(snapbuild):
+def _trigger_snap_build_webhook(snapbuild, action):
if getFeatureFlag(SNAP_WEBHOOKS_FEATURE_FLAG):
payload = {
"snap_build": canonical_url(snapbuild, force_local_path=True),
- "action": "status-changed",
+ "action": action,
}
payload.update(compose_webhook_payload(
- ISnapBuild, snapbuild, ["snap", "status", "store_upload_status"]))
+ ISnapBuild, snapbuild,
+ ["snap", "build_request", "status", "store_upload_status"]))
getUtility(IWebhookSet).trigger(
snapbuild.snap, "snap:build:0.1", payload)
+def snap_build_created(snapbuild, event):
+ """Trigger events when a new snap package build is created."""
+ _trigger_snap_build_webhook(snapbuild, "created")
+
+
def snap_build_status_changed(snapbuild, event):
"""Trigger events when snap package build statuses change."""
- _trigger_snap_build_webhook(snapbuild)
+ _trigger_snap_build_webhook(snapbuild, "status-changed")
if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
snapbuild.status == BuildStatus.FULLYBUILT):
@@ -42,4 +48,4 @@
def snap_build_store_upload_status_changed(snapbuild, event):
"""Trigger events when snap package build store upload statuses change."""
- _trigger_snap_build_webhook(snapbuild)
+ _trigger_snap_build_webhook(snapbuild, "status-changed")
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2018-07-30 09:07:30 +0000
+++ lib/lp/snappy/tests/test_snap.py 2018-08-03 14:22:02 +0000
@@ -76,6 +76,7 @@
from lp.services.propertycache import clear_property_cache
from lp.services.timeout import default_timeout
from lp.services.webapp.interfaces import OAuthPermission
+from lp.services.webapp.publisher import canonical_url
from lp.snappy.interfaces.snap import (
BadSnapSearchContext,
CannotFetchSnapcraftYaml,
@@ -368,6 +369,38 @@
snap.owner, snap.distro_series.main_archive, distroarchseries,
PackagePublishingPocket.UPDATES)
+ def test_requestBuild_triggers_webhooks(self):
+ # Requesting a build triggers webhooks.
+ processor = self.factory.makeProcessor(supports_virtualized=True)
+ distroarchseries = self.makeBuildableDistroArchSeries(
+ processor=processor)
+ snap = self.factory.makeSnap(
+ distroseries=distroarchseries.distroseries, processors=[processor])
+ hook = self.factory.makeWebhook(
+ target=snap, event_types=["snap:build:0.1"])
+ build = snap.requestBuild(
+ snap.owner, snap.distro_series.main_archive, distroarchseries,
+ PackagePublishingPocket.UPDATES)
+ expected_payload = {
+ "snap_build": Equals(canonical_url(build, force_local_path=True)),
+ "action": Equals("created"),
+ "snap": Equals(canonical_url(snap, force_local_path=True)),
+ "build_request": Is(None),
+ "status": Equals("Needs building"),
+ "store_upload_status": Equals("Unscheduled"),
+ }
+ with person_logged_in(snap.owner):
+ 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))
+
def test_requestBuilds(self):
# requestBuilds schedules a job and returns a corresponding
# SnapBuildRequest.
@@ -437,7 +470,8 @@
with person_logged_in(job.requester):
builds = job.snap.requestBuildsFromJob(
job.requester, job.archive, job.pocket,
- removeSecurityProxy(job.channels))
+ removeSecurityProxy(job.channels),
+ build_request=job.build_request)
self.assertRequestedBuildsMatch(builds, job, ["sparc"])
def test_requestBuildsFromJob_no_explicit_architectures(self):
@@ -449,10 +483,11 @@
with person_logged_in(job.requester):
builds = job.snap.requestBuildsFromJob(
job.requester, job.archive, job.pocket,
- removeSecurityProxy(job.channels))
+ removeSecurityProxy(job.channels),
+ build_request=job.build_request)
self.assertRequestedBuildsMatch(builds, job, ["mips64el", "riscv64"])
- def test_requestBuilds_unsupported_remote(self):
+ def test_requestBuildsFromJob_unsupported_remote(self):
# If the snap is based on an external Git repository from which we
# don't support fetching blobs, requestBuildsFromJob falls back to
# requesting builds for all configured architectures.
@@ -463,9 +498,43 @@
with person_logged_in(job.requester):
builds = job.snap.requestBuildsFromJob(
job.requester, job.archive, job.pocket,
- removeSecurityProxy(job.channels))
+ removeSecurityProxy(job.channels),
+ build_request=job.build_request)
self.assertRequestedBuildsMatch(builds, job, ["mips64el", "riscv64"])
+ def test_requestBuildsFromJob_triggers_webhooks(self):
+ # requestBuildsFromJob triggers webhooks, and the payload includes a
+ # link to the build request.
+ self.useFixture(GitHostingFixture(blob=dedent("""\
+ architectures:
+ - build-on: avr
+ - build-on: riscv64
+ """)))
+ job = self.makeRequestBuildsJob(["avr", "riscv64", "sparc"])
+ hook = self.factory.makeWebhook(
+ target=job.snap, event_types=["snap:build:0.1"])
+ with person_logged_in(job.requester):
+ builds = job.snap.requestBuildsFromJob(
+ job.requester, job.archive, job.pocket,
+ removeSecurityProxy(job.channels),
+ build_request=job.build_request)
+ self.assertEqual(2, len(builds))
+ self.assertThat(hook.deliveries, MatchesSetwise(*(
+ MatchesStructure(
+ event_type=Equals("snap:build:0.1"),
+ payload=MatchesDict({
+ "snap_build": Equals(canonical_url(
+ build, force_local_path=True)),
+ "action": Equals("created"),
+ "snap": Equals(canonical_url(
+ job.snap, force_local_path=True)),
+ "build_request": Equals(canonical_url(
+ job.build_request, force_local_path=True)),
+ "status": Equals("Needs building"),
+ "store_upload_status": Equals("Unscheduled"),
+ }))
+ for build in builds)))
+
def test_requestAutoBuilds(self):
# requestAutoBuilds creates new builds for all configured
# architectures with appropriate parameters.
=== modified file 'lib/lp/snappy/tests/test_snapbuild.py'
--- lib/lp/snappy/tests/test_snapbuild.py 2018-02-08 12:47:44 +0000
+++ lib/lp/snappy/tests/test_snapbuild.py 2018-08-03 14:22:02 +0000
@@ -20,6 +20,7 @@
import pytz
from testtools.matchers import (
Equals,
+ Is,
MatchesDict,
MatchesStructure,
)
@@ -241,6 +242,7 @@
"action": Equals("status-changed"),
"snap": Equals(
canonical_url(self.build.snap, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
"store_upload_status": Equals("Unscheduled"),
}
@@ -516,6 +518,7 @@
"action": Equals("status-changed"),
"snap": Equals(
canonical_url(self.build.snap, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
"store_upload_status": Equals("Pending"),
}
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2018-02-19 17:48:22 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2018-08-03 14:22:02 +0000
@@ -1,4 +1,4 @@
-# Copyright 2016-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2016-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for snap build jobs."""
@@ -12,6 +12,7 @@
from fixtures import FakeLogger
from testtools.matchers import (
Equals,
+ Is,
MatchesDict,
MatchesListwise,
MatchesStructure,
@@ -125,6 +126,7 @@
"action": Equals("status-changed"),
"snap": Equals(
canonical_url(snapbuild.snap, force_local_path=True)),
+ "build_request": Is(None),
"status": Equals("Successfully built"),
"store_upload_status": Equals(expected),
} for expected in expected_store_upload_statuses]
=== modified file 'lib/lp/snappy/tests/test_snapjob.py'
--- lib/lp/snappy/tests/test_snapjob.py 2018-06-15 12:54:27 +0000
+++ lib/lp/snappy/tests/test_snapjob.py 2018-08-03 14:22:02 +0000
@@ -113,13 +113,14 @@
job=MatchesStructure.byEquality(status=JobStatus.COMPLETED),
error_message=Is(None),
builds=AfterPreprocessing(set, MatchesSetwise(*[
- MatchesStructure.byEquality(
- requester=snap.registrant,
- snap=snap,
- archive=distroseries.main_archive,
- distro_arch_series=distroseries[arch],
- pocket=PackagePublishingPocket.RELEASE,
- channels={"core": "stable"})
+ MatchesStructure(
+ build_request=MatchesStructure.byEquality(id=job.job.id),
+ requester=Equals(snap.registrant),
+ snap=Equals(snap),
+ archive=Equals(distroseries.main_archive),
+ distro_arch_series=Equals(distroseries[arch]),
+ pocket=Equals(PackagePublishingPocket.RELEASE),
+ channels=Equals({"core": "stable"}))
for arch in ("avr2001", "x32")]))))
def test_run_failed(self):
Follow ups
