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

Fail Fast control for with item task #245

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

exp-vkishore
Copy link

@exp-vkishore exp-vkishore commented Dec 3, 2021

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

version: 1.0

description: A workflow demonstrating with items and concurrent processing.

input:
  - members  # [0,1,2,3]
  - concurrency: 1

tasks:
  task1:
    with:
      items: <% ctx(members) %>
      concurrency: <% ctx(concurrency) %>
    action: core.local cmd="[ item() -eq  1 ] && exit 1 || exit 0"

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.

version: 1.0

description: A workflow demonstrating with items and concurrent processing.

input:
  - members  # [0,1,2,3]
  - concurrency: 1

tasks:
  task1:
    failfast: false
    with:
      items: <% ctx(members) %>
      concurrency: <% ctx(concurrency) %>
    action: core.local cmd="[ item() -eq  1 ] && exit 1 || exit 0"

Copy link
Member

@cognifloyd cognifloyd left a 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

Comment on lines +797 to +801
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
Copy link
Member

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.

Copy link
Collaborator

@m4dcoder m4dcoder left a 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 the with models at https://docs.stackstorm.com/orquesta/languages/orquesta.html#with-items-model.
  • So far, the workflow definition language has avoided hard coding success and failure in the language. I like to keep it that way where possible. Can we do something like flow: stopOnFail and flow: continueOnFail? The flow: stopOnFail should be the default when it is not specified to be consistent with existing behavior.

@cognifloyd
Copy link
Member

@exp-vkishore would you be able to revisit this? What do you think of @m4dcoder 's suggestion of a flow: keyword under with:?

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants