Skip to content

Fix switch state event conditions required params + make default para… #270

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

Merged

Conversation

tsurdilo
Copy link
Contributor

…m not required

Signed-off-by: Tihomir Surdilovic tsurdilo@redhat.com

Many thanks for submitting your Pull Request ❤️!

Please specify parts this PR updates:

  • [ x] Specification
  • [ x] Schema
  • Examples
  • Extensions
  • Roadmap
  • Use Cases
  • Community
  • TCK
  • Other

What this PR does / why we need it:

  1. Fixes the switch state event conditions schema (bad cut/paste)
  2. Makes the "default" property of conditions not required
    Special notes for reviewers:

Additional information (if needed):

@tsurdilo tsurdilo added the area: spec Changes in the Specification label Feb 24, 2021
@tsurdilo tsurdilo added this to the v0.6 milestone Feb 24, 2021
@@ -1950,7 +1950,7 @@ Once all actions have been performed, a transition to another state can occur.
| [stateDataFilter](#state-data-filter) | State data filter | object | no |
| [onErrors](#Error-Definition) | States error handling and retries definitions | array | no |
| eventTimeout | If eventConditions is used, defines the time period to wait for events (ISO 8601 format). For example: "PT15M" (15 minutes), or "P2DT3H4M" (2 days, 3 hours and 4 minutes)| string | yes only if eventConditions is defined |
| default | Default transition of the workflow if there is no matching data conditions or event timeout is reached. Can be a transition or end definition | object | yes |
| default | Default transition of the workflow if there is no matching data conditions or event timeout is reached. Can be a transition or end definition | object | no |
Copy link
Contributor

Choose a reason for hiding this comment

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

Imho, if default is not provided, then we should treat it like the user specified an 'end' definition, and that the doc should be updated here to clearly call that out. On the call, the alternative idea was to say that a runtime exception should be thrown, imho these are functionally the same but 'end' seems less surprising.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

imho exception is better as modellers can catch it during testing. typically not being able to transition as wanted from a switch state is a really bad thing and masking it with an "end" (workflow termination) might be very confusing imo. wdyt?

…m not required

Signed-off-by: Tihomir Surdilovic <tsurdilo@redhat.com>
@tsurdilo tsurdilo force-pushed the fixschedmaforswitchstate branch from 668e76a to 8a1f494 Compare March 5, 2021 02:51
Signed-off-by: Tihomir Surdilovic <tsurdilo@redhat.com>
@tsurdilo
Copy link
Contributor Author

tsurdilo commented Mar 5, 2021

i think there were no objections to move this forward. will go ahead and merge. please feel free to sent any update prs if there are any needed. ty

@tsurdilo tsurdilo merged commit d4ddcf7 into serverlessworkflow:master Mar 5, 2021
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants