launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #23766
[Merge] lp:~cjwatson/launchpad/snap-request-build-explicit-arch-consistency into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-request-build-explicit-arch-consistency into lp:launchpad.
Commit message:
Use build request jobs for all snap build requests in the web UI.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-request-build-explicit-arch-consistency/+merge/369162
Previously build request jobs were only used when people didn't explicitly select architectures, which was very confusing since it meant bases weren't honoured properly.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-request-build-explicit-arch-consistency into lp:launchpad.
=== modified file 'lib/lp/snappy/browser/snap.py'
--- lib/lp/snappy/browser/snap.py 2019-05-16 10:21:14 +0000
+++ lib/lp/snappy/browser/snap.py 2019-06-21 11:37:21 +0000
@@ -57,7 +57,6 @@
from lp.registry.enums import VCSType
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.services.features import getFeatureFlag
-from lp.services.helpers import english_list
from lp.services.propertycache import cachedproperty
from lp.services.scripts import log
from lp.services.utils import seconds_since_epoch
@@ -92,7 +91,6 @@
MissingSnapcraftYaml,
NoSuchSnap,
SNAP_PRIVATE_FEATURE_FLAG,
- SnapBuildAlreadyPending,
SnapPrivateFeatureDisabled,
)
from lp.snappy.interfaces.snapbuild import ISnapBuildSet
@@ -256,20 +254,6 @@
return items
-def new_builds_notification_text(builds, already_pending=None):
- nr_builds = len(builds)
- if not nr_builds:
- builds_text = "All requested builds are already queued."
- elif nr_builds == 1:
- builds_text = "1 new build has been queued."
- else:
- builds_text = "%d new builds have been queued." % nr_builds
- if nr_builds and already_pending:
- return structured("<p>%s</p><p>%s</p>", builds_text, already_pending)
- else:
- return builds_text
-
-
class SnapRequestBuildsView(LaunchpadFormView):
"""A view for requesting builds of a snap package."""
@@ -328,45 +312,18 @@
'channels': self.context.auto_build_channels,
}
- def requestBuild(self, data):
- """User action for requesting a number of builds.
-
- We raise exceptions for most errors, but if there's already a
- pending build for a particular architecture, we simply record that
- so that other builds can be queued and a message displayed to the
- caller.
- """
- informational = {}
- builds = []
- already_pending = []
- for arch in data['distro_arch_series']:
- try:
- build = self.context.requestBuild(
- self.user, data['archive'], arch, data['pocket'],
- channels=data['channels'])
- builds.append(build)
- except SnapBuildAlreadyPending:
- already_pending.append(arch)
- if already_pending:
- informational['already_pending'] = (
- "An identical build is already pending for %s." %
- english_list(arch.architecturetag for arch in already_pending))
- return builds, informational
-
@action('Request builds', name='request')
def request_action(self, action, data):
if data.get('distro_arch_series', []):
- builds, informational = self.requestBuild(data)
- already_pending = informational.get('already_pending')
- notification_text = new_builds_notification_text(
- builds, already_pending)
- self.request.response.addNotification(notification_text)
+ architectures = [
+ arch.architecturetag for arch in data['distro_arch_series']]
else:
- self.context.requestBuilds(
- self.user, data['archive'], data['pocket'],
- channels=data['channels'])
- self.request.response.addNotification(
- _('Builds will be dispatched soon.'))
+ architectures = None
+ self.context.requestBuilds(
+ self.user, data['archive'], data['pocket'],
+ architectures=architectures, channels=data['channels'])
+ self.request.response.addNotification(
+ _('Builds will be dispatched soon.'))
self.next_url = self.cancel_url
=== modified file 'lib/lp/snappy/browser/tests/test_snap.py'
--- lib/lp/snappy/browser/tests/test_snap.py 2019-06-11 09:39:29 +0000
+++ lib/lp/snappy/browser/tests/test_snap.py 2019-06-21 11:37:21 +0000
@@ -29,6 +29,7 @@
AfterPreprocessing,
Equals,
Is,
+ MatchesDict,
MatchesListwise,
MatchesSetwise,
MatchesStructure,
@@ -1690,8 +1691,8 @@
Unauthorized, self.getViewBrowser, self.snap, "+request-builds")
def test_request_builds_with_architectures_action(self):
- # Requesting a build with architectures selected creates pending
- # builds.
+ # Requesting a build with architectures selected creates a pending
+ # build request limited to those architectures.
browser = self.getViewBrowser(
self.snap, "+request-builds", user=self.person)
browser.getControl("amd64").selected = True
@@ -1699,21 +1700,24 @@
browser.getControl("Request builds").click()
login_person(self.person)
- builds = self.snap.pending_builds
- self.assertContentEqual(
- [self.ubuntu.main_archive], set(build.archive for build in builds))
- self.assertContentEqual(
- ["amd64", "i386"],
- [build.distro_arch_series.architecturetag for build in builds])
- self.assertContentEqual(
- [PackagePublishingPocket.UPDATES],
- set(build.pocket for build in builds))
- self.assertContentEqual(
- [2510], set(build.buildqueue_record.lastscore for build in builds))
+ [request] = self.snap.pending_build_requests
+ self.assertThat(removeSecurityProxy(request), MatchesStructure(
+ snap=Equals(self.snap),
+ status=Equals(SnapBuildRequestStatus.PENDING),
+ error_message=Is(None),
+ builds=AfterPreprocessing(list, Equals([])),
+ archive=Equals(self.ubuntu.main_archive),
+ _job=MatchesStructure(
+ requester=Equals(self.person),
+ pocket=Equals(PackagePublishingPocket.UPDATES),
+ channels=Equals({}),
+ architectures=MatchesSetwise(Equals("amd64"), Equals("i386")),
+ )))
def test_request_builds_with_architectures_ppa(self):
# Selecting a different archive with architectures selected creates
- # builds in that archive.
+ # a build request targeting that archive and limited to those
+ # architectures.
ppa = self.factory.makeArchive(
distribution=self.ubuntu, owner=self.person, name="snap-ppa")
browser = self.getViewBrowser(
@@ -1725,12 +1729,15 @@
browser.getControl("Request builds").click()
login_person(self.person)
- builds = self.snap.pending_builds
- self.assertEqual([ppa], [build.archive for build in builds])
+ [request] = self.snap.pending_build_requests
+ self.assertThat(request, MatchesStructure(
+ archive=Equals(ppa),
+ architectures=MatchesSetwise(Equals("amd64"))))
def test_request_builds_with_architectures_channels(self):
- # Selecting different channels with architectures selected creates
- # builds using those channels.
+ # Selecting different channels with architectures selected creates a
+ # build request using those channels and limited to those
+ # architectures.
browser = self.getViewBrowser(
self.snap, "+request-builds", user=self.person)
browser.getControl(name="field.channels.core").value = "edge"
@@ -1739,25 +1746,10 @@
browser.getControl("Request builds").click()
login_person(self.person)
- builds = self.snap.pending_builds
- self.assertEqual(
- [{"core": "edge"}], [build.channels for build in builds])
-
- def test_request_builds_with_architectures_rejects_duplicate(self):
- # A duplicate build request with architectures selected causes a
- # notification.
- self.snap.requestBuild(
- self.person, self.ubuntu.main_archive, self.distroseries["amd64"],
- PackagePublishingPocket.UPDATES)
- browser = self.getViewBrowser(
- self.snap, "+request-builds", user=self.person)
- browser.getControl("amd64").selected = True
- browser.getControl("i386").selected = True
- browser.getControl("Request builds").click()
- main_text = extract_text(find_main_content(browser.contents))
- self.assertIn("1 new build has been queued.", main_text)
- self.assertIn(
- "An identical build is already pending for amd64.", main_text)
+ [request] = self.snap.pending_build_requests
+ self.assertThat(request, MatchesStructure(
+ channels=MatchesDict({"core": Equals("edge")}),
+ architectures=MatchesSetwise(Equals("amd64"))))
def test_request_builds_no_architectures_action(self):
# Requesting a build with no architectures selected creates a
@@ -1779,7 +1771,8 @@
_job=MatchesStructure(
requester=Equals(self.person),
pocket=Equals(PackagePublishingPocket.UPDATES),
- channels=Equals({}))))
+ channels=Equals({}),
+ architectures=Is(None))))
def test_request_builds_no_architectures_ppa(self):
# Selecting a different archive with no architectures selected
@@ -1796,7 +1789,9 @@
login_person(self.person)
[request] = self.snap.pending_build_requests
- self.assertEqual(ppa, request.archive)
+ self.assertThat(request, MatchesStructure(
+ archive=Equals(ppa),
+ architectures=Is(None)))
def test_request_builds_no_architectures_channels(self):
# Selecting different channels with no architectures selected
=== modified file 'lib/lp/snappy/interfaces/snap.py'
--- lib/lp/snappy/interfaces/snap.py 2019-05-22 18:33:10 +0000
+++ lib/lp/snappy/interfaces/snap.py 2019-06-21 11:37:21 +0000
@@ -76,6 +76,7 @@
Dict,
Int,
List,
+ Set,
Text,
TextLine,
)
@@ -336,6 +337,10 @@
title=_("Source snap channels for builds produced by this request"),
key_type=TextLine(), required=False, readonly=True)
+ architectures = Set(
+ title=_("If set, this request is limited to these architecture tags"),
+ value_type=TextLine(), required=False, readonly=True)
+
class ISnapView(Interface):
"""`ISnap` attributes that require launchpad.View permission."""
@@ -437,8 +442,9 @@
"""
def requestBuildsFromJob(requester, archive, pocket, channels=None,
- allow_failures=False, fetch_snapcraft_yaml=True,
- build_request=None, logger=None):
+ architectures=None, allow_failures=False,
+ fetch_snapcraft_yaml=True, build_request=None,
+ logger=None):
"""Synchronous part of `Snap.requestBuilds`.
Request that the snap package be built for relevant architectures.
@@ -448,6 +454,9 @@
:param pocket: The pocket that should be targeted.
:param channels: A dictionary mapping snap names to channels to use
for these builds.
+ :param architectures: If not None, limit builds to architectures
+ with these architecture tags (in addition to any other
+ applicable constraints).
:param allow_failures: If True, log exceptions other than "already
pending" from individual build requests; if False, raise them to
the caller.
=== modified file 'lib/lp/snappy/interfaces/snapjob.py'
--- lib/lp/snappy/interfaces/snapjob.py 2019-05-22 18:33:10 +0000
+++ lib/lp/snappy/interfaces/snapjob.py 2019-06-21 11:37:21 +0000
@@ -22,6 +22,7 @@
Datetime,
Dict,
List,
+ Set,
TextLine,
)
@@ -78,6 +79,10 @@
"are supported."),
key_type=TextLine(), required=False, readonly=True)
+ architectures = Set(
+ title=_("If set, limit builds to these architecture tags."),
+ value_type=TextLine(), required=False, readonly=True)
+
date_created = Datetime(
title=_("Time when this job was created."),
required=True, readonly=True)
@@ -101,7 +106,7 @@
class ISnapRequestBuildsJobSource(IJobSource):
- def create(snap, requester, archive, pocket, channels):
+ def create(snap, requester, archive, pocket, channels, architectures=None):
"""Request builds of a snap package.
:param snap: The snap package to build.
@@ -110,6 +115,9 @@
:param pocket: The pocket that should be targeted.
:param channels: A dictionary mapping snap names to channels to use
for these builds.
+ :param architectures: If not None, limit builds to architectures
+ with these architecture tags (in addition to any other
+ applicable constraints).
"""
def findBySnap(snap, statuses=None, job_ids=None):
=== modified file 'lib/lp/snappy/model/snap.py'
--- lib/lp/snappy/model/snap.py 2019-05-16 10:21:14 +0000
+++ lib/lp/snappy/model/snap.py 2019-06-21 11:37:21 +0000
@@ -249,6 +249,11 @@
"""See `ISnapBuildRequest`."""
return self._job.channels
+ @property
+ def architectures(self):
+ """See `ISnapBuildRequest`."""
+ return self._job.architectures
+
@implementer(ISnap, IHasOwner)
class Snap(Storm, WebhookTargetMixin):
@@ -655,11 +660,13 @@
notify(ObjectCreatedEvent(build, user=requester))
return build
- def requestBuilds(self, requester, archive, pocket, channels=None):
+ def requestBuilds(self, requester, archive, pocket, channels=None,
+ architectures=None):
"""See `ISnap`."""
self._checkRequestBuild(requester, archive)
job = getUtility(ISnapRequestBuildsJobSource).create(
- self, requester, archive, pocket, channels)
+ self, requester, archive, pocket, channels,
+ architectures=architectures)
return self.getBuildRequest(job.job_id)
@staticmethod
@@ -694,7 +701,8 @@
else:
return channels
- def requestBuildsFromJob(self, requester, archive, pocket, channels=None,
+ def requestBuildsFromJob(self, requester, archive, pocket,
+ channels=None, architectures=None,
allow_failures=False, fetch_snapcraft_yaml=True,
build_request=None, logger=None):
"""See `ISnap`."""
@@ -731,7 +739,9 @@
supported_arches = OrderedDict(
(das.architecturetag, das) for das in sorted(
self.getAllowedArchitectures(distro_series),
- key=attrgetter("processor.id")))
+ key=attrgetter("processor.id"))
+ if (architectures is None or
+ das.architecturetag in architectures))
architectures_to_build = determine_architectures_to_build(
snapcraft_data, supported_arches.keys())
except Exception as e:
=== modified file 'lib/lp/snappy/model/snapjob.py'
--- lib/lp/snappy/model/snapjob.py 2018-10-09 09:25:19 +0000
+++ lib/lp/snappy/model/snapjob.py 2019-06-21 11:37:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2018 Canonical Ltd. This software is licensed under the
+# Copyright 2018-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Snap package jobs."""
@@ -184,13 +184,18 @@
config = config.ISnapRequestBuildsJobSource
@classmethod
- def create(cls, snap, requester, archive, pocket, channels):
+ def create(cls, snap, requester, archive, pocket, channels,
+ architectures=None):
"""See `ISnapRequestBuildsJobSource`."""
metadata = {
"requester": requester.id,
"archive": archive.id,
"pocket": pocket.value,
"channels": channels,
+ # Really a set or None, but sets aren't directly
+ # JSON-serialisable.
+ "architectures": (
+ list(architectures) if architectures is not None else None),
}
snap_job = SnapJob(snap, cls.class_job_type, metadata)
job = cls(snap_job)
@@ -292,6 +297,12 @@
return self.metadata["channels"]
@property
+ def architectures(self):
+ """See `ISnapRequestBuildsJob`."""
+ architectures = self.metadata["architectures"]
+ return set(architectures) if architectures is not None else None
+
+ @property
def date_created(self):
"""See `ISnapRequestBuildsJob`."""
return self.context.job.date_created
@@ -346,6 +357,7 @@
try:
self.builds = self.snap.requestBuildsFromJob(
requester, archive, self.pocket, channels=self.channels,
+ architectures=self.architectures,
build_request=self.build_request, logger=log)
self.error_message = None
except self.retry_error_types:
=== modified file 'lib/lp/snappy/tests/test_snap.py'
--- lib/lp/snappy/tests/test_snap.py 2019-06-20 13:16:35 +0000
+++ lib/lp/snappy/tests/test_snap.py 2019-06-21 11:37:21 +0000
@@ -475,7 +475,9 @@
status=Equals(SnapBuildRequestStatus.PENDING),
error_message=Is(None),
builds=AfterPreprocessing(set, MatchesSetwise()),
- archive=Equals(snap.distro_series.main_archive)))
+ archive=Equals(snap.distro_series.main_archive),
+ channels=MatchesDict({"snapcraft": Equals("edge")}),
+ architectures=Is(None)))
[job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
self.assertThat(job, MatchesStructure(
job_id=Equals(request.id),
@@ -484,7 +486,8 @@
requester=Equals(snap.owner.teamowner),
archive=Equals(snap.distro_series.main_archive),
pocket=Equals(PackagePublishingPocket.UPDATES),
- channels=Equals({"snapcraft": "edge"})))
+ channels=Equals({"snapcraft": "edge"}),
+ architectures=Is(None)))
def test_requestBuilds_without_distroseries(self):
# requestBuilds schedules a job for a snap without a distroseries.
@@ -502,16 +505,51 @@
status=Equals(SnapBuildRequestStatus.PENDING),
error_message=Is(None),
builds=AfterPreprocessing(set, MatchesSetwise()),
- archive=Equals(archive)))
- [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
- self.assertThat(job, MatchesStructure(
- job_id=Equals(request.id),
- job=MatchesStructure.byEquality(status=JobStatus.WAITING),
- snap=Equals(snap),
- requester=Equals(snap.owner.teamowner),
- archive=Equals(archive),
- pocket=Equals(PackagePublishingPocket.UPDATES),
- channels=Equals({"snapcraft": "edge"})))
+ archive=Equals(archive),
+ channels=MatchesDict({"snapcraft": Equals("edge")}),
+ architectures=Is(None)))
+ [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
+ self.assertThat(job, MatchesStructure(
+ job_id=Equals(request.id),
+ job=MatchesStructure.byEquality(status=JobStatus.WAITING),
+ snap=Equals(snap),
+ requester=Equals(snap.owner.teamowner),
+ archive=Equals(archive),
+ pocket=Equals(PackagePublishingPocket.UPDATES),
+ channels=Equals({"snapcraft": "edge"}),
+ architectures=Is(None)))
+
+ def test_requestBuilds_with_architectures(self):
+ # If asked to build for particular architectures, requestBuilds
+ # passes those through to the job.
+ snap = self.factory.makeSnap()
+ now = get_transaction_timestamp(IStore(snap))
+ with person_logged_in(snap.owner.teamowner):
+ request = snap.requestBuilds(
+ snap.owner.teamowner, snap.distro_series.main_archive,
+ PackagePublishingPocket.UPDATES,
+ channels={"snapcraft": "edge"},
+ architectures={"amd64", "i386"})
+ self.assertThat(request, MatchesStructure(
+ date_requested=Equals(now),
+ date_finished=Is(None),
+ snap=Equals(snap),
+ status=Equals(SnapBuildRequestStatus.PENDING),
+ error_message=Is(None),
+ builds=AfterPreprocessing(set, MatchesSetwise()),
+ archive=Equals(snap.distro_series.main_archive),
+ channels=MatchesDict({"snapcraft": Equals("edge")}),
+ architectures=MatchesSetwise(Equals("amd64"), Equals("i386"))))
+ [job] = getUtility(ISnapRequestBuildsJobSource).iterReady()
+ self.assertThat(job, MatchesStructure(
+ job_id=Equals(request.id),
+ job=MatchesStructure.byEquality(status=JobStatus.WAITING),
+ snap=Equals(snap),
+ requester=Equals(snap.owner.teamowner),
+ archive=Equals(snap.distro_series.main_archive),
+ pocket=Equals(PackagePublishingPocket.UPDATES),
+ channels=Equals({"snapcraft": "edge"}),
+ architectures=MatchesSetwise(Equals("amd64"), Equals("i386"))))
def test__findBase(self):
snap_base_set = getUtility(ISnapBaseSet)
@@ -585,7 +623,7 @@
with person_logged_in(job.requester):
builds = job.snap.requestBuildsFromJob(
job.requester, job.archive, job.pocket,
- removeSecurityProxy(job.channels),
+ channels=removeSecurityProxy(job.channels),
build_request=job.build_request)
self.assertRequestedBuildsMatch(builds, job, ["sparc"], job.channels)
@@ -601,11 +639,29 @@
with person_logged_in(job.requester):
builds = job.snap.requestBuildsFromJob(
job.requester, job.archive, job.pocket,
- removeSecurityProxy(job.channels),
+ channels=removeSecurityProxy(job.channels),
build_request=job.build_request)
self.assertRequestedBuildsMatch(
builds, job, ["mips64el", "riscv64"], job.channels)
+ def test_requestBuildsFromJob_architectures_parameter(self):
+ # If an explicit set of architectures was given as a parameter,
+ # requestBuildsFromJob intersects those with any other constraints
+ # when requesting builds.
+ self.useFixture(GitHostingFixture(blob="name: foo\n"))
+ job = self.makeRequestBuildsJob(["avr", "mips64el", "riscv64"])
+ self.assertEqual(
+ get_transaction_timestamp(IStore(job.snap)), job.date_created)
+ transaction.commit()
+ with person_logged_in(job.requester):
+ builds = job.snap.requestBuildsFromJob(
+ job.requester, job.archive, job.pocket,
+ channels=removeSecurityProxy(job.channels),
+ architectures={"avr", "riscv64"},
+ build_request=job.build_request)
+ self.assertRequestedBuildsMatch(
+ builds, job, ["avr", "riscv64"], job.channels)
+
def test_requestBuildsFromJob_no_distroseries_explicit_base(self):
# If the snap doesn't specify a distroseries but has an explicit
# base, requestBuildsFromJob requests builds for the appropriate
=== modified file 'lib/lp/snappy/tests/test_snapjob.py'
--- lib/lp/snappy/tests/test_snapjob.py 2018-09-10 11:18:42 +0000
+++ lib/lp/snappy/tests/test_snapjob.py 2019-06-21 11:37:21 +0000
@@ -1,4 +1,4 @@
-# Copyright 2018 Canonical Ltd. This software is licensed under the
+# Copyright 2018-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for snap package jobs."""
@@ -133,6 +133,46 @@
channels=Equals({"core": "stable"}))
for arch in ("avr2001", "x32")]))))
+ def test_run_with_architectures(self):
+ # If the user explicitly requested architectures, the job passes
+ # those through when requesting builds, intersecting them with other
+ # constraints.
+ distroseries, processors = self.makeSeriesAndProcessors(
+ ["avr2001", "sparc64", "x32"])
+ [git_ref] = self.factory.makeGitRefs()
+ snap = self.factory.makeSnap(
+ git_ref=git_ref, distroseries=distroseries, processors=processors)
+ expected_date_created = get_transaction_timestamp(IStore(snap))
+ job = SnapRequestBuildsJob.create(
+ snap, snap.registrant, distroseries.main_archive,
+ PackagePublishingPocket.RELEASE, {"core": "stable"},
+ architectures=["sparc64", "x32"])
+ snapcraft_yaml = dedent("""\
+ architectures:
+ - build-on: avr2001
+ - build-on: x32
+ """)
+ self.useFixture(GitHostingFixture(blob=snapcraft_yaml))
+ with dbuser(config.ISnapRequestBuildsJobSource.dbuser):
+ JobRunner([job]).runAll()
+ now = get_transaction_timestamp(IStore(snap))
+ self.assertEmailQueueLength(0)
+ self.assertThat(job, MatchesStructure(
+ job=MatchesStructure.byEquality(status=JobStatus.COMPLETED),
+ date_created=Equals(expected_date_created),
+ date_finished=MatchesAll(
+ GreaterThan(expected_date_created), LessThan(now)),
+ error_message=Is(None),
+ builds=AfterPreprocessing(set, MatchesSetwise(
+ 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["x32"]),
+ pocket=Equals(PackagePublishingPocket.RELEASE),
+ channels=Equals({"core": "stable"}))))))
+
def test_run_failed(self):
# A failed run sets the job status to FAILED and records the error
# message.
Follow ups
