Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Define specific rules when merging action output by default #712

Closed
cdavernas opened this issue Nov 15, 2022 · 8 comments · Fixed by #832
Closed

Define specific rules when merging action output by default #712

cdavernas opened this issue Nov 15, 2022 · 8 comments · Fixed by #832
Assignees
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Milestone

Comments

@cdavernas
Copy link
Member

What would you like to be added:

Define specific rules when merging action output by default

Why is this needed:

So far, the spec mandates that the output of an action without toState data filter should be merged back 'as is' into the state data.
Problem is, if the output of said action is not an object, merging it into state data will effectilvely fault the whole workflow: all evaluations will fail.

Image for instance an action that returns, say, an array or, why not, an integer. Your state was the following beforehand:

{
  "someName": "someValue"
}

After the action, the state data would be:

[
 1,
 2,
 3
]

Or why not something like:

125

Which would result in all further actions to fail. Consider, for example, a next action with a fromState data filter => BOOM.

What I propose is that the action output be merged back "as-is" ONLY if/when it's an object. If it's an array or a value type (string, bool, int, ...), it should instead be merged back into a 'actionOuput' property, for example. The name of that property could be based on the name of the action, such as myActionOutput.

WDYT?

@cdavernas cdavernas added change: feature New feature or request. Impacts in a minor version change area: spec Changes in the Specification labels Nov 15, 2022
@fjtirado
Copy link
Collaborator

@cdavernas, what we did in Kogito for those cases is to merge the non object json node with a "response" name.

@ricardozanini
Copy link
Member

Just to illustrate using your example, we then have an output like this:

{
  "someName": "someValue",
  "response": [1, 2, 3]
}

But it's definitely doable to have a definition at the spec level of what to do in such situations.

@cdavernas
Copy link
Member Author

what we did in Kogito for those cases is to merge the non object json node with a "response" name.

Common sense indeed, but it would imho be nice to specify that in the spec so that users don't "discover" inconsistencies amongst runtimes.

@ricardozanini
Copy link
Member

Oh sure thing, I agree entirely with you. I remember we brought this to discussion, but didn't go further, so we did it by ourselves

@github-actions
Copy link

github-actions bot commented Jan 1, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

github-actions bot commented Apr 4, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

fjtirado added a commit to fjtirado/specification that referenced this issue Mar 18, 2024
Update specification to make runtimes predicateble when an action
produced a non json object and there is not toStateData filter
fjtirado added a commit to fjtirado/specification that referenced this issue Mar 18, 2024
Update specification to make runtimes predicateble when an action
produced a non json object and there is not toStateData filter

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
fjtirado added a commit to fjtirado/specification that referenced this issue Mar 18, 2024
fjtirado added a commit to fjtirado/specification that referenced this issue Mar 18, 2024
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
fjtirado added a commit to fjtirado/specification that referenced this issue Apr 4, 2024
fjtirado added a commit to fjtirado/specification that referenced this issue Apr 4, 2024
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
fjtirado added a commit to fjtirado/specification that referenced this issue Apr 4, 2024
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
cdavernas added a commit that referenced this issue May 15, 2024
[Fix #712] Describe merge behaviour for non object
cdavernas pushed a commit to neuroglia-io/serverless-workflow-specification that referenced this issue May 21, 2024
Update specification to make runtimes predicateble when an action
produced a non json object and there is not toStateData filter

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
cdavernas pushed a commit to neuroglia-io/serverless-workflow-specification that referenced this issue May 21, 2024
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
cdavernas pushed a commit to neuroglia-io/serverless-workflow-specification that referenced this issue May 21, 2024
Signed-off-by: Francisco Javier Tirado Sarti <ftirados@redhat.com>
Signed-off-by: Charles d'Avernas <charles.davernas@neuroglia.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: spec Changes in the Specification change: feature New feature or request. Impacts in a minor version change
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants