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

Make do an array #895

Conversation

matthias-pichler
Copy link
Collaborator

@matthias-pichler matthias-pichler commented Jun 11, 2024

Please specify parts of this PR update:

  • Specification
  • Schema
  • Examples
  • Extensions
  • Use Cases
  • Community
  • CTK
  • Other

Discussion or Issue link:

Closes #894

What this PR does:

  • I refactored the schema to make do, try, catch.do, do in for, before and after an array of named tasks
  • I refactored the schema to only allowed desired properties
  • I added test to make sure the schema rejects invalid definitions
  • I added tests to check every example in the dsl-reference doc
  • I updated the ctk and examples with the new schema

Additional information:

Now the usage of do both in for Tasks as well as sequential composite tasks lead to the result that for tasks now also matched the schema of the composite sequential task. As discussed earlier this is because additional/unevaluated properties were not restricted. This lead to the error that a for task was also recognized as a do task and hence matched two schemas in oneOf ... I therefore refactored the schema to distinguish between these and no allow additional properties

@matthias-pichler matthias-pichler force-pushed the matthias-pichler/invalid-do-switch-894 branch from 43d0fb2 to a2d5cd2 Compare June 12, 2024 07:03
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
@matthias-pichler matthias-pichler force-pushed the matthias-pichler/invalid-do-switch-894 branch from ab2e14c to ff4fb6e Compare June 12, 2024 07:08
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
@matthias-pichler matthias-pichler marked this pull request as ready for review June 12, 2024 11:55
@fjtirado
Copy link
Collaborator

@matthias-pichler-warrify
There are merge conflicts

@matthias-pichler
Copy link
Collaborator Author

@matthias-pichler-warrify There are merge conflicts

yep, @cdavernas just merged the rename from output.from to output.as 👍 they are resolved now

dsl-reference.md Outdated Show resolved Hide resolved
ctk/features/composite.feature Outdated Show resolved Hide resolved
dsl-reference.md Show resolved Hide resolved
dsl-reference.md Outdated Show resolved Hide resolved
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Great work, many thanks! Once we split the task section as pointed by @cdavernas I'll do another round.

.ci/validation/src/examples.test.ts Show resolved Hide resolved
ctk/features/data-flow.feature Outdated Show resolved Hide resolved
ctk/features/for.feature Outdated Show resolved Hide resolved
schema/workflow.yaml Outdated Show resolved Hide resolved
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
Signed-off-by: Matthias Pichler <[email protected]>
dsl.md Outdated Show resolved Hide resolved
Copy link
Member

@cdavernas cdavernas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Many thanks ❤️

@cdavernas cdavernas merged commit f574808 into serverlessworkflow:main Jun 13, 2024
3 checks passed
@matthias-pichler matthias-pichler deleted the matthias-pichler/invalid-do-switch-894 branch June 13, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid do:switch
5 participants