Launchpad itself

Merge ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master

  1. Git
  2. lp:~ines-almeida/launchpad
  3. fetch-service-update-build-metadata-url
  4. Merge into master
Proposed by Ines Almeida
Status: Merged
Approved by: Ines Almeida
Approved revision: 4869452f8dc0276c36d96c6f62419b61502db589
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~ines-almeida/launchpad:fetch-service-update-build-metadata-url
Merge into: launchpad:master
Diff against target: 258 lines (+144/-71)
2 files modified
lib/lp/snappy/model/snapbuild.py (+26/-4)
lib/lp/snappy/tests/test_snap.py (+118/-67)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+464839@code.launchpad.net

Commit message

Add DB load bulk load for `build_metadata_url` attributes when getting multiple builds

This reduces the number of DB queries made in the case for where a snap has multiple builds.
Also: revert commit that commented out a test that verified the snap.builds queries number

Description of the change

The test that was previously commented out now runs successfully.
Also added a new test, and re-ran the old `build_metadata_url` related tests succesfully.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) :
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 ea7bf99..842b142 100644
3--- a/lib/lp/snappy/model/snapbuild.py
4+++ b/lib/lp/snappy/model/snapbuild.py
5@@ -6,6 +6,7 @@ __all__ = [
6 "SnapFile",
7 ]
8
9+from collections import defaultdict
10 from datetime import timedelta, timezone
11 from operator import attrgetter
12
13@@ -51,7 +52,7 @@ from lp.registry.model.distribution import Distribution
14 from lp.registry.model.distroseries import DistroSeries
15 from lp.registry.model.person import Person
16 from lp.services.config import config
17-from lp.services.database.bulk import load_related
18+from lp.services.database.bulk import load_referencing, load_related
19 from lp.services.database.constants import DEFAULT
20 from lp.services.database.decoratedresultset import DecoratedResultSet
21 from lp.services.database.enumcol import DBEnum
22@@ -427,12 +428,15 @@ class SnapBuild(PackageBuildMixin, StormBase):
23 return [self.lfaUrl(lfa) for _, lfa, _ in self.getFiles()]
24
25 @property
26- def build_metadata_url(self):
27- metadata_filename = BUILD_METADATA_FILENAME_FORMAT.format(
28+ def metadata_filename(self):
29+ return BUILD_METADATA_FILENAME_FORMAT.format(
30 build_id=self.build_cookie
31 )
32+
33+ @cachedproperty
34+ def build_metadata_url(self):
35 try:
36- return self.lfaUrl(self.getFileByName(metadata_filename))
37+ return self.lfaUrl(self.getFileByName(self.metadata_filename))
38 except NotFoundError:
39 return None
40
41@@ -663,6 +667,24 @@ class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
42 )
43 load_related(Job, sbjs, ["job_id"])
44
45+ # Prefetch all snaps metadata files
46+ snap_files = load_referencing(SnapFile, builds, ["snapbuild_id"])
47+ lfas = load_related(LibraryFileAlias, snap_files, ["libraryfile_id"])
48+
49+ metadata_files = defaultdict(list)
50+ for snap_file in snap_files:
51+ if (
52+ snap_file.libraryfile.filename
53+ == snap_file.snapbuild.metadata_filename
54+ ):
55+ metadata_files[snap_file.snapbuild_id] = snap_file.libraryfile
56+
57+ for build in builds:
58+ cache = get_property_cache(build)
59+ cache.build_metadata_url = build.lfaUrl(
60+ metadata_files.get(build.id)
61+ )
62+
63 def getByBuildFarmJobs(self, build_farm_jobs):
64 """See `ISpecificBuildFarmJobSource`."""
65 if len(build_farm_jobs) == 0:
66diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
67index eace8dd..7aa8c29 100644
68--- a/lib/lp/snappy/tests/test_snap.py
69+++ b/lib/lp/snappy/tests/test_snap.py
70@@ -5798,70 +5798,121 @@ class TestSnapWebservice(TestCaseWithFactory):
71 self.webservice.get(url)
72 self.assertThat(recorder, HasQueryCount(Equals(15)))
73
74- # XXX ines-almeida 2024-04-22: after adding the new API attribute
75- # `build_metadata_url` to the snap builds, we actually perform an extra
76- # query per build. We need to make a decision on whether we are OK with
77- # this (and in such case, this test should be reworked or eventually
78- # removed)
79- #
80- # def test_builds_query_count(self):
81- # # The query count of Snap.builds is constant in the number of
82- # # builds, even if they have store upload jobs.
83- # self.pushConfig(
84- # "snappy",
85- # store_url="http://sca.example/",
86- # store_upload_url="http://updown.example/",
87- # )
88- # with admin_logged_in():
89- # snappyseries = self.factory.makeSnappySeries()
90- # distroseries = self.factory.makeDistroSeries(
91- # distribution=getUtility(IDistributionSet)["ubuntu"],
92- # registrant=self.person,
93- # )
94- # processor = self.factory.makeProcessor(supports_virtualized=True)
95- # distroarchseries = self.makeBuildableDistroArchSeries(
96- # distroseries=distroseries, processor=processor, owner=self.person
97- # )
98- # with person_logged_in(self.person):
99- # snap = self.factory.makeSnap(
100- # registrant=self.person,
101- # owner=self.person,
102- # distroseries=distroseries,
103- # processors=[processor],
104- # )
105- # snap.store_series = snappyseries
106- # snap.store_name = self.factory.getUniqueUnicode()
107- # snap.store_upload = True
108- # snap.store_secrets = {"root": Macaroon().serialize()}
109- # builds_url = "%s/builds" % api_url(snap)
110- # logout()
111- #
112- # def make_build():
113- # with person_logged_in(self.person):
114- # builder = self.factory.makeBuilder()
115- # build = snap.requestBuild(
116- # self.person,
117- # distroseries.main_archive,
118- # distroarchseries,
119- # PackagePublishingPocket.PROPOSED,
120- # )
121- # with dbuser(config.builddmaster.dbuser):
122- # build.updateStatus(
123- # BuildStatus.BUILDING, date_started=snap.date_created
124- # )
125- # build.updateStatus(
126- # BuildStatus.FULLYBUILT,
127- # builder=builder,
128- # date_finished=(
129- # snap.date_created + timedelta(minutes=10)
130- # ),
131- # )
132- # return build
133- #
134- # def get_builds():
135- # response = self.webservice.get(builds_url)
136- # self.assertEqual(200, response.status)
137- # return response
138- #
139- # recorder1, recorder2 = record_two_runs(get_builds, make_build, 2)
140- # self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
141+ def test_builds_query_count(self):
142+ # The query count of Snap.builds is constant regardless of the number
143+ # of builds, even if they have store upload jobs.
144+ self.pushConfig(
145+ "snappy",
146+ store_url="http://sca.example/",
147+ store_upload_url="http://updown.example/",
148+ )
149+ with admin_logged_in():
150+ snappyseries = self.factory.makeSnappySeries()
151+ distroseries = self.factory.makeDistroSeries(
152+ distribution=getUtility(IDistributionSet)["ubuntu"],
153+ registrant=self.person,
154+ )
155+ processor = self.factory.makeProcessor(supports_virtualized=True)
156+ distroarchseries = self.makeBuildableDistroArchSeries(
157+ distroseries=distroseries, processor=processor, owner=self.person
158+ )
159+ with person_logged_in(self.person):
160+ snap = self.factory.makeSnap(
161+ registrant=self.person,
162+ owner=self.person,
163+ distroseries=distroseries,
164+ processors=[processor],
165+ )
166+ snap.store_series = snappyseries
167+ snap.store_name = self.factory.getUniqueUnicode()
168+ snap.store_upload = True
169+ snap.store_secrets = {"root": Macaroon().serialize()}
170+ builds_url = "%s/builds" % api_url(snap)
171+ logout()
172+
173+ def make_build():
174+ with person_logged_in(self.person):
175+ builder = self.factory.makeBuilder()
176+ build = snap.requestBuild(
177+ self.person,
178+ distroseries.main_archive,
179+ distroarchseries,
180+ PackagePublishingPocket.PROPOSED,
181+ )
182+ with dbuser(config.builddmaster.dbuser):
183+ build.updateStatus(
184+ BuildStatus.BUILDING, date_started=snap.date_created
185+ )
186+ build.updateStatus(
187+ BuildStatus.FULLYBUILT,
188+ builder=builder,
189+ date_finished=(
190+ snap.date_created + timedelta(minutes=10)
191+ ),
192+ )
193+ return build
194+
195+ def get_builds():
196+ response = self.webservice.get(builds_url)
197+ self.assertEqual(200, response.status)
198+ return response
199+
200+ recorder1, recorder2 = record_two_runs(get_builds, make_build, 2)
201+ self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))
202+
203+ def test_builds_metadata_url(self):
204+ # The DB load for the `build_metadata_url` attribute of builds returns
205+ # the expected value
206+ with admin_logged_in():
207+ snappyseries = self.factory.makeSnappySeries()
208+ distroseries = self.factory.makeDistroSeries(
209+ distribution=getUtility(IDistributionSet)["ubuntu"],
210+ registrant=self.person,
211+ )
212+ processor = self.factory.makeProcessor(supports_virtualized=True)
213+ distroarchseries = self.makeBuildableDistroArchSeries(
214+ distroseries=distroseries, processor=processor, owner=self.person
215+ )
216+ with person_logged_in(self.person):
217+ snap = self.factory.makeSnap(
218+ registrant=self.person,
219+ owner=self.person,
220+ distroseries=distroseries,
221+ processors=[processor],
222+ )
223+ snap.store_series = snappyseries
224+ snap.store_name = self.factory.getUniqueUnicode()
225+ snap.store_upload = True
226+ snap.store_secrets = {"root": Macaroon().serialize()}
227+ builds_url = "%s/builds" % api_url(snap)
228+ logout()
229+
230+ with person_logged_in(self.person):
231+ build = snap.requestBuild(
232+ self.person,
233+ distroseries.main_archive,
234+ distroarchseries,
235+ PackagePublishingPocket.PROPOSED,
236+ )
237+ snap1_lfa = self.factory.makeLibraryFileAlias(
238+ filename="test.snap",
239+ content=b"dummy snap content",
240+ db_only=True,
241+ )
242+ metadata_filename = f"{build.build_cookie}_metadata.json"
243+ snap2_lfa = self.factory.makeLibraryFileAlias(
244+ filename=metadata_filename,
245+ content=b"dummy snap content",
246+ db_only=True,
247+ )
248+ self.factory.makeSnapFile(snapbuild=build, libraryfile=snap2_lfa)
249+ self.factory.makeSnapFile(snapbuild=build, libraryfile=snap1_lfa)
250+
251+ response = self.webservice.get(builds_url)
252+ self.assertEqual(200, response.status)
253+ snap_builds = (response.jsonBody()["entries"],)
254+ self.assertEqual(1, len(snap_builds))
255+ self.assertIn(
256+ metadata_filename,
257+ snap_builds[0][0]["build_metadata_url"],
258+ )

Subscribers

People subscribed via source and target branches

to status/vote changes: