Bug 1339604: Allow nesting of keyed-by values; r?kmoir
MozReview-Commit-ID: LRzDWauipGQ
--- a/taskcluster/taskgraph/test/test_util_schema.py
+++ b/taskcluster/taskgraph/test/test_util_schema.py
@@ -48,16 +48,42 @@ class TestResolveKeyedBy(unittest.TestCa
resolve_keyed_by({'x': 10}, 'x.y', 'n'),
{'x': 10})
def test_no_by_not_by(self):
self.assertEqual(
resolve_keyed_by({'x': {'a': 10}}, 'x', 'n'),
{'x': {'a': 10}})
+ def test_nested(self):
+ x = {
+ 'by-foo': {
+ 'F1': {
+ 'by-bar': {
+ 'B1': 11,
+ 'B2': 12,
+ },
+ },
+ 'F2': 20,
+ 'default': 0,
+ },
+ }
+ self.assertEqual(
+ resolve_keyed_by({'x': x}, 'x', 'x', foo='F1', bar='B1'),
+ {'x': 11})
+ self.assertEqual(
+ resolve_keyed_by({'x': x}, 'x', 'x', foo='F1', bar='B2'),
+ {'x': 12})
+ self.assertEqual(
+ resolve_keyed_by({'x': x}, 'x', 'x', foo='F2'),
+ {'x': 20})
+ self.assertEqual(
+ resolve_keyed_by({'x': x}, 'x', 'x', foo='F99', bar='B1'),
+ {'x': 0})
+
def test_no_by_empty_dict(self):
self.assertEqual(
resolve_keyed_by({'x': {}}, 'x', 'n'),
{'x': {}})
def test_no_by_not_only_by(self):
self.assertEqual(
resolve_keyed_by({'x': {'by-y': True, 'a': 10}}, 'x', 'n'),
--- a/taskcluster/taskgraph/util/schema.py
+++ b/taskcluster/taskgraph/util/schema.py
@@ -29,88 +29,110 @@ def optionally_keyed_by(*arguments):
"""
Mark a schema value as optionally keyed by any of a number of fields. The
schema is the last argument, and the remaining fields are taken to be the
field names. For example:
'some-value': optionally_keyed_by(
'test-platform', 'build-platform',
Any('a', 'b', 'c'))
+
+ The resulting schema will allow nesting of `by-test-platform` and
+ `by-build-platform` in either order.
"""
- subschema = arguments[-1]
+ schema = arguments[-1]
fields = arguments[:-1]
- options = [subschema]
- for field in fields:
- options.append({'by-' + field: {basestring: subschema}})
- return voluptuous.Any(*options)
+
+ # build the nestable schema by generating schema = Any(schema,
+ # by-fld1, by-fld2, by-fld3) once for each field. So we don't allow
+ # infinite nesting, but one level of nesting for each field.
+ for _ in arguments:
+ options = [schema]
+ for field in fields:
+ options.append({'by-' + field: {basestring: schema}})
+ schema = voluptuous.Any(*options)
+ return schema
def resolve_keyed_by(item, field, item_name, **extra_values):
"""
For values which can either accept a literal value, or be keyed by some
other attribute of the item, perform that lookup and replacement in-place
(modifying `item` directly). The field is specified using dotted notation
to traverse dictionaries.
- For example, given item
+ For example, given item::
job:
test-platform: linux128
chunks:
by-test-platform:
macosx-10.11/debug: 13
win.*: 6
default: 12
a call to `resolve_keyed_by(item, 'job.chunks', item['thing-name'])
- would mutate item in-place to
+ would mutate item in-place to::
job:
chunks: 12
The `item_name` parameter is used to generate useful error messages.
If extra_values are supplied, they represent additional values available
for reference from by-<field>.
+
+ Items can be nested as deeply as the schema will allow::
+
+ chunks:
+ by-test-platform:
+ win.*:
+ by-project:
+ ash: ..
+ cedar: ..
+ linux: 13
+ default: 12
"""
# find the field, returning the item unchanged if anything goes wrong
container, subfield = item, field
while '.' in subfield:
f, subfield = subfield.split('.', 1)
if f not in container:
return item
container = container[f]
if not isinstance(container, dict):
return item
if subfield not in container:
return item
value = container[subfield]
- if not isinstance(value, dict) or len(value) != 1 or not value.keys()[0].startswith('by-'):
- return item
+ while True:
+ if not isinstance(value, dict) or len(value) != 1 or not value.keys()[0].startswith('by-'):
+ return item
- keyed_by = value.keys()[0][3:] # strip off 'by-' prefix
- key = extra_values.get(keyed_by) if keyed_by in extra_values else item[keyed_by]
- alternatives = value.values()[0]
+ keyed_by = value.keys()[0][3:] # strip off 'by-' prefix
+ key = extra_values.get(keyed_by) if keyed_by in extra_values else item[keyed_by]
+ alternatives = value.values()[0]
- # exact match
- if key in alternatives:
- container[subfield] = alternatives[key]
- return item
+ # exact match
+ if key in alternatives:
+ value = container[subfield] = alternatives[key]
+ continue
- # regular expression match
- matches = [(k, v) for k, v in alternatives.iteritems() if re.match(k + '$', key)]
- if len(matches) > 1:
+ # regular expression match
+ matches = [(k, v) for k, v in alternatives.iteritems() if re.match(k + '$', key)]
+ if len(matches) > 1:
+ raise Exception(
+ "Multiple matching values for {} {!r} found while "
+ "determining item {} in {}".format(
+ keyed_by, key, field, item_name))
+ elif matches:
+ value = container[subfield] = matches[0][1]
+ continue
+
+ # default
+ if 'default' in alternatives:
+ value = container[subfield] = alternatives['default']
+ continue
+
raise Exception(
- "Multiple matching values for {} {!r} found while determining item {} in {}".format(
+ "No {} matching {!r} nor 'default' found while determining item {} in {}".format(
keyed_by, key, field, item_name))
- elif matches:
- container[subfield] = matches[0][1]
- return item
-
- # default
- if 'default' in alternatives:
- container[subfield] = alternatives['default']
- return item
-
- raise Exception(
- "No {} matching {!r} nor 'default' found while determining item {} in {}".format(
- keyed_by, key, field, item_name))