launchpad-reviewers team mailing list archive

[Colin Watson <mp+425184@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:ci-build-upload-job-filter-by-repository-format into launchpad:master.

Commit message:
Limit files uploaded by CIBuildUploadJob by repository format

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425184

If an archive has `repository_format=PYTHON`, then we only want to upload Python wheels to it, not Conda packages (and so on).  Make `CIBuildUploadJob` limit the files it uploads based on the target archive's repository format.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:ci-build-upload-job-filter-by-repository-format into launchpad:master.

diff --git a/lib/lp/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
index e3948d8..499e83d 100644
--- a/lib/lp/soyuz/model/archivejob.py
+++ b/lib/lp/soyuz/model/archivejob.py
@@ -43,6 +43,7 @@ from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.librarian.utils import copy_and_close
 from lp.soyuz.enums import (
     ArchiveJobType,
+    ArchiveRepositoryFormat,
     BinaryPackageFileType,
     BinaryPackageFormat,
     PackageUploadStatus,
@@ -218,6 +219,21 @@ class CIBuildUploadJob(ArchiveJobDerived):
         BinaryPackageFormat.CONDA_V2: BinaryPackageFileType.CONDA_V2,
         }
 
+    # We're only interested in uploading certain kinds of packages to
+    # certain kinds of archives.
+    binary_format_by_repository_format = {
+        ArchiveRepositoryFormat.DEBIAN: {
+            BinaryPackageFormat.DEB,
+            BinaryPackageFormat.UDEB,
+            BinaryPackageFormat.DDEB,
+            },
+        ArchiveRepositoryFormat.PYTHON: {BinaryPackageFormat.WHL},
+        ArchiveRepositoryFormat.CONDA: {
+            BinaryPackageFormat.CONDA_V1,
+            BinaryPackageFormat.CONDA_V2,
+            },
+        }
+
     @classmethod
     def create(cls, ci_build, requester, target_archive, target_distroseries,
                target_pocket, target_channel=None):
@@ -336,6 +352,9 @@ class CIBuildUploadJob(ArchiveJobDerived):
             releases_by_name = {
                 release.binarypackagename: release
                 for release in self.ci_build.binarypackages}
+            allowed_binary_formats = (
+                self.binary_format_by_repository_format.get(
+                    self.archive.repository_format, set()))
             binaries = {}
             for artifact in getUtility(
                     IRevisionStatusArtifactSet).findByCIBuild(self.ci_build):
@@ -349,6 +368,12 @@ class CIBuildUploadJob(ArchiveJobDerived):
                 if metadata is None:
                     logger.info("No upload handler for %s" % name)
                     continue
+                binpackageformat = metadata["binpackageformat"]
+                if binpackageformat not in allowed_binary_formats:
+                    logger.info(
+                        "Skipping %s (not relevant to %s archives)" % (
+                            name, self.archive.repository_format))
+                    continue
                 logger.info(
                     "Uploading %s to %s %s (%s)" % (
                         name, self.archive.reference,
@@ -357,8 +382,7 @@ class CIBuildUploadJob(ArchiveJobDerived):
                 metadata["binarypackagename"] = bpn = (
                     getUtility(IBinaryPackageNameSet).ensure(metadata["name"]))
                 del metadata["name"]
-                filetype = self.filetype_by_format[
-                    metadata["binpackageformat"]]
+                filetype = self.filetype_by_format[binpackageformat]
                 bpr = releases_by_name.get(bpn)
                 if bpr is None:
                     bpr = self.ci_build.createBinaryPackageRelease(**metadata)
diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
index a0c39b4..b42ae5a 100644
--- a/lib/lp/soyuz/tests/test_archivejob.py
+++ b/lib/lp/soyuz/tests/test_archivejob.py
@@ -28,6 +28,7 @@ from lp.services.librarian.interfaces.client import LibrarianServerError
 from lp.services.mail.sendmail import format_address_for_person
 from lp.soyuz.enums import (
     ArchiveJobType,
+    ArchiveRepositoryFormat,
     BinaryPackageFileType,
     BinaryPackageFormat,
     PackageUploadStatus,
@@ -318,7 +319,8 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
         self.assertEqual(expected, job._scanFile(datadir(path)))
 
     def test_run_indep(self):
-        archive = self.factory.makeArchive()
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON)
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         dases = [
@@ -365,7 +367,8 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
             for das in dases)))
 
     def test_run_arch(self):
-        archive = self.factory.makeArchive()
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON)
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         dases = [
@@ -411,7 +414,8 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
                 distroarchseries=Equals(dases[0]))))
 
     def test_run_conda(self):
-        archive = self.factory.makeArchive()
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.CONDA)
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         dases = [
@@ -457,7 +461,8 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
                 distroarchseries=Equals(dases[0]))))
 
     def test_run_conda_v2(self):
-        archive = self.factory.makeArchive()
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.CONDA)
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         dases = [
@@ -506,7 +511,8 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
         # A `CIBuildUploadJob` can be run even if the build in question was
         # already uploaded somewhere, and in that case may add publications
         # in other locations for the same package.
-        archive = self.factory.makeArchive()
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON)
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         das = self.factory.makeDistroArchSeries(distroseries=distroseries)
@@ -553,14 +559,24 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
                 channel=Equals(channel))
             for channel in ("edge", "0.0.1/edge"))))
 
+<<<<<<< lib/lp/soyuz/tests/test_archivejob.py
     def test_librarian_server_error_retries(self):
         # A run that gets an error from the librarian server schedules
         # itself to be retried.
         archive = self.factory.makeArchive()
+=======
+    def test_skips_disallowed_binary_formats(self):
+        # A CI job might build multiple types of packages, of which only
+        # some are interesting to upload to archives with a given repository
+        # format.  Others are skipped.
+        archive = self.factory.makeArchive(
+            repository_format=ArchiveRepositoryFormat.PYTHON)
+>>>>>>> lib/lp/soyuz/tests/test_archivejob.py
         distroseries = self.factory.makeDistroSeries(
             distribution=archive.distribution)
         das = self.factory.makeDistroArchSeries(distroseries=distroseries)
         build = self.factory.makeCIBuild(distro_arch_series=das)
+<<<<<<< lib/lp/soyuz/tests/test_archivejob.py
         report = build.getOrCreateRevisionStatusReport("build:0")
         path = "wheel-indep/dist/wheel_indep-0.0.1-py3-none-any.whl"
         with open(datadir(path), mode="rb") as f:
@@ -586,6 +602,38 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
             JobRunner([job]).runAll()
 
         self.assertEqual(JobStatus.COMPLETED, job.job.status)
+=======
+        wheel_report = build.getOrCreateRevisionStatusReport("build-wheel:0")
+        wheel_path = "wheel-indep/dist/wheel_indep-0.0.1-py3-none-any.whl"
+        conda_path = "conda-v2-arch/dist/linux-64/conda-v2-arch-0.1-0.conda"
+        with open(datadir(wheel_path), mode="rb") as f:
+            wheel_report.attach(
+                name=os.path.basename(wheel_path), data=f.read())
+        conda_report = build.getOrCreateRevisionStatusReport("build-conda:0")
+        with open(datadir(conda_path), mode="rb") as f:
+            conda_report.attach(
+                name=os.path.basename(conda_path), data=f.read())
+        job = CIBuildUploadJob.create(
+            build, build.git_repository.owner, archive, distroseries,
+            PackagePublishingPocket.RELEASE, target_channel="edge")
+        transaction.commit()
+
+        with dbuser(job.config.dbuser):
+            JobRunner([job]).runAll()
+
+        self.assertThat(archive.getAllPublishedBinaries(), MatchesSetwise(
+            MatchesStructure(
+                binarypackagename=MatchesStructure.byEquality(
+                    name="wheel-indep"),
+                binarypackagerelease=MatchesStructure(
+                    ci_build=Equals(build),
+                    binarypackagename=MatchesStructure.byEquality(
+                        name="wheel-indep"),
+                    version=Equals("0.0.1"),
+                    binpackageformat=Equals(BinaryPackageFormat.WHL)),
+                binarypackageformat=Equals(BinaryPackageFormat.WHL),
+                distroarchseries=Equals(das))))
+>>>>>>> lib/lp/soyuz/tests/test_archivejob.py
 
 
 class TestViaCelery(TestCaseWithFactory):