outputDataSchema addition

[fjtirado]

This adds possibility of validating workflow output in a similar way that is already done for workflow input

See issue #611

Signed-off-by: Francisco Javier Tirado Sarti ftirados@redhat.com

Many thanks for submitting your Pull Request ❤️!

Please specify parts of this PR update:

• Specification
• Schema
• Examples
• Extensions
• Roadmap
• Use Cases
• Community
• TCK
• Other

Discussion or Issue link:

What this PR does / why we need it:

Special notes for reviewers:

Additional information:

[tsurdilo]

Missing update to the schema , see https://github.com/serverlessworkflow/specification/blob/main/schema/workflow.json#L41

Would add that data output schema is done only on wf exec success, not on failure / timeout.

Missing entry in roadmap (any pr that adds/removes functionality in spec repo needs an entry in roadmap)

Would also search older issues / discussions as to why this was previously removed. We had both input and output schemas support i believe in 0.5/6 but then it was decided to be removed, now seems we want to add it back in

[tsurdilo]

There is also question on what happens on failure to validate subflow result as this can be defined in the subflow workflow definition, so this is tied to the error handling still WIP pr. Imo they should be committed at the same time if we decided to go w/ this.

I think there is also little no no examples / explanation as to what happens if this output validation fails as to what should happen with the wf exec, can exec recover from this failure via compensation? Can it be "caught" in last workflow state error handling? Many things to look at imo

[tsurdilo]

related to #673, please do not merge one without the other

[tsurdilo]

changes requested according to comments on this pr

[fjtirado]

@tsurdilo I opened this PR because reading @cdavernas I believe this feature was accepted.
In my view, the output validation should be performed only for parent process (not subprocess) and if it fails, it behave as when input validation fails: it throws an exception if the failOnValidation is true.
Since it only applies to parent process, we can consider it is is not part of error handling feature, because it does not really belong to flow execution. Thats why I just change the dataInputShema bits to add dataOuputSchema.

[tsurdilo]

Yeah definitely lets add it, just would be nice to update the little things mentioned. Ty!!

[fjtirado]

@tsurdilo Sure, Ill do, thanks

[fjtirado]

@tsurdilo I tried to reflect your concerns into the text.

[cdavernas]

Please consider #705 before merging, as requested change might be included in this PR as well.

[ricardozanini]

@tsurdilo can you please take a final look so we can merge it? Thank you!

[cdavernas]

@tsurdilo Can you please review requested changes?

[cdavernas]

Linked to #705

[ricardozanini]

@tsurdilo can you please take a final look so we can merge it? Thank you!

[tsurdilo]

@ricardozanini

If its please possible to change the naming convention:
"inputDataSchema"
"outputDataSchema"

back to dataInputSchema, dataOutputSchema

then +1 from me :) I think input/outputDataSchema reads weird, what is a "DataSchema"?

[tsurdilo]

left couple of small change requests for the docs part hope thats ok

[fjtirado]

@ricardozanini

If its please possible to change the naming convention: "inputDataSchema" "outputDataSchema"

back to dataInputSchema, dataOutputSchema

then +1 from me :) I think input/outputDataSchema reads weird, what is a "DataSchema"?

@tsurdilo If I recalled correctly this was a comment from @cdavernas , I think the change is to reflect that both data schemas are the same, acting as input or output

[tsurdilo]

I see, i think both says they are the same, they are both schemas one defines data input and the other data output of the workflow exec

[ricardozanini]

@tsurdilo @cdavernas @fjtirado I'd rather stay with inputDataSchema and outputDataSchema if you don't mind. @tsurdilo I think all your comments have been handled. Can you take a final look/approve so we can merge this?

[cdavernas]

@tsurdilo @cdavernas @fjtirado I'd rather stay with inputDataSchema and outputDataSchema if you don't mind.

Works for me either way! I guess it's just a matter of point of view/personal prefs anyways

[ricardozanini]

@cdavernas I agree. Either works for me too if we can move forward!

[fjtirado]

@tsurdilo Any further request?

[tsurdilo]

@fjtirado hi, no new requests :) Just same one to use dataInputSchema/dataOutputSchema please if possible. Also left a small comment asking for update/correction in readme.md. Thanks.

[ricardozanini]

@cdavernas any other comments?

[cdavernas]

@ricardozanin Nope, looking great to me! Thanks @fjtirado