Merge ~ilasc/launchpad:add-get-snapbuild-by-revision into launchpad:master

Proposed by Ioana Lasc
Status: Merged
Approved by: Ioana Lasc
Approved revision: 54adb9f372ec8e62c9ef5cd34452cf5351c0d0e0
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ilasc/launchpad:add-get-snapbuild-by-revision
Merge into: launchpad:master
Prerequisite: ~ilasc/launchpad:populate-store-upload-revision
Diff against target: 333 lines (+123/-36)
6 files modified
lib/lp/scripts/tests/test_garbo.py (+18/-36)
lib/lp/snappy/interfaces/snap.py (+15/-0)
lib/lp/snappy/model/snap.py (+14/-0)
lib/lp/snappy/model/snapbuildjob.py (+1/-0)
lib/lp/snappy/tests/test_snap.py (+49/-0)
lib/lp/snappy/tests/test_snapbuildjob.py (+26/-0)
Reviewer Review Type Date Requested Status
Colin Watson Approve
Review via email: mp+407781@code.launchpad.net

Commit message

Add getBuildByStoreRevision to Snap

Description of the change

This needs the new column "_store_upload_revision" to be added to SnapBuild and populated (prerequisite branch: populate-store-upload-revision).

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

As I noted in comments on the prerequisite MP, it seems likely that you'll need to adjust garbo tests here.

review: Needs Fixing
5d86f07... by Ioana Lasc

Merge branch 'populate-store-upload-revision' into add-get-snapbuild-by-revision

df6797e... by Ioana Lasc

Address code review comments

Revision history for this message
Colin Watson (cjwatson) :
review: Approve
54adb9f... by Ioana Lasc

Remove MP number from comment

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
2index 3a43045..5fb7f77 100644
3--- a/lib/lp/scripts/tests/test_garbo.py
4+++ b/lib/lp/scripts/tests/test_garbo.py
5@@ -2013,52 +2013,34 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
6 with dbuser(config.ISnapStoreUploadJobSource.dbuser):
7 run_isolated_jobs([job])
8
9- # this mimics what we have in the DB right now:
10- # uploaded snaps that do not have the new DB column
11- # _store_upload_revision populated yet
12- populator = PopulateSnapBuildStoreRevision(None)
13- filter = populator.findSnapBuilds()
14+ # The _store_upload_revision now gets populated when
15+ # uploading the build as part of the
16+ # `SnapStoreUploadJob.store_revision` property setter.
17+ # Assert that upload job above populated _store_upload_revision
18+ # for build1:
19 build1 = removeSecurityProxy(build1)
20- self.assertEqual(1, filter.count())
21- self.assertEqual(build1, filter.one())
22- self.assertEqual(build1._store_upload_revision, None)
23-
24- # run the garbo job and verify _store_upload_revision
25- # is not populated with the value assigned to the build during upload
26- self.runDaily()
27- switch_dbuser('testadmin')
28 self.assertEqual(build1._store_upload_revision, 1)
29
30- # Tests that of all builds for the same snap only those that have
31- # been uploaded to the store will get
32- # their new _store_upload_revision DB column updated
33- build2 = self.factory.makeSnapBuild(
34- snap=snap1,
35- status=BuildStatus.FULLYBUILT)
36- build3 = self.factory.makeSnapBuild(
37- snap=snap1,
38- status=BuildStatus.FULLYBUILT)
39- job = getUtility(ISnapStoreUploadJobSource).create(build2)
40- client = FakeSnapStoreClient()
41- client.upload.result = (
42- "http://sca.example/dev/api/snaps/1/builds/2/status")
43- client.checkStatus.result = (
44- "http://sca.example/dev/click-apps/1/rev/2/", 1)
45- self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
46- with dbuser(config.ISnapStoreUploadJobSource.dbuser):
47- run_isolated_jobs([job])
48+ # and that the populator job doesn't pick up build1 anymore.
49+ populator = PopulateSnapBuildStoreRevision(None)
50+ filter = populator.findSnapBuilds()
51+ self.assertEqual(0, filter.count())
52+
53+ # We manually simulate here the situation
54+ # where a job to upload snap to the store has run in the past
55+ # without populating the new column (prior to MP 407781).
56+ build1._store_upload_revision = None
57
58+ # in this case the populator garbo job should pick up build1
59+ # to update the _store_upload_revision for it:
60 populator = PopulateSnapBuildStoreRevision(None)
61 filter = populator.findSnapBuilds()
62 self.assertEqual(1, filter.count())
63- self.assertEqual(build2, filter.one())
64+ self.assertEqual(build1, filter.one())
65
66 self.runDaily()
67 switch_dbuser('testadmin')
68- build2 = removeSecurityProxy(build2)
69- self.assertEqual(build2._store_upload_revision, 1)
70- build3 = removeSecurityProxy(build3)
71- self.assertIsNone(build3._store_upload_revision)
72+ self.assertEqual(build1._store_upload_revision, 1)
73
74
75 class TestGarboTasks(TestCaseWithFactory):
76diff --git a/lib/lp/snappy/interfaces/snap.py b/lib/lp/snappy/interfaces/snap.py
77index 2445553..088af8c 100644
78--- a/lib/lp/snappy/interfaces/snap.py
79+++ b/lib/lp/snappy/interfaces/snap.py
80@@ -529,6 +529,21 @@ class ISnapView(Interface):
81
82 @call_with(user=REQUEST_USER)
83 @operation_parameters(
84+ store_upload_revision=Int(title="Store revision",
85+ required=True))
86+ @export_read_operation()
87+ @operation_for_version("devel")
88+ def getBuildByStoreRevision(store_upload_revision, user=None):
89+ """Returns the build (if any) of that snap recipe
90+ that has the given store_upload_revision.
91+
92+ :param store_upload_revision: The revision assigned by the store.
93+ :param user: The `IPerson` requesting this information.
94+ :return: An 'ISnapBuild' or None.
95+ """
96+
97+ @call_with(user=REQUEST_USER)
98+ @operation_parameters(
99 request_ids=List(
100 title=_("A list of snap build request IDs."), value_type=Int(),
101 required=False),
102diff --git a/lib/lp/snappy/model/snap.py b/lib/lp/snappy/model/snap.py
103index 24aa2be..e68182e 100644
104--- a/lib/lp/snappy/model/snap.py
105+++ b/lib/lp/snappy/model/snap.py
106@@ -1111,6 +1111,20 @@ class Snap(Storm, WebhookTargetMixin):
107
108 return result
109
110+ def getBuildByStoreRevision(self, store_upload_revision, user=None):
111+ build = Store.of(self).find(
112+ SnapBuild,
113+ SnapBuild.snap == self,
114+ SnapBuild._store_upload_revision == store_upload_revision,
115+ SnapBuild.archive == Archive.id,
116+ Archive._enabled,
117+ get_enabled_archive_filter(
118+ user,
119+ include_public=True,
120+ include_subscribed=True)
121+ ).one()
122+ return build
123+
124 @property
125 def builds(self):
126 """See `ISnap`."""
127diff --git a/lib/lp/snappy/model/snapbuildjob.py b/lib/lp/snappy/model/snapbuildjob.py
128index 9634f62..a8fc49f 100644
129--- a/lib/lp/snappy/model/snapbuildjob.py
130+++ b/lib/lp/snappy/model/snapbuildjob.py
131@@ -263,6 +263,7 @@ class SnapStoreUploadJob(SnapBuildJobDerived):
132 if self.snapbuild.store_upload_metadata is None:
133 self.snapbuild.store_upload_metadata = {}
134 self.snapbuild.store_upload_metadata["store_revision"] = revision
135+ self.snapbuild._store_upload_revision = revision
136
137 @property
138 def status_url(self):
139diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
140index 4d9c92f..adb8cfa 100644
141--- a/lib/lp/snappy/tests/test_snap.py
142+++ b/lib/lp/snappy/tests/test_snap.py
143@@ -125,6 +125,7 @@ from lp.snappy.interfaces.snapbase import (
144 from lp.snappy.interfaces.snapbuild import (
145 ISnapBuild,
146 ISnapBuildSet,
147+ SnapBuildStoreUploadStatus,
148 )
149 from lp.snappy.interfaces.snapbuildjob import ISnapStoreUploadJobSource
150 from lp.snappy.interfaces.snapjob import ISnapRequestBuildsJobSource
151@@ -137,6 +138,10 @@ from lp.snappy.model.snap import (
152 from lp.snappy.model.snapbuild import SnapFile
153 from lp.snappy.model.snapbuildjob import SnapBuildJob
154 from lp.snappy.model.snapjob import SnapJob
155+from lp.snappy.tests.test_snapbuildjob import (
156+ FakeSnapStoreClient,
157+ run_isolated_jobs,
158+ )
159 from lp.testing import (
160 admin_logged_in,
161 ANONYMOUS,
162@@ -1079,6 +1084,50 @@ class TestSnap(TestCaseWithFactory):
163 snap.destroySelf()
164 self.assertFalse(getUtility(ISnapSet).exists(owner, "condemned"))
165
166+ def test_getBuildByStoreRevision(self):
167+ snap1 = self.factory.makeSnap()
168+ build = self.factory.makeSnapBuild(
169+ snap=snap1,
170+ status=BuildStatus.FULLYBUILT)
171+
172+ # There is no build with revision 5 for snap1
173+ self.assertIsNone(snap1.getBuildByStoreRevision(5))
174+
175+ # Upload build1 and check we return it by version 1
176+ job = getUtility(ISnapStoreUploadJobSource).create(build)
177+ client = FakeSnapStoreClient()
178+ client.upload.result = (
179+ "http://sca.example/dev/api/snaps/1/builds/1/status")
180+ client.checkStatus.result = (
181+ "http://sca.example/dev/click-apps/1/rev/1/", 1)
182+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
183+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
184+ run_isolated_jobs([job])
185+ self.assertEqual(
186+ SnapBuildStoreUploadStatus.UPLOADED, build.store_upload_status)
187+ self.assertEqual(build.store_upload_revision, 1)
188+ self.assertEqual(snap1.getBuildByStoreRevision(1), build)
189+
190+ # build & upload again, check revision
191+ # and that we return the second build for revision 2
192+ build2 = self.factory.makeSnapBuild(
193+ snap=snap1,
194+ status=BuildStatus.FULLYBUILT)
195+ job = getUtility(ISnapStoreUploadJobSource).create(build2)
196+ client = FakeSnapStoreClient()
197+ client.upload.result = (
198+ "http://sca.example/dev/api/snaps/1/builds/2/status")
199+ client.checkStatus.result = (
200+ "http://sca.example/dev/click-apps/1/rev/2/", 2)
201+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
202+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
203+ run_isolated_jobs([job])
204+ self.assertEqual(
205+ SnapBuildStoreUploadStatus.UPLOADED, build2.store_upload_status)
206+ self.assertEqual(build2.store_upload_revision, 2)
207+ self.assertEqual(snap1.getBuildByStoreRevision(2), build2)
208+ self.assertEqual(snap1.getBuildByStoreRevision(1), build)
209+
210 def test_getBuildSummariesForSnapBuildIds(self):
211 snap1 = self.factory.makeSnap()
212 snap2 = self.factory.makeSnap()
213diff --git a/lib/lp/snappy/tests/test_snapbuildjob.py b/lib/lp/snappy/tests/test_snapbuildjob.py
214index 3acefa2..5b56264 100644
215--- a/lib/lp/snappy/tests/test_snapbuildjob.py
216+++ b/lib/lp/snappy/tests/test_snapbuildjob.py
217@@ -178,6 +178,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
218 self.assertContentEqual([job], snapbuild.store_upload_jobs)
219 self.assertEqual(self.store_url, job.store_url)
220 self.assertEqual(1, job.store_revision)
221+ self.assertEqual(1,
222+ removeSecurityProxy(snapbuild)._store_upload_revision)
223 self.assertIsNone(job.error_message)
224 self.assertEqual([], pop_notifications())
225 self.assertWebhookDeliveries(
226@@ -199,6 +201,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
227 self.assertContentEqual([job], snapbuild.store_upload_jobs)
228 self.assertIsNone(job.store_url)
229 self.assertIsNone(job.store_revision)
230+ self.assertIsNone(
231+ removeSecurityProxy(snapbuild)._store_upload_revision)
232 self.assertEqual("An upload failure", job.error_message)
233 self.assertEqual([], pop_notifications())
234 self.assertWebhookDeliveries(
235@@ -225,6 +229,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
236 self.assertContentEqual([job], snapbuild.store_upload_jobs)
237 self.assertIsNone(job.store_url)
238 self.assertIsNone(job.store_revision)
239+ self.assertIsNone(
240+ removeSecurityProxy(snapbuild)._store_upload_revision)
241 self.assertEqual("Authorization failed.", job.error_message)
242 [notification] = pop_notifications()
243 self.assertEqual(
244@@ -272,6 +278,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
245 self.assertContentEqual([job], snapbuild.store_upload_jobs)
246 self.assertIsNone(job.store_url)
247 self.assertIsNone(job.store_revision)
248+ self.assertIsNone(
249+ removeSecurityProxy(snapbuild)._store_upload_revision)
250 self.assertIsNone(job.error_message)
251 self.assertEqual([], pop_notifications())
252 self.assertEqual(JobStatus.WAITING, job.job.status)
253@@ -290,6 +298,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
254 self.assertContentEqual([job], snapbuild.store_upload_jobs)
255 self.assertEqual(self.store_url, job.store_url)
256 self.assertEqual(1, job.store_revision)
257+ self.assertEqual(1,
258+ removeSecurityProxy(snapbuild)._store_upload_revision)
259 self.assertIsNone(job.error_message)
260 self.assertEqual([], pop_notifications())
261 self.assertEqual(JobStatus.COMPLETED, job.job.status)
262@@ -317,6 +327,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
263 self.assertContentEqual([job], snapbuild.store_upload_jobs)
264 self.assertIsNone(job.store_url)
265 self.assertIsNone(job.store_revision)
266+ self.assertIsNone(
267+ removeSecurityProxy(snapbuild)._store_upload_revision)
268 self.assertEqual("SSO melted.", job.error_message)
269 [notification] = pop_notifications()
270 self.assertEqual(
271@@ -369,6 +381,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
272 self.assertContentEqual([job], snapbuild.store_upload_jobs)
273 self.assertIsNone(job.store_url)
274 self.assertIsNone(job.store_revision)
275+ self.assertIsNone(
276+ removeSecurityProxy(snapbuild)._store_upload_revision)
277 self.assertEqual("Failed to upload", job.error_message)
278 [notification] = pop_notifications()
279 self.assertEqual(
280@@ -419,6 +433,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
281 self.assertContentEqual([job], snapbuild.store_upload_jobs)
282 self.assertIsNone(job.store_url)
283 self.assertIsNone(job.store_revision)
284+ self.assertIsNone(
285+ removeSecurityProxy(snapbuild)._store_upload_revision)
286 self.assertIsNone(job.error_message)
287 self.assertEqual([], pop_notifications())
288 self.assertEqual(JobStatus.WAITING, job.job.status)
289@@ -437,6 +453,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
290 self.assertContentEqual([job], snapbuild.store_upload_jobs)
291 self.assertEqual(self.store_url, job.store_url)
292 self.assertEqual(1, job.store_revision)
293+ self.assertEqual(1,
294+ removeSecurityProxy(snapbuild)._store_upload_revision)
295 self.assertIsNone(job.error_message)
296 self.assertEqual([], pop_notifications())
297 self.assertEqual(JobStatus.COMPLETED, job.job.status)
298@@ -468,6 +486,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
299 self.assertContentEqual([job], snapbuild.store_upload_jobs)
300 self.assertIsNone(job.store_url)
301 self.assertIsNone(job.store_revision)
302+ self.assertIsNone(
303+ removeSecurityProxy(snapbuild)._store_upload_revision)
304 self.assertEqual(
305 "Scan failed.\nConfinement not allowed.", job.error_message)
306 self.assertEqual([
307@@ -519,6 +539,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
308 self.assertContentEqual([job], snapbuild.store_upload_jobs)
309 self.assertIsNone(job.store_url)
310 self.assertIsNone(job.store_revision)
311+ self.assertIsNone(
312+ removeSecurityProxy(snapbuild)._store_upload_revision)
313 self.assertIsNone(job.error_message)
314 self.assertEqual([], pop_notifications())
315 self.assertWebhookDeliveries(
316@@ -542,6 +564,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
317 self.assertContentEqual([job], snapbuild.store_upload_jobs)
318 self.assertEqual(self.store_url, job.store_url)
319 self.assertEqual(1, job.store_revision)
320+ self.assertEqual(1,
321+ removeSecurityProxy(snapbuild)._store_upload_revision)
322 self.assertIsNone(job.error_message)
323 self.assertEqual([], pop_notifications())
324 self.assertWebhookDeliveries(
325@@ -603,6 +627,8 @@ class TestSnapStoreUploadJob(TestCaseWithFactory):
326 # Check we uploaded as expected
327 self.assertEqual(self.store_url, job.store_url)
328 self.assertEqual(1, job.store_revision)
329+ self.assertEqual(1,
330+ removeSecurityProxy(snapbuild)._store_upload_revision)
331 self.assertEqual(timedelta(seconds=60), job.retry_delay)
332 self.assertEqual(1, len(client.upload.calls))
333 self.assertIsNone(job.error_message)

Subscribers

People subscribed via source and target branches

to status/vote changes: