Merge lp:~cjwatson/launchpad/snap-auto-builds-oops into lp:launchpad

Proposed by Colin Watson on 2018-08-14
Status: Merged
Merged at revision: 18751
Proposed branch: lp:~cjwatson/launchpad/snap-auto-builds-oops
Merge into: lp:launchpad
Diff against target: 103 lines (+59/-22)
2 files modified
lib/lp/snappy/model/snap.py (+29/-22)
lib/lp/snappy/tests/test_snap.py (+30/-0)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-auto-builds-oops
Reviewer Review Type Date Requested Status
William Grant code 2018-08-14 Approve on 2018-08-16
Review via email: mp+353094@code.launchpad.net

Commit message

Tolerate failures to get snapcraft.yaml in SnapSet.makeAutoBuilds.

Description of the change

We already handled failures to request builds for each individual architecture, but not failure to work out which architectures to build.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/snappy/model/snap.py'
2--- lib/lp/snappy/model/snap.py 2018-08-06 14:42:30 +0000
3+++ lib/lp/snappy/model/snap.py 2018-08-14 14:42:38 +0000
4@@ -565,29 +565,36 @@
5 allow_failures=False, fetch_snapcraft_yaml=True,
6 logger=None):
7 """See `ISnap`."""
8- if fetch_snapcraft_yaml:
9- try:
10- snapcraft_data = removeSecurityProxy(
11- getUtility(ISnapSet).getSnapcraftYaml(self))
12- except CannotFetchSnapcraftYaml as e:
13- if not e.unsupported_remote:
14- raise
15- # The only reason we can't fetch the file is because we
16- # don't support fetching from this repository's host. In
17- # this case the best thing is to fall back to building for
18- # all supported architectures.
19+ try:
20+ if fetch_snapcraft_yaml:
21+ try:
22+ snapcraft_data = removeSecurityProxy(
23+ getUtility(ISnapSet).getSnapcraftYaml(self))
24+ except CannotFetchSnapcraftYaml as e:
25+ if not e.unsupported_remote:
26+ raise
27+ # The only reason we can't fetch the file is because we
28+ # don't support fetching from this repository's host.
29+ # In this case the best thing is to fall back to
30+ # building for all supported architectures.
31+ snapcraft_data = {}
32+ else:
33 snapcraft_data = {}
34- else:
35- snapcraft_data = {}
36- # Sort by Processor.id for determinism. This is chosen to be the
37- # same order as in BinaryPackageBuildSet.createForSource, to
38- # minimise confusion.
39- supported_arches = OrderedDict(
40- (das.architecturetag, das) for das in sorted(
41- self.getAllowedArchitectures(),
42- key=attrgetter("processor.id")))
43- architectures_to_build = determine_architectures_to_build(
44- snapcraft_data, supported_arches.keys())
45+ # Sort by Processor.id for determinism. This is chosen to be
46+ # the same order as in BinaryPackageBuildSet.createForSource, to
47+ # minimise confusion.
48+ supported_arches = OrderedDict(
49+ (das.architecturetag, das) for das in sorted(
50+ self.getAllowedArchitectures(),
51+ key=attrgetter("processor.id")))
52+ architectures_to_build = determine_architectures_to_build(
53+ snapcraft_data, supported_arches.keys())
54+ except Exception as e:
55+ if not allow_failures:
56+ raise
57+ elif logger is not None:
58+ logger.exception(" - %s/%s: %s", self.owner.name, self.name, e)
59+ return []
60
61 builds = []
62 for build_instance in architectures_to_build:
63
64=== modified file 'lib/lp/snappy/tests/test_snap.py'
65--- lib/lp/snappy/tests/test_snap.py 2018-08-06 14:42:30 +0000
66+++ lib/lp/snappy/tests/test_snap.py 2018-08-14 14:42:38 +0000
67@@ -1441,6 +1441,36 @@
68 self.assertEqual(
69 expected_log_entries, logger.getLogBuffer().splitlines())
70
71+ def test_makeAutoBuilds_tolerates_failures(self):
72+ # If requesting builds of one snap fails, ISnapSet.makeAutoBuilds
73+ # just logs the problem and carries on to the next.
74+ das1, snap1 = self.makeAutoBuildableSnap(is_stale=True)
75+ das2, snap2 = self.makeAutoBuildableSnap(is_stale=True)
76+ logger = BufferLogger()
77+ self.useFixture(GitHostingFixture()).getBlob.failure = (
78+ GitRepositoryBlobNotFound("dummy", "snap/snapcraft.yaml"))
79+ self.assertEqual(
80+ [], getUtility(ISnapSet).makeAutoBuilds(logger=logger))
81+ # The u'...' here is a little odd, but that's how KeyError (and thus
82+ # NotFoundError) stringifies.
83+ expected_log_entries = [
84+ "DEBUG Scheduling builds of snap package %s/%s" % (
85+ snap1.owner.name, snap1.name),
86+ "ERROR - %s/%s: u'Cannot find snapcraft.yaml in %s'" % (
87+ snap1.owner.name, snap1.name, snap1.git_ref.unique_name),
88+ "DEBUG Scheduling builds of snap package %s/%s" % (
89+ snap2.owner.name, snap2.name),
90+ "ERROR - %s/%s: u'Cannot find snapcraft.yaml in %s'" % (
91+ snap2.owner.name, snap2.name, snap2.git_ref.unique_name),
92+ ]
93+ self.assertThat(
94+ logger.getLogBuffer().splitlines(),
95+ # Ignore ordering differences, since ISnapSet.makeAutoBuilds
96+ # might process the two snaps in either order.
97+ MatchesSetwise(*(Equals(entry) for entry in expected_log_entries)))
98+ self.assertFalse(snap1.is_stale)
99+ self.assertFalse(snap2.is_stale)
100+
101 def test_makeAutoBuilds_with_older_build(self):
102 # If a previous build is not recent and the snap package is stale,
103 # ISnapSet.makeAutoBuilds requests builds.