Merge ~cjwatson/launchpad:snap-improve-logging into launchpad:master

Proposed by Colin Watson on 2019-10-09
Status: Merged
Approved by: Colin Watson on 2019-10-10
Approved revision: 281a5873ecd563de3f3d638f6734f77def63995d
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:snap-improve-logging
Merge into: launchpad:master
Diff against target: 105 lines (+41/-4)
3 files modified
lib/lp/snappy/model/snapbuild.py (+4/-0)
lib/lp/snappy/subscribers/snapbuild.py (+10/-4)
lib/lp/snappy/tests/test_snapbuild.py (+27/-0)
Reviewer Review Type Date Requested Status
Tom Wardill 2019-10-09 Approve on 2019-10-10
Review via email: mp+373915@code.launchpad.net

Commit message

Improve logging of snap store upload scheduling

It's helpful to log the decision in snap_build_status_changed on whether
to schedule a store upload, since otherwise things are a bit invisible
when debugging user-reported problems.

To post a comment you must log in.
Tom Wardill (twom) wrote :

Looks like an improvement to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
2index 0415c66..6971e83 100644
3--- a/lib/lp/snappy/model/snapbuild.py
4+++ b/lib/lp/snappy/model/snapbuild.py
5@@ -222,6 +222,10 @@ class SnapBuild(PackageBuildMixin, Storm):
6 self.archive.private
7 )
8
9+ def __repr__(self):
10+ return "<SnapBuild ~%s/+snap/%s/+build/%d>" % (
11+ self.snap.owner.name, self.snap.name, self.id)
12+
13 @property
14 def title(self):
15 das = self.distro_arch_series
16diff --git a/lib/lp/snappy/subscribers/snapbuild.py b/lib/lp/snappy/subscribers/snapbuild.py
17index 86196cd..0eaa0dc 100644
18--- a/lib/lp/snappy/subscribers/snapbuild.py
19+++ b/lib/lp/snappy/subscribers/snapbuild.py
20@@ -1,4 +1,4 @@
21-# Copyright 2016-2018 Canonical Ltd. This software is licensed under the
22+# Copyright 2016-2019 Canonical Ltd. This software is licensed under the
23 # GNU Affero General Public License version 3 (see the file LICENSE).
24
25 """Event subscribers for snap builds."""
26@@ -11,6 +11,7 @@ from zope.component import getUtility
27
28 from lp.buildmaster.enums import BuildStatus
29 from lp.services.features import getFeatureFlag
30+from lp.services.scripts import log
31 from lp.services.webapp.publisher import canonical_url
32 from lp.services.webhooks.interfaces import IWebhookSet
33 from lp.services.webhooks.payload import compose_webhook_payload
34@@ -41,9 +42,14 @@ def snap_build_status_changed(snapbuild, event):
35 """Trigger events when snap package build statuses change."""
36 _trigger_snap_build_webhook(snapbuild, "status-changed")
37
38- if (snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload and
39- snapbuild.status == BuildStatus.FULLYBUILT):
40- getUtility(ISnapStoreUploadJobSource).create(snapbuild)
41+ if snapbuild.status == BuildStatus.FULLYBUILT:
42+ if snapbuild.snap.can_upload_to_store and snapbuild.snap.store_upload:
43+ log.info("Scheduling upload of %r to the store." % snapbuild)
44+ getUtility(ISnapStoreUploadJobSource).create(snapbuild)
45+ else:
46+ log.info(
47+ "%r is not configured for upload to the store." %
48+ snapbuild.snap)
49
50
51 def snap_build_store_upload_status_changed(snapbuild, event):
52diff --git a/lib/lp/snappy/tests/test_snapbuild.py b/lib/lp/snappy/tests/test_snapbuild.py
53index 1c28c20..aa69ae3 100644
54--- a/lib/lp/snappy/tests/test_snapbuild.py
55+++ b/lib/lp/snappy/tests/test_snapbuild.py
56@@ -113,6 +113,14 @@ class TestSnapBuild(TestCaseWithFactory):
57 self.assertProvides(self.build, IPackageBuild)
58 self.assertProvides(self.build, ISnapBuild)
59
60+ def test___repr__(self):
61+ # SnapBuild has an informative __repr__.
62+ self.assertEqual(
63+ "<SnapBuild ~%s/+snap/%s/+build/%s>" % (
64+ self.build.snap.owner.name, self.build.snap.name,
65+ self.build.id),
66+ repr(self.build))
67+
68 def test_title(self):
69 # SnapBuild has an informative title.
70 das = self.build.distro_arch_series
71@@ -339,8 +347,21 @@ class TestSnapBuild(TestCaseWithFactory):
72 self.build.updateStatus(BuildStatus.FAILEDTOBUILD)
73 self.assertContentEqual([], self.build.store_upload_jobs)
74
75+ def test_updateStatus_fullybuilt_not_configured(self):
76+ # A completed SnapBuild does not trigger store uploads if the snap
77+ # is not properly configured for that.
78+ logger = self.useFixture(FakeLogger())
79+ with dbuser(config.builddmaster.dbuser):
80+ self.build.updateStatus(BuildStatus.FULLYBUILT)
81+ self.assertEqual(0, len(list(self.build.store_upload_jobs)))
82+ self.assertIn(
83+ "<Snap ~%s/+snap/%s> is not configured for upload to the "
84+ "store." % (self.build.snap.owner.name, self.build.snap.name),
85+ logger.output.splitlines())
86+
87 def test_updateStatus_fullybuilt_triggers_store_uploads(self):
88 # A completed SnapBuild triggers store uploads.
89+ logger = self.useFixture(FakeLogger())
90 self.build.snap.store_series = self.factory.makeSnappySeries()
91 self.build.snap.store_name = self.factory.getUniqueUnicode()
92 self.build.snap.store_upload = True
93@@ -348,6 +369,12 @@ class TestSnapBuild(TestCaseWithFactory):
94 with dbuser(config.builddmaster.dbuser):
95 self.build.updateStatus(BuildStatus.FULLYBUILT)
96 self.assertEqual(1, len(list(self.build.store_upload_jobs)))
97+ self.assertIn(
98+ "Scheduling upload of <SnapBuild ~%s/+snap/%s/+build/%d> to the "
99+ "store." % (
100+ self.build.snap.owner.name, self.build.snap.name,
101+ self.build.id),
102+ logger.output.splitlines())
103
104 def test_notify_fullybuilt(self):
105 # notify does not send mail when a SnapBuild completes normally.

Subscribers

People subscribed via source and target branches