Differential D1176

taskformula: replace vars in dictionary keys
ClosedPublic
Actions

Authored by kparal on Mar 23 2017, 12:53 PM.

Details

Reviewers
jskladan
Group Reviewers
libtaskotron
Commits
rLTRNcb1f5d59a884: taskformula: replace vars in dictionary keys
Summary

Allow to replace vars also in dictionary keys. This is required e.g. for
current depcheck/rpmdeplint formula, which uses nested python
dictionaries when passing data to a python directive.

Test Plan

test suite works, and also my patched rpmdeplint formula now works

Diff Detail

Repository
rLTRN libtaskotron
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kparal created this revision.Mar 23 2017, 12:53 PM
jskladan accepted this revision.Mar 24 2017, 9:21 AM

I'd like to see some inline comments, but looks good otherwise

libtaskotron/taskformula.py
161–163

pls comment the purpose

This revision is now accepted and ready to land.Mar 24 2017, 9:21 AM
Closed by commit rLTRNcb1f5d59a884: taskformula: replace vars in dictionary keys (authored by Kamil Páral <kparal@redhat.com>). · Explain WhyMar 24 2017, 9:25 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathPackages
Mlibtaskotron/taskformula.py (38 lines)
Mtesting/test_taskformula.py (28 lines)

Diff 2995

View Options

libtaskotron/taskformula.py

Show First 20 Lines • Show All 103 Lines • ▼ Show 20 Line(s)103 except ValueError as e:
104 raise exc.TaskotronYamlError("The task formula includes an incorrect " 104​ raise exc.TaskotronYamlError("The task formula includes an incorrect "
105 "variable definition. Dollar signs must be doubled if they " 105​ "variable definition. Dollar signs must be doubled if they "
106 "shouldn't be considered as a variable denotation.\n" 106​ "shouldn't be considered as a variable denotation.\n"
107 "Error: %s\n" 107​ "Error: %s\n"
108 "Text: %s" % (e, text)) 108​ "Text: %s" % (e, text))
109 109​
110 110​
111def replace_vars_in_action(action, variables): 111​def replace_vars_in_action(action, variables):
112 '''Find all variables that are leaves (in a tree sense) in an action. Leaves 112​ '''Replace variables (${var} or ${var.key}) with their values in an action (YAML dictionary).
113 are variables which can no longer be traversed, i.e. "primitive" types like ​
114 ``str`` or ``int``. Non-leaves are containers like ``dict`` or ``list``. ​
115 113​
116 For all leaves, call :func:`_replace_vars` and update their value with 114​ Only basestring values are searched for variables syntax, which means variables can either be
117 the function's output. 115​ inside leaves (in a tree sense, meaning they can't be traversed further and are of a primitive
116​ type), or inside key names in a dictionary.
118 117​
119 :param dict action: An action specification parsed from the task formula. 118​ :param dict action: An action specification parsed from the task formula. See
120 See :meth:`.Runner.do_actions` to see what an action 119​ :meth:`.Runner.do_actions` to see what an action looks like.
121 looks like. ​
122 :param dict variables: names (keys) and values of variables to replace 120​ :param dict variables: names (keys) and values of variables to replace
123 :raise TaskotronYamlError: if ``text`` contains a variable that is not 121​ :raise TaskotronYamlError: if ``text`` contains a variable that is not present in
124 present in ``variables``, or if the variable 122​ ``variables``; or if the variable syntax is incorrect; of if you try
125 syntax is incorrect 123​ to replace a dictionary key with something else than a string
126 ''' 124​ '''
127 125​
128 visited = [] # all visited nodes in a tree 126​ visited = [] # all visited nodes in a tree
129 stack = [action] # nodes waiting for inspection 127​ stack = [action] # nodes waiting for inspection
130 128​
131 while stack: 129​ while stack:
132 vertex = stack.pop() 130​ vertex = stack.pop()
133 131​
134 if vertex in visited: 132​ if vertex in visited:
135 continue 133​ continue
136 134​
137 visited.append(vertex) 135​ visited.append(vertex)
138 children = [] # list of tuples (index/key, child_value) 136​ children = [] # list of tuples (index/key, child_value)
139 137​
140 if isinstance(vertex, collections.MutableMapping): 138​ if isinstance(vertex, collections.MutableMapping):
141 children = vertex.items() # list of (key, value) 139​ children = vertex.items() # list of (key, value)
142 elif isinstance(vertex, collections.MutableSequence): 140​ elif isinstance(vertex, collections.MutableSequence):
143 children = list(enumerate(vertex)) # list of (index, value) 141​ children = list(enumerate(vertex)) # list of (index, value)
144 else: 142​ else:
145 log.warn("Unknown structure '%s' in YAML file, this shouldn't " 143​ log.warn("Unknown structure '%s' in YAML file, this shouldn't happen: %s",
146 "happen: %s", type(vertex), vertex) 144​ type(vertex), vertex)
147 145​
148 for index, child_val in children: 146​ for index, child_val in children:
149 if isinstance(child_val, basestring): 147​ if isinstance(child_val, basestring):
150 # leaf node and a string, replace variables 148​ # leaf node and a string, replace variables
151 vertex[index] = _replace_vars(child_val, variables) 149​ vertex[index] = _replace_vars(child_val, variables)
152 elif isinstance(child_val, collections.Iterable): 150​ elif isinstance(child_val, collections.Iterable):
153 # traversable further down, mark for visit 151​ # traversable further down, mark for visit
154 stack.append(child_val) 152​ stack.append(child_val)
155 153​
154​ # for dicts, also replace vars in key names
155​ if isinstance(index, basestring): # meaning it's a dictionary
156​ new_key = _replace_vars(index, variables)
157​ if not isinstance(new_key, basestring):
158​ raise exc.TaskotronYamlError(
159​ "Dictionary key '%s' can't be replaced with value %r, because it's not a "
160​ "string, but %s" % (index, new_key, type(new_key)))
161​ # only swap keys if the new key is actually different from the old one
162​ if new_key != index:
163​ vertex[new_key] = vertex[index]
jskladanUnsubmitted
Not Done

pls comment the purpose

164​ del vertex[index]
165​
156 166​
157def devise_environment(formula, arg_data): 167​def devise_environment(formula, arg_data):
158 '''Takes a parsed formula, and returns a required run-environment (i.e. 168​ '''Takes a parsed formula, and returns a required run-environment (i.e.
159 distro, arch, fedora release, and base-image flavor), based on item and type. 169​ distro, arch, fedora release, and base-image flavor), based on item and type.
160 170​
161 :param dict formula: parsed formula file (or dict with equivalent structure) 171​ :param dict formula: parsed formula file (or dict with equivalent structure)
162 :param dict arg_data: parsed command-line arguments. item, type and arch 172​ :param dict arg_data: parsed command-line arguments. item, type and arch
163 are used in this method 173​ are used in this method
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines
View Options

testing/test_taskformula.py

Show First 20 Lines • Show All 42 Lines • ▼ Show 20 Line(s)15class TestGetKeyOrAttribute(object):
43 def test_exceptions(self): 43​ def test_exceptions(self):
44 with pytest.raises(KeyError): 44​ with pytest.raises(KeyError):
45 taskformula._get_key_or_attribute({}, "foo") 45​ taskformula._get_key_or_attribute({}, "foo")
46 with pytest.raises(KeyError): 46​ with pytest.raises(KeyError):
47 taskformula._get_key_or_attribute({"foo": {}}, "foo.bar") 47​ taskformula._get_key_or_attribute({"foo": {}}, "foo.bar")
48 with pytest.raises(AttributeError): 48​ with pytest.raises(AttributeError):
49 taskformula._get_key_or_attribute({"foo": None}, "foo.bar") 49​ taskformula._get_key_or_attribute({"foo": None}, "foo.bar")
50 50​
51​
51class TestReplaceVarsInAction(object): 52​class TestReplaceVarsInAction(object):
52 53​
53 def setup_method(self, method): 54​ def setup_method(self, method):
54 self.action = { 55​ self.action = {
55 'name': 'run foo${foo}', 56​ 'name': 'run foo${foo}',
56 'export': '${export}', 57​ 'export': '${export}',
57 'python': { 58​ 'python': {
58 'file': 'somefile.py', 59​ 'file': 'somefile.py',
59 'args': ['arg1', '${number}', 'arg2', '${list}', '${dict}'], 60​ 'args': ['arg1', '${number}', 'arg2', '${list}', '${dict}'],
60 'cmdline': '-s ${foo} -i ${number}', 61​ 'cmdline': '-s ${foo} -i ${number}',
61 'nest': { 62​ 'nest': {
62 'deepnest': '${dict}', 63​ 'deepnest': '${dict}',
63 'null': '${null}', 64​ 'null': '${null}',
64 }, 65​ },
65 'from_dict': '${dict.pi}', 66​ 'from_dict': '${dict.pi}',
66 'from_object': '${number.imag}', 67​ 'from_object': '${number.imag}',
67 'from_mixed': '${dict.pi.imag}', 68​ 'from_mixed': '${dict.pi.imag}',
69​ '${dictkey}': '$foo',
68 } 70​ }
69 } 71​ }
70 self.vars = { 72​ self.vars = {
71 'foo': 'bar', 73​ 'foo': 'bar',
72 'export': 'a:b\nc:d\n-e', 74​ 'export': 'a:b\nc:d\n-e',
73 'number': 1, 75​ 'number': 1,
74 'list': [1, 'item'], 76​ 'list': [1, 'item'],
75 'dict': {'key': 'val', 'pi': 3.14}, 77​ 'dict': {'key': 'val', 'pi': 3.14},
76 'null': None, 78​ 'null': None,
77 'extra': 'juicy', 79​ 'extra': 'juicy',
80​ 'dictkey': 'a brand new key name',
78 } 81​ }
79 82​
80 def test_complex(self): 83​ def test_full_replace(self):
81 rend_action = copy.deepcopy(self.action) 84​ rend_action = copy.deepcopy(self.action)
82 rend_action['name'] = 'run foo%s' % self.vars['foo'] 85​ rend_action['name'] = 'run foo%s' % self.vars['foo']
83 rend_action['export'] = self.vars['export'] 86​ rend_action['export'] = self.vars['export']
84 rend_action['python']['args'] = ['arg1', self.vars['number'], 'arg2', 87​ rend_action['python']['args'] = ['arg1', self.vars['number'], 'arg2',
85 self.vars['list'], self.vars['dict']] 88​ self.vars['list'], self.vars['dict']]
86 rend_action['python']['cmdline'] = '-s %s -i %s' % (self.vars['foo'], 89​ rend_action['python']['cmdline'] = '-s %s -i %s' % (self.vars['foo'], self.vars['number'])
87 self.vars['number']) ​
88 rend_action['python']['nest']['deepnest'] = self.vars['dict'] 90​ rend_action['python']['nest']['deepnest'] = self.vars['dict']
89 rend_action['python']['nest']['null'] = self.vars['null'] 91​ rend_action['python']['nest']['null'] = self.vars['null']
90 rend_action['python']['from_dict'] = self.vars['dict']['pi'] 92​ rend_action['python']['from_dict'] = self.vars['dict']['pi']
91 rend_action['python']['from_object'] = self.vars['number'].imag 93​ rend_action['python']['from_object'] = self.vars['number'].imag
92 rend_action['python']['from_mixed'] = self.vars['dict']['pi'].imag 94​ rend_action['python']['from_mixed'] = self.vars['dict']['pi'].imag
95​ del rend_action['python']['${dictkey}']
96​ rend_action['python'][self.vars['dictkey']] = self.vars['foo']
93 97​
94 taskformula.replace_vars_in_action(self.action, self.vars) 98​ taskformula.replace_vars_in_action(self.action, self.vars)
95 assert self.action == rend_action 99​ assert self.action == rend_action
96 100​
97 def test_raise(self): 101​ def test_raise_missing_all_vars(self):
98 with pytest.raises(exc.TaskotronYamlError): 102​ with pytest.raises(exc.TaskotronYamlError):
99 taskformula.replace_vars_in_action(self.action, {}) 103​ taskformula.replace_vars_in_action(self.action, {})
100 104​
105​ def test_raise_missing_one_var(self):
101 del self.vars['number'] 106​ del self.vars['number']
102 with pytest.raises(exc.TaskotronYamlError): 107​ with pytest.raises(exc.TaskotronYamlError):
103 taskformula.replace_vars_in_action(self.action, self.vars) 108​ taskformula.replace_vars_in_action(self.action, self.vars)
104 109​
110​ def test_raise_dictkey_not_string(self):
111​ '''It's not possible to replace dict keys with anything but a string'''
112​ self.vars['dictkey'] = {'non-string': 'value'}
113​ with pytest.raises(exc.TaskotronYamlError):
114​ taskformula.replace_vars_in_action(self.action, self.vars)
115​
116​ def test_replace_dictkey_with_itself(self):
117​ '''After replacing dict key with exactly the same string, the key must still be present.
118​ An obvious bug would be to have that key removed (because the implementation would try
119​ to remove the "old" key)'''
120​ self.vars['dictkey'] = '${dictkey}' # I'm so sneaky
121​ taskformula.replace_vars_in_action(self.action, self.vars)
122​ assert '${dictkey}' in self.action['python']
123​ assert self.action['python']['${dictkey}'] == self.vars['foo']
124​
105 125​
106class TestReplaceVars(object): 126​class TestReplaceVars(object):
107 127​
108 def test_standadlone_var_str(self): 128​ def test_standadlone_var_str(self):
109 res = taskformula._replace_vars('${foo}', {'foo': 'hedgehog'}) 129​ res = taskformula._replace_vars('${foo}', {'foo': 'hedgehog'})
110 assert res == 'hedgehog' 130​ assert res == 'hedgehog'
111 131​
112 def test_standalone_var_nonstr(self): 132​ def test_standalone_var_nonstr(self):
▲ Show 20 Lines • Show All 127 Lines • Show Last 20 Lines