ManageIQ / floe

Floe is a runner for Amazon States Language workflows
Apache License 2.0
0 stars 5 forks source link

Handle non-hash input/output values #214

Closed agrare closed 3 months ago

agrare commented 4 months ago

We currently assume that input/output have to be a JSON/hash payload, however while looking at some map examples it seems like it is possible for simple strings as well (and I assume by extension integers, etc...).

Example: https://docs.aws.amazon.com/step-functions/latest/dg/tutorial-use-inline-map.html

Input to Map state:

{
  "foo": "bar",
  "colors": [
    "red",
    "green",
    "blue",
    "yellow",
    "white"
  ]
}

So with "ItemsPath": "$.colors" the input to the "ItemProcessor" would be e.g. "red" and the output would be a simple UUID string.

Fryguy commented 4 months ago

Not sure I understand - it seems like the input to Map is an array always. Or are you saying the contents of the Array itself can be anything (in this case an Array of Strings as opposed to an Array of Inputs/Hashes)?

agrare commented 4 months ago

Or are you saying the contents of the Array itself can be anything

^ this

In the step-functions examples the input to Map is ["red", "green", etc...] which means the input to the sub-workflow is just "red"

agrare commented 4 months ago

Also I think there isn't anything to prevent you from passing e.g. "red" as the top-level workflow input (if you don't have a Map state that is)

kbrock commented 4 months ago

Can we solve this by removing the JSON parsing? Instead we can put that parsing into manageiq-workflow-providers, exe/floe, and possibly in Task.

I've noticed that a lambda (task) can return a string (not json) and it will work. The basic guide stated some advantages of this. Not sure if we want to support this behavior. Think we throw away the input if it is not json.

We had an issue when we were merging the output with the input for the next State. That forced us to always deal with Json. Now, we may be able to better support non-hash values

agrare commented 4 months ago

Can we solve this by removing the JSON parsing?

Oh that's interesting, yeah that might be the way to go

miq-bot commented 3 months ago

Checked commit https://github.com/agrare/floe/commit/a943da6a2754f5830d30d84d85db5694cf61a547 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint 3 files checked, 0 offenses detected Everything looks fine. :cake:

kbrock commented 3 months ago

I really like this PR

Fryguy commented 3 months ago

Just wanted to add some clarifications here (as discussed with @agrare in PM). Strings, integers, etc are JSON. I've updated the title to say non-hash (and Adam updated his commit before it was merged)

[1] pry(main)> require "json"
=> true
[2] pry(main)> "test".to_json
=> "\"test\""
[3] pry(main)> JSON.parse(_)
=> "test"
[4] pry(main)> 1.to_json
=> "1"
[5] pry(main)> JSON.parse(_)
=> 1