Bug 1497575: [taskgraph] Handle missing value in `by-*` gracefully. r=dustin, a=release
authorTom Prince <mozilla@hocat.ca>
Wed, 17 Oct 2018 17:15:52 +0000
changeset 500806 67eae480ee468a4105363b84f82f30e1ec8e000e
parent 500805 a2063ad6267a6e5ce1a61d5128eedf373048c435
child 500807 c7d638d18847747e9dd767852ca0bc61024f2ee5
push id1864
push userffxbld-merge
push dateMon, 03 Dec 2018 15:51:40 +0000
treeherdermozilla-release@f040763d99ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdustin, release
bugs1497575
milestone64.0
Bug 1497575: [taskgraph] Handle missing value in `by-*` gracefully. r=dustin, a=release Differential Revision: https://phabricator.services.mozilla.com/D8330
taskcluster/taskgraph/test/test_util_schema.py
taskcluster/taskgraph/util/schema.py
--- a/taskcluster/taskgraph/test/test_util_schema.py
+++ b/taskcluster/taskgraph/test/test_util_schema.py
@@ -130,11 +130,31 @@ class TestResolveKeyedBy(unittest.TestCa
             Exception, resolve_keyed_by,
             {'f': 'shoes', 'x': {'by-f': {'hat': 'head'}}}, 'x', 'n')
 
     def test_multiple_matches(self):
         self.assertRaises(
             Exception, resolve_keyed_by,
             {'f': 'hats', 'x': {'by-f': {'hat.*': 'head', 'ha.*': 'hair'}}}, 'x', 'n')
 
+    def test_no_key_no_default(self):
+        """
+        When the key referenced in `by-*` doesn't exist, and there is not default value,
+        an exception is raised.
+        """
+        self.assertRaises(
+            Exception, resolve_keyed_by,
+            {'x': {'by-f': {'hat.*': 'head', 'ha.*': 'hair'}}}, 'x', 'n')
+
+    def test_no_key(self):
+        """
+        When the key referenced in `by-*` doesn't exist, and there is a default value,
+        that value is used as the result.
+        """
+        self.assertEqual(
+            resolve_keyed_by(
+                {'x': {'by-f': {'hat': 'head', 'default': 'anywhere'}}},
+                'x', 'n'),
+            {'x': 'anywhere'})
+
 
 if __name__ == '__main__':
     main()
--- a/taskcluster/taskgraph/util/schema.py
+++ b/taskcluster/taskgraph/util/schema.py
@@ -110,27 +110,37 @@ def resolve_keyed_by(item, field, item_n
     if subfield not in container:
         return item
     value = container[subfield]
     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]
+        key = extra_values[keyed_by] if keyed_by in extra_values else item.get(keyed_by)
         alternatives = value.values()[0]
 
         if len(alternatives) == 1 and 'default' in alternatives:
             # Error out when only 'default' is specified as only alternatives,
             # because we don't need to by-{keyed_by} there.
             raise Exception(
                 "Keyed-by '{}' unnecessary with only value 'default' "
                 "found, when determining item '{}' in '{}'".format(
                     keyed_by, field, item_name))
 
+        if key is None:
+            if 'default' in alternatives:
+                value = container[subfield] = alternatives['default']
+                continue
+            else:
+                raise Exception(
+                    "No attribute {} and no value for 'default' found "
+                    "while determining item {} in {}".format(
+                        keyed_by, field, item_name))
+
         matches = keymatch(alternatives, 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]