Merge ~cjwatson/launchpad:rework-archive-job-series-check into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: bf0f237f50338f0b0b75b1d0b55f3cabe733ce7c
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:rework-archive-job-series-check
Merge into: launchpad:master
Diff against target: 90 lines (+30/-28)
2 files modified
lib/lp/soyuz/model/archivejob.py (+21/-21)
lib/lp/soyuz/tests/test_archivejob.py (+9/-7)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+456336@code.launchpad.net

Commit message

Rework CI build upload series check to use DistroSeriesParent

Description of the change

In https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/448906, I had to resort to doing series name comparison because I thought that there was no proper database-level link between the build and target series in the cases where we're using this at present. I've since realized that was wrong, and the two series are linked via the `DistroSeriesParent` table, so we can do this more neatly.

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/soyuz/model/archivejob.py b/lib/lp/soyuz/model/archivejob.py
2index 8c9ff5a..b3beedf 100644
3--- a/lib/lp/soyuz/model/archivejob.py
4+++ b/lib/lp/soyuz/model/archivejob.py
5@@ -797,28 +797,28 @@ class CIBuildUploadJob(ArchiveJobDerived):
6 continue
7 artifact = scanned_artifact.artifact
8 library_file = artifact.library_file
9- # XXX cjwatson 2023-08-09: Comparing distroseries names here is
10- # a pretty unpleasant hack, but it's necessary because in
11- # practice what we're doing is taking the output of build jobs
12- # that were run on (e.g.) Ubuntu focal and uploading them to
13- # (e.g.) <private distribution> focal. Unfortunately the
14- # private series doesn't have its parent series set to the
15- # corresponding Ubuntu series, so we have no way to make the
16- # connection other than comparing names. We need to figure out
17- # something better here, but at least this hack isn't too deep
18- # in the core of the system.
19- if (
20- artifact.report.distro_arch_series is not None
21- and artifact.report.distro_arch_series.distroseries.name
22- != self.target_distroseries.name
23- ):
24- logger.info(
25- "Skipping %s (built for %s, not %s)",
26- library_file.filename,
27- artifact.report.distro_arch_series.distroseries.name,
28- self.target_distroseries.name,
29+ if artifact.report.distro_arch_series is not None:
30+ build_distroseries = (
31+ artifact.report.distro_arch_series.distroseries
32 )
33- continue
34+ # It might seem that we ought to check that the build job's
35+ # series is equal to the target series, but in practice what
36+ # we're doing is taking the output of build jobs that were
37+ # run on (e.g.) Ubuntu focal and uploading them to (e.g.)
38+ # <private distribution> focal, so we need to check parent
39+ # series links as well.
40+ if (
41+ build_distroseries != self.target_distroseries
42+ and build_distroseries
43+ not in self.target_distroseries.getParentSeries()
44+ ):
45+ logger.info(
46+ "Skipping %s (built for %s, not %s)",
47+ library_file.filename,
48+ build_distroseries.name,
49+ self.target_distroseries.name,
50+ )
51+ continue
52 logger.info(
53 "Uploading %s to %s %s (%s)",
54 library_file.filename,
55diff --git a/lib/lp/soyuz/tests/test_archivejob.py b/lib/lp/soyuz/tests/test_archivejob.py
56index 4592b7a..3244e4d 100644
57--- a/lib/lp/soyuz/tests/test_archivejob.py
58+++ b/lib/lp/soyuz/tests/test_archivejob.py
59@@ -1660,8 +1660,7 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
60 # series are selected.
61 #
62 # The build distribution is always Ubuntu for now, but the target
63- # distribution may differ. Unfortunately, this currently requires
64- # matching on series names.
65+ # distribution may differ.
66 logger = self.useFixture(FakeLogger())
67 target_distribution = self.factory.makeDistribution()
68 archive = self.factory.makeArchive(
69@@ -1672,13 +1671,16 @@ class TestCIBuildUploadJob(TestCaseWithFactory):
70 self.factory.makeUbuntuDistroSeries() for _ in range(2)
71 ]
72 target_distroserieses = [
73- self.factory.makeDistroSeries(
74- distribution=target_distribution,
75- name=ubuntu_distroseries.name,
76- version=ubuntu_distroseries.version,
77- )
78+ self.factory.makeDistroSeries(distribution=target_distribution)
79 for ubuntu_distroseries in ubuntu_distroserieses
80 ]
81+ for ubuntu_distroseries, target_distroseries in zip(
82+ ubuntu_distroserieses, target_distroserieses
83+ ):
84+ self.factory.makeDistroSeriesParent(
85+ parent_series=ubuntu_distroseries,
86+ derived_series=target_distroseries,
87+ )
88 processor = self.factory.makeProcessor()
89 ubuntu_dases = [
90 self.factory.makeDistroArchSeries(

Subscribers

People subscribed via source and target branches

to status/vote changes: