Fix issues with exe/floe and various combinations of workflow and input #174
Conversation
This commit cleans up the exe/floe interface for bare workflow/input pairs
when passed in combination with `--workflow` and `--input`. This commit
also makes the `--workflow` and `--input` params no longer legacy but
supported.
The following are now all accepted ways to pass these parameters:
```sh
# Single workflow
exe/floe workflow1.asl
exe/floe workflow1.asl '{"input":1}'
exe/floe workflow1.asl --input '{"input":1}'
exe/floe --workflow workflow1.asl --input '{"input":1}'
# Multiple workflows
exe/floe workflow1.asl '{"input":1}' workflow2.asl '{"input":2}'
exe/floe workflow1.asl workflow2.asl --input '{"input":1}' # input is dup'd for each workflow
```
The following will raise errors:
```sh
exe/floe workflow1.asl workflow2.asl # ambiguous as to workflow/input pair or 2 workflows, and so requires input
exe/floe workflow1.asl --workflow workflow2.asl # invalid mixing of bare workflows and parameter
exe/floe workflow1.asl workflow2.asl '{"input":1}' # mismatched workflow/input pairs
exe/floe --input '{"input":1}' # missing workflow
exe/floe # no parameters
```
| RSpec.describe "exe/floe" do | ||
| RSpec.describe "exe/floe", :slow => true do |
There was a problem hiding this comment.
Thought this might be useful, since these tests are kinda slow, so they can be ignored in local dev with
--tag ~slow
| workflow | ||
| === | ||
| {"foo"=>1} | ||
|
|
||
| workflow | ||
| === | ||
| {"foo"=>2} |
There was a problem hiding this comment.
This was surprising to me (here and of course everywhere else), as I assumed the output would be JSON formatted and not Ruby formatted. So, the question is, should it be JSON? If so, it's an easy fix for a followup PR.
There was a problem hiding this comment.
I also expected either the workflow.asl file name or perhaps the workflow comment (or both), so just the word "workflow" was also surprising.
There was a problem hiding this comment.
I modified this to run multiple workflows.
Goals:
- Run multiple workflows.
- Run in a way similar to how the provider runs.
- Allow easy migration to the non-polling code.
- Easily determine that all workflows were run and their output status.
It did not take into consideration a computer parsing the output.
Modifying it sounds great.
There was a problem hiding this comment.
I wasn't really thinking about post processing, but that would be useful in the single workflow case. Even so, it was more of an expectation thing...for some reason I expected a json output.
|
also, can we drop Don't see any reason to support multiple ways to specify these values. Thoughts? |
|
No this PR makes them a) permanent (I use them personally and prefer them) and b) input, specifically, is useful for multiple workflows that take the same input cause you only need to specify it once |
|
I could see maybe removing --workflow in a future PR, but --input has a use case. |
|
Checked commit Fryguy@7ecc3c7 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
| # Create workflow/input pairs from the various combinations of paramaters | ||
| args = | ||
| if opts[:workflow_given] | ||
| Optimist.die("cannot specify both --workflow and bare workflows") if ARGV.any? | ||
|
|
||
| [opts[:workflow], opts.fetch(:input, "{}")] | ||
| elsif opts[:input_given] | ||
| Optimist.die("workflow(s) must be specified") if ARGV.empty? | ||
|
|
||
| ARGV.flat_map { |w| [w, opts[:input].dup] } | ||
| else | ||
| Optimist.die("workflow/input pairs must be specified") if ARGV.empty? || (ARGV.size > 1 && ARGV.size.odd?) | ||
|
|
||
| ARGV | ||
| end |
There was a problem hiding this comment.
👍 like how this creates a consistent set of args regardless of how exe/floe was invoked and we don't need to do input || opts[:input] || "{}" below
This commit cleans up the exe/floe interface for bare workflow/input pairs when passed in combination with
--workflowand--input. This commit also makes the--workflowand--inputparams no longer legacy but supported.The following are now all accepted ways to pass these parameters:
The following will raise errors: