Differential D20

added simple yaml validation
ClosedPublic
Actions

Authored by garretraziel on Feb 28 2014, 9:10 AM.

Details

Reviewers
kparal
tflink
mkrizek
Maniphest Tasks
T87: Check if task's input is a valid yaml file
Commits
rLTRN518f6eac4398: added simple yaml validation
Summary

Added some simple yaml validation for "input", "args" and "task" sections.

Problem is that almost everything is valid yaml, so using python-Rx and
writing schema would be better solution.

Test Plan

Run:
python runtask.py -e xchat-2.8.8-21.fc20 -a x86_64 yaml_file
where yaml_file is correct taskotron task file, empty file, task file
where "input:", "args:" and/or "task:" sections are missing or task without
task list in "tasks:" section.

Diff Detail

Branch
develop
Lint
No Linters Available
Unit
No Unit Test Coverage
ralph added a comment.Feb 28 2014, 2:24 PM

Are there any unit tests that could be associated with this change?

libtaskotron/runner.py
100

Any opinion on making this a KeyError instead of just an Exception? It is an error that is raised when a "key" is missing from the taskdata dict.

112

Same here.

148

This could perhaps be a ValueError?

garretraziel added a comment.Feb 28 2014, 3:30 PM

I think that the best solution is to catch those errors and report it to user nicely, but I wanted to write it like it is written on line 117 to be consistent with existing code.

tflink added a comment.Feb 28 2014, 3:45 PM

I think that the best solution is to catch those errors and report it to user nicely, but I wanted to write it like it is written on line 117 to be consistent with existing code.

The existing code needs to be changed at some point, though. It was never my intention to leave everything as base Exceptions.

Kamil added some custom exceptions yesterday, maybe adding a TaskotronYamlError or TaskotronInputError would be appropriate here.

garretraziel updated this revision.Mar 4 2014, 9:35 AM
mkrizek requested changes to this revision.Mar 4 2014, 11:55 AM

I'd like to see some unit tests, but otherwise it looks good to me.

garretraziel updated this revision.Mar 4 2014, 12:57 PM
  • added unit tests for YAML validation
garretraziel updated this revision.Mar 4 2014, 2:13 PM
  • corrected condition where "input" can be string
tflink accepted this revision.Mar 4 2014, 4:40 PM

Looks good to me, thanks for adding those tests

mkrizek accepted this revision.Mar 5 2014, 9:56 AM

LGTM

garretraziel closed this revision.Mar 6 2014, 8:51 AM

Revision Contents

Diff 54

View Options

libtaskotron/exceptions.py

Show All 9 Lines
10 10​
11class TaskotronValueError(ValueError, TaskotronError): 11​class TaskotronValueError(ValueError, TaskotronError):
12 '''Taskotron-specific ValueError''' 12​ '''Taskotron-specific ValueError'''
13 pass 13​ pass
14 14​
15class TaskotronConfigError(TaskotronError): 15​class TaskotronConfigError(TaskotronError):
16 '''All errors related to Taskotron config files''' 16​ '''All errors related to Taskotron config files'''
17 pass 17​ pass
18​
19​class TaskotronYamlError(TaskotronError):
20​ '''Error in YAML config file of the executed check'''
21​ pass
View Options

libtaskotron/runner.py

1import logging 1​import logging
2import tempfile 2​import tempfile
3import os.path 3​import os.path
4import argparse 4​import argparse
5import imp 5​import imp
6from jinja2 import Template, Environment 6​from jinja2 import Template, Environment
7import copy 7​import copy
8 8​
9from libtaskotron import taskyaml 9​from libtaskotron import taskyaml
10from libtaskotron import logger 10​from libtaskotron import logger
11from libtaskotron.logger import log 11​from libtaskotron.logger import log
12​from libtaskotron.exceptions import TaskotronYamlError
12 13​
13class Runner: 14​class Runner:
14 def __init__(self, taskdata, argdata, workdir=None): 15​ def __init__(self, taskdata, argdata, workdir=None):
15 self.taskdata = taskdata 16​ self.taskdata = taskdata
16 self.envdata = argdata 17​ self.envdata = argdata
17 self.working_data = {} 18​ self.working_data = {}
18 self.directives = {} 19​ self.directives = {}
19 self.jinja_env = None 20​ self.jinja_env = None
▲ Show 20 Lines • Show All 70 Lines • ▼ Show 20 Line(s)80 def do_single_action(self, action):
90 91​
91 output = directive_callable.process(rendered_action[directive_name], 92​ output = directive_callable.process(rendered_action[directive_name],
92 self.envdata) 93​ self.envdata)
93 94​
94 if 'export' in action: 95​ if 'export' in action:
95 self.working_data[action['export']] = output 96​ self.working_data[action['export']] = output
96 97​
97 def do_actions(self): 98​ def do_actions(self):
99​ if 'task' not in self.taskdata or not self.taskdata['task']:
100​ raise TaskotronYamlError("At least one task should be specified in input yaml file")
ralphUnsubmitted
Not Done

Any opinion on making this a KeyError instead of just an Exception? It is an error that is raised when a "key" is missing from the taskdata dict.

101​
98 for action in self.taskdata['task']: 102​ for action in self.taskdata['task']:
99 self.do_single_action(action) 103​ self.do_single_action(action)
100 104​
101 def _validate_input(self): 105​ def _validate_input(self):
106​ if 'input' not in self.taskdata:
107​ return
108​
109​ if not isinstance(self.taskdata['input'], dict):
110​ raise TaskotronYamlError("Input yaml should contain correct 'input' section")
111​
112​ if ('args' not in self.taskdata['input']) or not isinstance(self.taskdata['input']['args'], str):
ralphUnsubmitted
Not Done

Same here.

113​ raise TaskotronYamlError("Input yaml should contain correct 'args' section")
114​
102 required_input = self.taskdata['input']['args'].split(',') 115​ required_input = self.taskdata['input']['args'].split(',')
103 116​
104 for arg in required_input: 117​ for arg in required_input:
105 if not arg in self.envdata: 118​ if not arg in self.envdata:
106 raise Exception('Required input arg %s was not defined' % arg) 119​ raise TaskotronYamlError('Required input arg %s was not defined' % arg)
107 120​
108 def _validate_env(self): 121​ def _validate_env(self):
109 raise NotImplementedError("Environment validation is not yet implemented") 122​ raise NotImplementedError("Environment validation is not yet implemented")
110 123​
111 124​
112def get_argparser(): 125​def get_argparser():
113 parser = argparse.ArgumentParser() 126​ parser = argparse.ArgumentParser()
114 parser.add_argument("task", nargs=1, help="task to run") 127​ parser.add_argument("task", nargs=1, help="task to run")
Show All 11 Lines136def main():
126 139​
127 logger.init(level=logging.DEBUG) 140​ logger.init(level=logging.DEBUG)
128 141​
129 arg_data = vars(args) 142​ arg_data = vars(args)
130 arg_data['taskfile'] = args.task[0] 143​ arg_data['taskfile'] = args.task[0]
131 144​
132 task_data = taskyaml.parse_yaml_from_file(arg_data['taskfile']) 145​ task_data = taskyaml.parse_yaml_from_file(arg_data['taskfile'])
133 146​
147​ if not task_data:
148​ raise TaskotronYamlError('Input file should not be empty')
ralphUnsubmitted
Not Done

This could perhaps be a ValueError?

149​
134 task_runner = Runner(task_data, arg_data) 150​ task_runner = Runner(task_data, arg_data)
135 task_runner.run() 151​ task_runner.run()
136 152​
137 for output in task_runner.working_data: 153​ for output in task_runner.working_data:
138 log.info(task_runner.working_data[output]) 154​ log.info(task_runner.working_data[output])
View Options

testing/test_runner.py

1import pytest 1​import pytest
2import os 2​import os
3import sys 3​import sys
4 4​
5from libtaskotron import runner 5​from libtaskotron import runner
6 6​from libtaskotron.exceptions import TaskotronYamlError
7 7​
8 8​
9class TestRunnerInputVerify(): 9​class TestRunnerInputVerify():
10 def test_yamlrunner_valid_input(self): 10​ def test_yamlrunner_valid_input(self):
11 ref_cmd = '-e foo-1.2-3.fc99 -a x86_64 footask.yml' 11​ ref_cmd = '-e foo-1.2-3.fc99 -a x86_64 footask.yml'
12 ref_argdata = {'input': {'args': 'envr,arch'}} 12​ ref_argdata = {'input': {'args': 'envr,arch'}}
13 ref_data = {'envr': 'foo-1.2-3.fc99', 'arch': 'x86_64', 'taskfile': 'footask.yml'} 13​ ref_data = {'envr': 'foo-1.2-3.fc99', 'arch': 'x86_64', 'taskfile': 'footask.yml'}
14 14​
15 test_parser = runner.get_argparser() 15​ test_parser = runner.get_argparser()
16 test_args = vars(test_parser.parse_args(ref_cmd.split())) 16​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
17 17​
18 test_runner = runner.Runner(ref_argdata, test_args) 18​ test_runner = runner.Runner(ref_argdata, test_args)
19 19​
20 test_runner._validate_input() 20​ test_runner._validate_input()
21 21​
22​ def test_yamlrunner_missing_input(self):
23​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
24​ ref_inputspec = {}
25​
26​ test_parser = runner.get_argparser()
27​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
28​
29​ test_runner = runner.Runner(ref_inputspec, test_args)
30​
31​ test_runner._validate_input()
32​
33​ def test_yamlrunner_fails_empty_input(self):
34​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
35​ ref_inputspec = {'input': None}
36​
37​ test_parser = runner.get_argparser()
38​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
39​
40​ test_runner = runner.Runner(ref_inputspec, test_args)
41​
42​ with pytest.raises(TaskotronYamlError):
43​ test_runner._validate_input()
44​
45​ def test_yamlrunner_fails_empty_arg(self):
46​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
47​ ref_inputspec = {'input': {'args': None}}
48​
49​ test_parser = runner.get_argparser()
50​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
51​
52​ test_runner = runner.Runner(ref_inputspec, test_args)
53​
54​ with pytest.raises(TaskotronYamlError):
55​ test_runner._validate_input()
56​
22 def test_yamlrunner_fails_missing_arg(self): 57​ def test_yamlrunner_fails_missing_arg(self):
23 ref_cmd = '-e foo-1.2-3.fc99 footask.yml' 58​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
24 ref_inputspec = {'args':'envr,arch'} 59​ ref_inputspec = {'input': 'other_item'}
25 60​
26 test_parser = runner.get_argparser() 61​ test_parser = runner.get_argparser()
27 test_args = vars(test_parser.parse_args(ref_cmd.split())) 62​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
28 63​
29 test_runner = runner.Runner(ref_inputspec, test_args) 64​ test_runner = runner.Runner(ref_inputspec, test_args)
30 65​
31 with pytest.raises(Exception): 66​ with pytest.raises(TaskotronYamlError):
67​ test_runner._validate_input()
68​
69​ def test_yamlrunner_fails_undefined_arg(self):
70​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
71​ ref_inputspec = {'input': {'args': 'nonexistent_input_arg'}}
72​
73​ test_parser = runner.get_argparser()
74​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
75​
76​ test_runner = runner.Runner(ref_inputspec, test_args)
77​
78​ with pytest.raises(TaskotronYamlError):
79​ test_runner._validate_input()
80​
81​ def test_yamlrunner_fails_input_is_string(self):
82​ ref_cmd = '-e foo-1.2-3.fc99 footask.yml'
83​ ref_inputspec = {'input': {'args'}}
84​
85​ test_parser = runner.get_argparser()
86​ test_args = vars(test_parser.parse_args(ref_cmd.split()))
87​
88​ test_runner = runner.Runner(ref_inputspec, test_args)
89​
90​ with pytest.raises(TaskotronYamlError):
32 test_runner._validate_input() 91​ test_runner._validate_input()
33 92​
34class TestRunnerSetup(): 93​class TestRunnerSetup():
35 def test_trivial_creation(self, tmpdir): 94​ def test_trivial_creation(self, tmpdir):
36 ref_taskdata = {'preparation': {'koji': 'download envr'}, 95​ ref_taskdata = {'preparation': {'koji': 'download envr'},
37 'input': {'args': 'envr,arch'}, 'post': {'shell': 'clean'}, 96​ 'input': {'args': 'envr,arch'}, 'post': {'shell': 'clean'},
38 'execution': {'python': 'run_rpmlint.py'}, 97​ 'execution': {'python': 'run_rpmlint.py'},
39 'dependencies': ['rpmlint', 'libtaskbot']} 98​ 'dependencies': ['rpmlint', 'libtaskbot']}
▲ Show 20 Lines • Show All 160 Lines • ▼ Show 20 Line(s)249 def test_runner_do_multiple_dummy_pass_export_rendered_msg(self):
200 259​
201 test_runner = runner.Runner(self.ref_taskdata, self.ref_inputdata) 260​ test_runner = runner.Runner(self.ref_taskdata, self.ref_inputdata)
202 test_runner.working_data[ref_messagename] = ref_message 261​ test_runner.working_data[ref_messagename] = ref_message
203 262​
204 test_runner.do_actions() 263​ test_runner.do_actions()
205 264​
206 assert test_runner.working_data[ref_exportname] == ref_message 265​ assert test_runner.working_data[ref_exportname] == ref_message
207 266​
267​ def test_runner_fails_missing_action(self):
268​ self.ref_taskdata = {'task': None}
269​
270​ test_runner = runner.Runner(self.ref_taskdata, self.ref_inputdata)
271​
272​ with pytest.raises(TaskotronYamlError):
273​ test_runner.do_actions()
274​
275​ def test_runner_fails_missing_task(self):
276​ self.ref_taskdata = {}
277​
278​ test_runner = runner.Runner(self.ref_taskdata, self.ref_inputdata)
279​
280​ with pytest.raises(TaskotronYamlError):
281​ test_runner.do_actions()
282​
208 283​
209class TestRunnerRenderAction(): 284​class TestRunnerRenderAction():
210 285​
211 def setup_method(self, method): 286​ def setup_method(self, method):
212 self.ref_taskdata = {} 287​ self.ref_taskdata = {}
213 self.ref_inputdata = {} 288​ self.ref_inputdata = {}
214 289​
215 def test_runner_render_single_variable(self): 290​ def test_runner_render_single_variable(self):
Show All 12 Lines