Skip to content

Commit deaeb37

Browse files
committed
fix!: stop merging "by-*" keys in util.templates.merge
BREAKING CHANGE: This can impact how `task-defaults` gets merged into each task definition. The default merge behaviour would mix the `by-<key>` key in with non-keyed-by configs. This is a bug and certainly not what the user would be expecting. It is possible that the user intended to mix two `by-<key>` configs together, but this case should be extremely rare. This is a port of the following `gecko_taskgraph` patch: https://hg.mozilla.org/mozilla-central/rev/0fe5ffc10ff8488d780c5c3cbcff1b43ca974cf7
1 parent 3ba3846 commit deaeb37

File tree

2 files changed

+34
-5
lines changed

2 files changed

+34
-5
lines changed

src/taskgraph/util/templates.py

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
# License, v. 2.0. If a copy of the MPL was not distributed with this
33
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
44

5-
65
import copy
76

87

@@ -18,9 +17,19 @@ def merge_to(source, dest):
1817
"""
1918

2019
for key, value in source.items():
20+
if (
21+
isinstance(value, dict)
22+
and len(value) == 1
23+
and list(value)[0].startswith("by-")
24+
):
25+
# Do not merge by-* values as it will almost certainly not do what
26+
# the user expects.
27+
dest[key] = value
28+
continue
29+
2130
# Override mismatching or empty types
2231
if type(value) != type(dest.get(key)): # noqa
23-
dest[key] = source[key]
32+
dest[key] = value
2433
continue
2534

2635
# Merge dict
@@ -29,10 +38,10 @@ def merge_to(source, dest):
2938
continue
3039

3140
if isinstance(value, list):
32-
dest[key] = dest[key] + source[key]
41+
dest[key] = dest[key] + value
3342
continue
3443

35-
dest[key] = source[key]
44+
dest[key] = value
3645

3746
return dest
3847

test/test_util_templates.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
# License, v. 2.0. If a copy of the MPL was not distributed with this
33
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
44

5-
65
import unittest
76

87
from taskgraph.util.templates import merge, merge_to
@@ -50,3 +49,24 @@ def test_merge(self):
5049
self.assertEqual(first, {"a": 1, "b": 2, "d": 11})
5150
self.assertEqual(second, {"b": 20, "c": 30})
5251
self.assertEqual(third, {"c": 300, "d": 400})
52+
53+
def test_merge_by(self):
54+
source = {
55+
"x": "abc",
56+
"y": {"by-foo": {"quick": "fox", "default": ["a", "b", "c"]}},
57+
}
58+
dest = {"y": {"by-foo": {"purple": "rain", "default": ["x", "y", "z"]}}}
59+
expected = {
60+
"x": "abc",
61+
"y": {"by-foo": {"quick": "fox", "default": ["a", "b", "c"]}},
62+
} # source wins
63+
self.assertEqual(merge_to(source, dest), expected)
64+
self.assertEqual(dest, expected)
65+
66+
def test_merge_multiple_by(self):
67+
source = {"x": {"by-foo": {"quick": "fox", "default": ["a", "b", "c"]}}}
68+
dest = {"x": {"by-bar": {"purple": "rain", "default": ["x", "y", "z"]}}}
69+
expected = {
70+
"x": {"by-foo": {"quick": "fox", "default": ["a", "b", "c"]}}
71+
} # source wins
72+
self.assertEqual(merge_to(source, dest), expected)

0 commit comments

Comments
 (0)