Merge ~cjwatson/launchpad:fix-missing-cibuild-config into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 632c398cf056c48c1e87a675fb6c4619d176b354
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:fix-missing-cibuild-config
Merge into: launchpad:master
Diff against target: 90 lines (+32/-10)
2 files modified
lib/lp/code/model/cibuildbehaviour.py (+4/-0)
lib/lp/code/model/tests/test_cibuildbehaviour.py (+28/-10)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+423096@code.launchpad.net

Commit message

Fix case where cibuild.* config section is present but empty

Description of the change

If the `cibuild.*` configuration section for a distribution exists in the schema but has no `environment_variables` or `apt_repositories` keys, dispatching CI builds crashed. Handle this case.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Thank you, Colin!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/code/model/cibuildbehaviour.py b/lib/lp/code/model/cibuildbehaviour.py
2index cd51eb4..8bdb499 100644
3--- a/lib/lp/code/model/cibuildbehaviour.py
4+++ b/lib/lp/code/model/cibuildbehaviour.py
5@@ -54,6 +54,8 @@ def build_environment_variables(distribution_name: str) -> dict:
6 pairs = config["cibuild."+distribution_name]["environment_variables"]
7 except NoSectionError:
8 return {}
9+ if pairs is None:
10+ return {}
11 rv = {}
12 for key, value in json.loads(pairs).items():
13 rv[key] = replace_placeholders(value)
14@@ -67,6 +69,8 @@ def build_apt_repositories(distribution_name: str) -> list:
15 lines = config["cibuild."+distribution_name]["apt_repositories"]
16 except NoSectionError:
17 return []
18+ if lines is None:
19+ return []
20 rv = []
21 for line in json.loads(lines):
22 rv.append(replace_placeholders(line))
23diff --git a/lib/lp/code/model/tests/test_cibuildbehaviour.py b/lib/lp/code/model/tests/test_cibuildbehaviour.py
24index 9b5df18..17b78e9 100644
25--- a/lib/lp/code/model/tests/test_cibuildbehaviour.py
26+++ b/lib/lp/code/model/tests/test_cibuildbehaviour.py
27@@ -181,15 +181,6 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
28 base_url="canonical.artifactory.com",
29 read_credentials="user:pass",
30 )
31- self.pushConfig(
32- "cibuild.soss",
33- environment_variables=json.dumps({
34- "PIP_INDEX_URL": "http://%(read_auth)s@%(base_url)s/simple",
35- "SOME_PATH": "/bin/zip"}),
36- apt_repositories=json.dumps([
37- "deb https://%(read_auth)s@%(base_url)s/repository focal main",
38- "deb https://public_ppa.example.net/repository focal main"])
39- )
40 self.now = time.time()
41 self.useFixture(MockPatch("time.time", return_value=self.now))
42 self.addCleanup(shut_down_default_process_pool)
43@@ -284,7 +275,7 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
44 }))
45
46 @defer.inlineCallbacks
47- def test_extraBuildArgs_git_does_not_contain_artifactory_data(self):
48+ def test_extraBuildArgs_git_no_artifactory_configuration_section(self):
49 # create a distribution with a name other than "soss", see
50 # schema-lazr.conf -> cibuild.soss
51 # for this distribution neither Artifactory environment variables nor
52@@ -303,11 +294,38 @@ class TestAsyncCIBuildBehaviour(StatsMixin, TestCIBuildBehaviourBase):
53 self.assertNotIn([], args["apt_repositories"])
54
55 @defer.inlineCallbacks
56+ def test_extraBuildArgs_git_no_artifactory_configuration(self):
57+ # If the cibuild.* section exists for the distribution but has no
58+ # relevant configuration entries, neither Artifactory environment
59+ # variables nor apt repositories will be dispatched.
60+ package = self.factory.makeDistributionSourcePackage(
61+ distribution=self.factory.makeDistribution(name="soss")
62+ )
63+ git_repository = self.factory.makeGitRepository(target=package)
64+ job = self.makeJob(
65+ stages=[[("test", 0)]],
66+ git_repository=git_repository
67+ )
68+ with dbuser(config.builddmaster.dbuser):
69+ args = yield job.extraBuildArgs()
70+ self.assertEqual({}, args["environment_variables"])
71+ self.assertNotIn([], args["apt_repositories"])
72+
73+ @defer.inlineCallbacks
74 def test_extraBuildArgs_git_include_artifactory_configuration(self):
75 # we use the `soss` distribution which refers to `cibuild.soss` in the
76 # global configuration
77 # suitable Artifactory configuration data will be dispatched for this
78 # distribution
79+ self.pushConfig(
80+ "cibuild.soss",
81+ environment_variables=json.dumps({
82+ "PIP_INDEX_URL": "http://%(read_auth)s@%(base_url)s/simple",
83+ "SOME_PATH": "/bin/zip"}),
84+ apt_repositories=json.dumps([
85+ "deb https://%(read_auth)s@%(base_url)s/repository focal main",
86+ "deb https://public_ppa.example.net/repository focal main"])
87+ )
88 package = self.factory.makeDistributionSourcePackage(
89 distribution=self.factory.makeDistribution(name="soss")
90 )

Subscribers

People subscribed via source and target branches

to status/vote changes: