Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Fail Fast control for with item task #245

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.

Sign up for GitHub

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

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

Fail Fast control for with item task #245

wants to merge 4 commits into from

Conversation

exp-vkishore
Copy link

@exp-vkishore exp-vkishore commented Dec 3, 2021 •
edited by cognifloyd
Loading

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"

cognifloyd
cognifloyd reviewed Dec 4, 2021
View reviewed changes
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

orquesta/machines.py
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.

m4dcoder
m4dcoder requested changes Dec 25, 2021
View reviewed changes
Copy link
Collaborator

@m4dcoder m4dcoder left a comment •
edited
Loading

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

Reviewers

@cognifloyd cognifloyd cognifloyd left review comments

@m4dcoder m4dcoder m4dcoder requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@exp-vkishore @cognifloyd @CLAassistant @m4dcoder
Morty Proxy This is a proxified and sanitized view of the page, visit original site.