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

Fix remediated task with retry #200

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Fix remediated task with retry #200

merged 1 commit into from
Apr 24, 2020

Conversation

m4dcoder
Copy link
Collaborator

Fix issue when a task that has a transition on failure will also traverse the transition on retry. The issue is caused by the recursion on update_task_state. There should be a return in the recursive call.

Fix issue when a task that has a transition on failure will also traverse the transition on retry. The issue is caused by the recursion on update_task_state. There should be a return in the recursive call.
@codecov-io
Copy link

codecov-io commented Apr 23, 2020

Codecov Report

Merging #200 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #200   +/-   ##
=======================================
  Coverage   94.04%   94.04%           
=======================================
  Files          41       41           
  Lines        2735     2735           
  Branches      545      545           
=======================================
  Hits         2572     2572           
  Misses        100      100           
  Partials       63       63           
Impacted Files Coverage Δ
orquesta/conducting.py 93.24% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afdb704...36a3475. Read the comment docs.

Copy link
Contributor

@amanda11 amanda11 left a comment

Choose a reason for hiding this comment

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

looks good. Passed the scenario that caused me to report issue. Re-tested with and without retries and all went fine.

@arm4b arm4b added this to To Do in StackStorm v3.2.0 Apr 24, 2020
@arm4b arm4b moved this from To Do to In Progress in StackStorm v3.2.0 Apr 24, 2020
@@ -1,6 +1,14 @@
Changelog
=========

In Development
Copy link
Member

@arm4b arm4b Apr 24, 2020

Choose a reason for hiding this comment

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

We'll take it to st2 v3.2.0

@m4dcoder I guess we'll release v1.1.1 orquesta once its merged to include in st2 dependencies?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. I'll do a release of v1.1.1.

@arm4b arm4b added the bug label Apr 24, 2020
Copy link
Contributor

@blag blag left a comment

Choose a reason for hiding this comment

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

👍

@soumyabk
Copy link

we're using the stackstorm-ha to deploy our cluster (https://github.com/StackStorm/stackstorm-ha) - once this is merged, how do we pick up the change?

@blag
Copy link
Contributor

blag commented Apr 24, 2020

@soumyabk
We are working on releasing ST2 version 3.2 next week. That version will automatically include an up-to-date version of Orquesta that includes this change. You can pick up this change by installing that version once it is officially released.

@arm4b
Copy link
Member

arm4b commented Apr 24, 2020

It's a bit different in stackstorm-ha Helm chart context.

@soumyabk Because stackstorm-ha is in beta state it already relies on most recent 3.2dev version meaning it gets latest updates from the master.
See https://github.com/StackStorm/stackstorm-ha/blob/master/Chart.yaml#L3

We'll need to get this patch merged and then include into st2. Automation will get all the packages ready, generate & deploy Docker images and so on.
I think you should see the fix in 1-2 days rolled out in your environment. Please follow-up if not.

@m4dcoder m4dcoder merged commit 1e2d962 into master Apr 24, 2020
@m4dcoder m4dcoder deleted the fix-task-retry branch April 24, 2020 22:47
@arm4b arm4b moved this from In Progress to Completed in StackStorm v3.2.0 Apr 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
StackStorm v3.2.0
  
Completed
Development

Successfully merging this pull request may close these issues.

None yet

6 participants