As I'm working on the taskotron runner, I've been trying to work through a good way to declare actions in the task yaml. The current code allows for one arg and multiple key=value pairs but I don't like the limitation of just one non-key=value arg and would like to eliminate them entirely.
I figure that this will be easier to show with an example so I'll use the following action which would be valid with the current code:
------------------------------------------------------------ Execution: python: runtask.py method_name=do_something action=fix ------------------------------------------------------------
The intention of this action is to import a python file called runtask.py, find and execute a method in that file named "do_something" and pass in action="fix" to that method, effectively leaving us with a call similar to:
do_something(action='fix')
Looking strictly at the yaml, there is a mix of args and key=value pairs. 'runtask.py' is an arg, the other two are key=value. These are currently passed into the python directive as a string and a dictionary, roughly equivalent to:
command = 'runtask.py' input_data = {'method_name':'do_something', 'action':'fix'} env_data = get_envdata() # runtime information, like workdir process(command, input_data, env_data)
What I'm looking to decide is whether we really need args in addition to of key=value pairs and if the added complexity is worth it.
I propose that we alter the format to use only key=value pairs which would be parsed into a dict in the runner. Using the same example as above, the new action would look something like:
------------------------------------------------------------ Execution: python: pyfile=runtask.py method_name=do_something action=fix ------------------------------------------------------------
This would simplify the parsing code and the data passing logic for execution since all input data would be in the form of a dictionary instead of separated out into a list/string and a dictionary.
While I slightly prefer the aesthetics of allowing one or more keyless args in an action instead of all key=value pairs, I can't think of a use case where restricting input to key=value pairs would cause problems.
If we do decide that keyless args are needed, I'd rather support more than one instead of the current arbitrary restriction of just one arg.
Any other thoughts?
Tim
On Tue, Feb 04, 2014 at 02:02:42PM -0700, Tim Flink wrote:
---- SNIP ----
What I'm looking to decide is whether we really need args in addition to of key=value pairs and if the added complexity is worth it.
I propose that we alter the format to use only key=value pairs which would be parsed into a dict in the runner. Using the same example as above, the new action would look something like:
Execution: python: pyfile=runtask.py method_name=do_something action=fix
This would simplify the parsing code and the data passing logic for execution since all input data would be in the form of a dictionary instead of separated out into a list/string and a dictionary.
While I slightly prefer the aesthetics of allowing one or more keyless args in an action instead of all key=value pairs, I can't think of a use case where restricting input to key=value pairs would cause problems.
If we do decide that keyless args are needed, I'd rather support more than one instead of the current arbitrary restriction of just one arg.
Any other thoughts?
Tim
How about something like this:
------------------------------------------------------------ Execution: python: file: runtask.py method: do_something args: - first_positional_arg - second_positional_arg kwargs: action: fix ------------------------------------------------------------
It takes many more lines to express a task, but the indentation shows structure and it is much more flexible. Perhaps it is overkill?
On Wed, 5 Feb 2014 05:43:16 -0500 Ralph Bean rbean@redhat.com wrote:
On Tue, Feb 04, 2014 at 02:02:42PM -0700, Tim Flink wrote:
---- SNIP ----
What I'm looking to decide is whether we really need args in addition to of key=value pairs and if the added complexity is worth it.
I propose that we alter the format to use only key=value pairs which would be parsed into a dict in the runner. Using the same example as above, the new action would look something like:
Execution: python: pyfile=runtask.py method_name=do_something action=fix
This would simplify the parsing code and the data passing logic for execution since all input data would be in the form of a dictionary instead of separated out into a list/string and a dictionary.
While I slightly prefer the aesthetics of allowing one or more keyless args in an action instead of all key=value pairs, I can't think of a use case where restricting input to key=value pairs would cause problems.
If we do decide that keyless args are needed, I'd rather support more than one instead of the current arbitrary restriction of just one arg.
Any other thoughts?
Tim
How about something like this:
Execution: python: file: runtask.py method: do_something args: - first_positional_arg - second_positional_arg kwargs: action: fix
It takes many more lines to express a task, but the indentation shows structure and it is much more flexible. Perhaps it is overkill?
It's certainly easier to read for more complex actions and does make it easier to pass in stuff like lists. For the python directive, it also makes the treatment of args and kwargs passed into the python callable much more explicit.
The only concern I have is that it requires a bit more knowledge of how yaml works and is parsed. For example, the difference between
------------------------------------------------------------ args: - first_positional_arg - second_positional_arg ------------------------------------------------------------
and
------------------------------------------------------------ args: first_positional_arg second_positional_arg ------------------------------------------------------------
The first is parsed as a list (['first_positional_arg', 'second_positional_arg']) and the second is parsed as a space delimited string ('first_positional_arg second_positional_arg'). I'm not sure how we could check for validity on a generic basis since there are valid uses for both parsed forms.
Either way, I think that it makes sense to do away with the non-key-value inputs. Now the question becomes what format the actions should take.
For a larger example, let's look at an excerpt from rpmlint. This is an example of what a set of actions could look like using key=value pairs and not taking advantage of many yaml structures.
----------------------------------------------------------- - name: download rpms from koji koji: action=download envr={{ envr }}
- name: run rpmlint on downloaded rpms python: file=run_rpmlint.py workdir={{ workdir }} register: rpmlint_output
- name: report results to resultsdb resultdb: results={{ rpmlint_output }} -----------------------------------------------------------
It's somewhat dense but for this example, it's not that difficult to read.
For an example with the structures mentioned above:
------------------------------------------------------------ Execution: - name: run createrepo createrepo: action: create dir: "{{workdir}}/somedir"
- name: run rpmlint on downloaded rpms python: file: run_rpmlint.py workdir: "{{ workdir }}" register: rpmlint_output
- name: report results to resultsdb resultdb: results: "{{ rpmlint_output }}" -----------------------------------------------------------
There are some changes to the way that the actions are represented but it's much easier for my human eyes to parse the actions since it's effectively limited to one input per line.
I'm kind of partial to the second form, myself. Any other thoughts?
Tim
It's certainly easier to read for more complex actions and does make it easier to pass in stuff like lists. For the python directive, it also makes the treatment of args and kwargs passed into the python callable much more explicit.
The only concern I have is that it requires a bit more knowledge of how yaml works and is parsed.
Yes, there are even some pitfalls, mainly with string detection: http://en.wikipedia.org/wiki/YAML#Pitfalls_and_implementation_defects
I assume we can provide a script as part of our "SDK" (heh) that will: 1. validate the yaml file (I haven't found any simple CLI yaml validator anyway, that wouldn't start with "python -c" or "perl -e") 2. re-print what is going to happen in some pseudo syntax (for example the directives can be shown as real shell commands if possible, variables evaluated, etc) so that people can verify it's going to do what they want
Alternatively, all of this can be a part of our standard execution process and displayed as a part of the debugging info.
For a larger example, let's look at an excerpt from rpmlint. This is an example of what a set of actions could look like using key=value pairs and not taking advantage of many yaml structures.
- name: download rpms from koji koji: action=download envr={{ envr }} - name: run rpmlint on downloaded rpms python: file=run_rpmlint.py workdir={{ workdir }} register: rpmlint_output - name: report results to resultsdb resultdb: results={{ rpmlint_output }}
It's somewhat dense but for this example, it's not that difficult to read.
For an example with the structures mentioned above:
Execution: - name: run createrepo createrepo: action: create dir: "{{workdir}}/somedir"
- name: run rpmlint on downloaded rpms python: file: run_rpmlint.py workdir: "{{ workdir }}" register: rpmlint_output - name: report results to resultsdb resultdb: results: "{{ rpmlint_output }}"
There are some changes to the way that the actions are represented but it's much easier for my human eyes to parse the actions since it's effectively limited to one input per line.
I'm kind of partial to the second form, myself. Any other thoughts?
The former one looks better, the latter one is more readable. Especially when things get complex, the latter one might be a win. I'm OK with both approaches.
qa-devel@lists.fedoraproject.org
-
Kamil Paral -
Ralph Bean -
Tim Flink