launchpad-reviewers team mailing list archive

[Ines Almeida <mp+464751@xxxxxxxxxxxxxxxxxx>]

Ines Almeida has proposed merging ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master.

Commit message:
Update `build_metadat_url` logic and tests and queries associated with it

This includes:
 - Update the query used to get the metadata file to be lighter
 - Remove test that no longer applies
 - Pre-load librarian file data when fetching all builds

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~ines-almeida/launchpad/+git/launchpad/+merge/464751

All `build_metadata_url` tests re-ran.

I'm not 100% set if there is more to be done regarding keeping the query count stable regardless of builds, unless we store the link to the librarian file directly in the SnapBuild DB table. I'd love opinions on this.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad:fetch-service-update-build-metadata-url into launchpad:master.

diff --git a/lib/lp/buildmaster/builderproxy.py b/lib/lp/buildmaster/builderproxy.py
index 1d08e03..9bacf76 100644
--- a/lib/lp/buildmaster/builderproxy.py
+++ b/lib/lp/buildmaster/builderproxy.py
@@ -11,6 +11,7 @@ token if and only if they are allowed general internet access.
 """
 
 __all__ = [
+    "BUILD_METADATA_FILENAME_FORMAT",
     "BuilderProxyMixin",
 ]
 
diff --git a/lib/lp/snappy/model/snapbuild.py b/lib/lp/snappy/model/snapbuild.py
index c7418e6..0fc5d16 100644
--- a/lib/lp/snappy/model/snapbuild.py
+++ b/lib/lp/snappy/model/snapbuild.py
@@ -51,7 +51,7 @@ from lp.registry.model.distribution import Distribution
 from lp.registry.model.distroseries import DistroSeries
 from lp.registry.model.person import Person
 from lp.services.config import config
-from lp.services.database.bulk import load_related
+from lp.services.database.bulk import load_referencing, load_related
 from lp.services.database.constants import DEFAULT
 from lp.services.database.decoratedresultset import DecoratedResultSet
 from lp.services.database.enumcol import DBEnum
@@ -431,10 +431,10 @@ class SnapBuild(PackageBuildMixin, StormBase):
         metadata_filename = BUILD_METADATA_FILENAME_FORMAT.format(
             build_id=self.build_cookie
         )
-        for url in self.getFileUrls():
-            if url.endswith(metadata_filename):
-                return url
-        return None
+        try:
+            return self.lfaUrl(self.getFileByName(metadata_filename))
+        except NotFoundError:
+            return None
 
     @cachedproperty
     def eta(self):
@@ -663,6 +663,10 @@ class SnapBuildSet(SpecificBuildFarmJobSourceMixin):
             )
         load_related(Job, sbjs, ["job_id"])
 
+        # Prefetch snap files
+        snap_files = load_referencing(SnapFile, builds, ["snapbuild_id"])
+        load_related(LibraryFileAlias, snap_files, ["libraryfile_id"])
+
     def getByBuildFarmJobs(self, build_farm_jobs):
         """See `ISpecificBuildFarmJobSource`."""
         if len(build_farm_jobs) == 0:
diff --git a/lib/lp/snappy/tests/test_snap.py b/lib/lp/snappy/tests/test_snap.py
index 9f671b8..39ba808 100644
--- a/lib/lp/snappy/tests/test_snap.py
+++ b/lib/lp/snappy/tests/test_snap.py
@@ -5797,65 +5797,3 @@ class TestSnapWebservice(TestCaseWithFactory):
         with StormStatementRecorder() as recorder:
             self.webservice.get(url)
         self.assertThat(recorder, HasQueryCount(Equals(15)))
-
-    def test_builds_query_count(self):
-        # The query count of Snap.builds is constant in the number of
-        # builds, even if they have store upload jobs.
-        self.pushConfig(
-            "snappy",
-            store_url="http://sca.example/";,
-            store_upload_url="http://updown.example/";,
-        )
-        with admin_logged_in():
-            snappyseries = self.factory.makeSnappySeries()
-        distroseries = self.factory.makeDistroSeries(
-            distribution=getUtility(IDistributionSet)["ubuntu"],
-            registrant=self.person,
-        )
-        processor = self.factory.makeProcessor(supports_virtualized=True)
-        distroarchseries = self.makeBuildableDistroArchSeries(
-            distroseries=distroseries, processor=processor, owner=self.person
-        )
-        with person_logged_in(self.person):
-            snap = self.factory.makeSnap(
-                registrant=self.person,
-                owner=self.person,
-                distroseries=distroseries,
-                processors=[processor],
-            )
-            snap.store_series = snappyseries
-            snap.store_name = self.factory.getUniqueUnicode()
-            snap.store_upload = True
-            snap.store_secrets = {"root": Macaroon().serialize()}
-        builds_url = "%s/builds" % api_url(snap)
-        logout()
-
-        def make_build():
-            with person_logged_in(self.person):
-                builder = self.factory.makeBuilder()
-                build = snap.requestBuild(
-                    self.person,
-                    distroseries.main_archive,
-                    distroarchseries,
-                    PackagePublishingPocket.PROPOSED,
-                )
-                with dbuser(config.builddmaster.dbuser):
-                    build.updateStatus(
-                        BuildStatus.BUILDING, date_started=snap.date_created
-                    )
-                    build.updateStatus(
-                        BuildStatus.FULLYBUILT,
-                        builder=builder,
-                        date_finished=(
-                            snap.date_created + timedelta(minutes=10)
-                        ),
-                    )
-                return build
-
-        def get_builds():
-            response = self.webservice.get(builds_url)
-            self.assertEqual(200, response.status)
-            return response
-
-        recorder1, recorder2 = record_two_runs(get_builds, make_build, 2)
-        self.assertThat(recorder2, HasQueryCount.byEquality(recorder1))