-
Notifications
You must be signed in to change notification settings - Fork 39
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
Fail Fast control for with item task #245
base: master
Are you sure you want to change the base?
Conversation
…complete tasks too` in add_context_to_task_item_event
…NTINUE to continue even if no current active task but incomplete task(s)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh. I really like ux for this!
Please add some tests so we can move this forward.
I believe this will address StackStorm/st2#4968
Something like this was requested in StackStorm/st2#5054 (comment) The rest of that issue discusses failfast across parallel workflow branches. This PR only addresses the piece mentioned in that comment: failfast for with items loops.
And though this is slightly orthogonal, I think this PR may be part of the solution for StackStorm/st2#4679
workflow_state.status == statuses.PAUSED | ||
and wf_ex_event.status in [statuses.RUNNING, statuses.RESUMING] | ||
and not workflow_state.has_active_tasks | ||
and not workflow_state.has_staged_tasks | ||
and not workflow_state.has_paused_tasks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please drop the whitespace only changes. Use black
to reformat.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to the workflow definition language should start as an issue with the proposal otherwise you are going to expect rework. Having said this, I have some issue with the proposal here.
- As I understand the case here,
failfast
is used to control flow for with items. The attribute should be defined under thewith
models at https://docs.stackstorm.com/orquesta/languages/orquesta.html#with-items-model. - So far, the workflow definition language has avoided hard coding
success
andfailure
in the language. I like to keep it that way where possible. Can we do something likeflow: stopOnFail
andflow: continueOnFail
? The flow: stopOnFail should be the default when it is not specified to be consistent with existing behavior.
@exp-vkishore would you be able to revisit this? What do you think of @m4dcoder 's suggestion of a |
|
Currently the incomplete items in a task's with list won't be processed if there are no active task item and parent task has failed status. For example let's consider this workflow
If we run above workflow it will process items 0 and 1 only, as on member 1 action will fail which will set the task1 status as fail and the concurrency is set to 1 there for there will be no active task item and state of task1 will be set to failed from running.
And no other item will be processed.
With this change I have introduce a boolean parameter
failfast
in task which will decide wether to fail the entire task if there are not active task item and task status is failed.