lpci

Merge ~artivis/lpci:feature/yaml-alias into lpci:main

  1. Git
  2. lp:~artivis/lpci
  3. feature/yaml-alias
  4. Merge into main
Proposed by jeremie
Status: Needs review
Proposed branch: ~artivis/lpci:feature/yaml-alias
Merge into: lpci:main
Diff against target: 105 lines (+85/-1)
2 files modified
lpcraft/config.py (+1/-1)
lpcraft/tests/test_config.py (+84/-0)
Reviewer Review Type Date Requested Status
Launchpad code reviewers Pending
Review via email: mp+435370@code.launchpad.net

Description of the change

Changes pydantic configuration for extra fields from 'forbid' to 'ignore'. This allows defining extra fields which can be used to e.g. define yaml anchors. Aliases are resolved to anchors by pyyaml before being ignored (filtered out) by pydantic.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) wrote :

I'm concerned about this approach, because it would mean that people's configuration files might break when we add new official fields.

Is there some way that we could at least confine field names used for anchors to some kind of namespace?

Reply
Revision history for this message
jeremie (artivis) wrote :

> Is there some way that we could at least confine field names used for anchors
> to some kind of namespace?

Thanks for the prompt review.
Another option would be to implement a pydantic root_validator that simply filters out entries at the root level based on a prefix - see here (https://git.launchpad.net/~artivis/lpcraft/tree/lpcraft/config.py?h=feature/yaml-alias-filter#n269). Note that the prefix I used is a simple dot but it could be more elaborated. For instance Docker Compose as a similar mechanism called extension fields using the prefix 'x-' (https://docs.docker.com/compose/compose-file/compose-file-v3/#extension-fields).

Reply
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

Some more info about this topic:
https://medium.com/@kinghuang/docker-compose-anchors-aliases-extensions-a1e4105d70bd

Reply
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

On our internal channels I have proposed a solution which leverages an external tool (or Python one-liner) to convert an enhanced version of a YAML file including alias/includes into a standard YAML file which we already support as of now:
https://chat.canonical.com/canonical/pl/r71cgk3ejtr1zmbfnfitbi187c

Let's keep the conversation on the chat until we come to a conclusion.

Reply

Unmerged commits

9e4b6f6... by jeremie

couple tests yaml anchor/alias

Signed-off-by: artivis <email address hidden>

Succeeded
[SUCCEEDED] test:0 (build)
[SUCCEEDED] build:0 (build)
1 → 2 of 2 results First • Previous • Next • Last
e31b622... by jeremie

pydantic ignore extra fields in config

Signed-off-by: artivis <email address hidden>

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lpcraft/config.py b/lpcraft/config.py
2index d228270..eaaf73c 100644
3--- a/lpcraft/config.py
4+++ b/lpcraft/config.py
5@@ -31,7 +31,7 @@ class _Identifier(pydantic.ConstrainedStr):
6
7 class ModelConfigDefaults(
8 pydantic.BaseModel,
9- extra=pydantic.Extra.forbid,
10+ extra=pydantic.Extra.ignore,
11 alias_generator=lambda s: s.replace("_", "-"),
12 underscore_attrs_are_private=True,
13 ):
14diff --git a/lpcraft/tests/test_config.py b/lpcraft/tests/test_config.py
15index 6c69eab..8695520 100644
16--- a/lpcraft/tests/test_config.py
17+++ b/lpcraft/tests/test_config.py
18@@ -825,3 +825,87 @@ class TestConfig(TestCase):
19 Config.load,
20 path,
21 )
22+
23+ def test_yaml_anchor_alias_single_value(self):
24+ # A yaml alias is resolved to an anchor holding a single value.
25+ path = self.create_config(
26+ dedent(
27+ """
28+ pipeline:
29+ - test
30+
31+ jobs:
32+ test-a:
33+ series: &myseries focal
34+ architectures: [amd64]
35+
36+ test-b:
37+ series: *myseries
38+ architectures: [amd64]
39+ """
40+ )
41+ )
42+ config = Config.load(path)
43+ self.assertEqual(
44+ config.jobs["test-a"][0].series, config.jobs["test-b"][0].series
45+ )
46+ self.assertEqual("focal", config.jobs["test-b"][0].series)
47+
48+ def test_yaml_anchor_alias_single_value_at_root(self):
49+ # A yaml alias is resolved to an anchor holding a single value.
50+ path = self.create_config(
51+ dedent(
52+ """
53+ .series: &myseries focal
54+
55+ pipeline:
56+ - test
57+
58+ jobs:
59+ test:
60+ series: *myseries
61+ architectures: [amd64]
62+ """
63+ )
64+ )
65+ config = Config.load(path)
66+ self.assertEqual("focal", config.jobs["test"][0].series)
67+
68+ def test_yaml_anchor_alias_job(self):
69+ # A yaml alias is resolved to an anchor holding a whole job.
70+ # The root-level extra filed is filtered out by pydantic.
71+ path = self.create_config(
72+ dedent(
73+ """
74+ .test: &mytest
75+ series: focal
76+ architectures: [amd64]
77+
78+ pipeline:
79+ - test
80+
81+ jobs:
82+ test: *mytest
83+ """
84+ )
85+ )
86+ config = Config.load(path)
87+
88+ self.assertThat(
89+ config,
90+ MatchesStructure(
91+ pipeline=Equals([["test"]]),
92+ jobs=MatchesDict(
93+ {
94+ "test": MatchesListwise(
95+ [
96+ MatchesStructure.byEquality(
97+ series="focal",
98+ architectures=["amd64"],
99+ )
100+ ]
101+ )
102+ }
103+ ),
104+ ),
105+ )

Subscribers

People subscribed via source and target branches