Cascade terminate/Purge Workflow Support #7340

shivamkm07 · 2024-01-02T15:14:21Z

Description

Support for cascade terminate/purge was added in DTF-Go: microsoft/durabletask-go#47.

This PR adds the support for the same in Dapr. Also it makes the cascade feature default i.e. when no option is provided, it would assume cascade terminate/purge true The related components-contrib PR is dapr/components-contrib#3296

Issue reference

Please reference the issue this PR will close: #[issue number]

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Unit tests passing
End-to-end tests passing
Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Specification has been updated / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]
Provided sample for the feature / Created issue in the https://github.com/dapr/docs/ repo: dapr/docs#[issue number]

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

ItalyPaleAle · 2024-01-02T16:42:43Z

LGTM but all builds are failing

pkg/proto/common/v1/common.pb.go

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

codecov · 2024-01-03T07:40:15Z

Codecov Report

Attention: 12 lines in your changes are missing coverage. Please review.

Comparison is base (994b037) 62.32% compared to head (87ef171) 62.35%.

Files	Patch %	Lines
pkg/runtime/wfengine/backends/actors/backend.go	0.00%	5 Missing ⚠️
pkg/api/http/workflow.go	80.00%	2 Missing and 2 partials ⚠️
pkg/runtime/wfengine/component.go	0.00%	2 Missing ⚠️
pkg/api/universal/workflow.go	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7340      +/-   ##
==========================================
+ Coverage   62.32%   62.35%   +0.02%     
==========================================
  Files         240      240              
  Lines       22055    22075      +20     
==========================================
+ Hits        13746    13765      +19     
- Misses       7156     7158       +2     
+ Partials     1153     1152       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

shivamkm07 · 2024-01-16T13:34:23Z

Integration test will fail in this PR for now. Will be resolved once #7385 is merged.

Should pass now

mukundansundar

lgtm.
@cgillum Do we need the API to have cascade terminate by default?

cgillum

My only question about this PR is whether we can make the parameter name be recursive instead of non_recursive. I understand that we like to default Boolean parameter values to false, but I wonder if it would actually be better to name the query string parameter to recursive and default its value to true.

cgillum · 2024-01-16T18:17:03Z

pkg/api/http/http.go

@@ -99,6 +99,7 @@ const (
 	workflowComponent        = "workflowComponent"
 	workflowName             = "workflowName"
 	instanceID               = "instanceID"
+	nonRecursive             = "non_recursive"


Can we make this recursive instead of non_recursive? It's harder for people to understand negatives.

the only concern I have here is, in general query string default parameters when not specified are the default values like false, 0 etc.
By defaulting to recursive terminate/purge and then asking users to set ?recursive=false does not seem intuitive to me, rather at that point setting ?non_recursive=true make it more intuitive IMO.

I kind of incline towards recursive set to true by default. Understanding this was quite easier for me.
Is there any reasoning behind keeping all flags to false by default, apart from general practice?

I personally agree with Mukundan that it's more intuitive when the default value of a boolean is false.

If the default value is "true", then you are introducing 3 states: set true, set false, null.

cgillum · 2024-01-16T18:18:28Z

pkg/runtime/wfengine/backends/actors/backend.go

-func (*ActorBackend) GetOrchestrationRuntimeState(context.Context, *backend.OrchestrationWorkItem) (*backend.OrchestrationRuntimeState, error) {
-	return nil, errors.New("not supported")
+func (abe *ActorBackend) GetOrchestrationRuntimeState(ctx context.Context, owi *backend.OrchestrationWorkItem) (*backend.OrchestrationRuntimeState, error) {
+	state, err := LoadWorkflowState(ctx, abe.actorRuntime, string(owi.InstanceID), abe.config)


Any particular reason for implementing this?

@cgillum this is required for cascade purge: we need to get list of sub-orchestrations to purge: https://github.com/microsoft/durabletask-go/blob/b58abfd02e48667ae6d8622de1531b322178b8b9/backend/backend.go#L135

* Updating protos Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * enable recursive terminate/purge Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Updating dtf-go Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * updating contrib Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Correct proto generated files Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * update comment Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * implementing getOrchestrationRuntimeState Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding unit tests Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * make modtidy-all Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding recursive option in query parameter Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding integration test for workflow Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * fix bug in Creating sub-orchestrations Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Setting recursive/terminate purge to default and adding tests for same Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Removing fix from this PR Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * renaming to Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com>

* Cascade terminate/Purge Workflow Support (#7340) * Updating protos Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * enable recursive terminate/purge Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Updating dtf-go Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * updating contrib Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Correct proto generated files Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * update comment Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * implementing getOrchestrationRuntimeState Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding unit tests Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * make modtidy-all Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding recursive option in query parameter Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding integration test for workflow Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * fix bug in Creating sub-orchestrations Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Setting recursive/terminate purge to default and adding tests for same Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Removing fix from this PR Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * renaming to Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName parameter to Helm chart Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName to README doc Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> Co-authored-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Co-authored-by: Viktor Subota <viktor.subota@c.delinea.com>

* Updating protos Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * enable recursive terminate/purge Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Updating dtf-go Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * updating contrib Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Correct proto generated files Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * update comment Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * implementing getOrchestrationRuntimeState Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding unit tests Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * make modtidy-all Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding recursive option in query parameter Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding integration test for workflow Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * fix bug in Creating sub-orchestrations Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Setting recursive/terminate purge to default and adding tests for same Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Removing fix from this PR Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * renaming to Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com>

* Cascade terminate/Purge Workflow Support (dapr#7340) * Updating protos Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * enable recursive terminate/purge Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Updating dtf-go Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * updating contrib Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Correct proto generated files Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * update comment Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * implementing getOrchestrationRuntimeState Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding unit tests Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * make modtidy-all Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding recursive option in query parameter Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding integration test for workflow Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * fix bug in Creating sub-orchestrations Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Setting recursive/terminate purge to default and adding tests for same Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Removing fix from this PR Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * renaming to Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName parameter to Helm chart Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName to README doc Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> Co-authored-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Co-authored-by: Viktor Subota <viktor.subota@c.delinea.com>

* Cascade terminate/Purge Workflow Support (dapr#7340) * Updating protos Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * enable recursive terminate/purge Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Updating dtf-go Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * updating contrib Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Correct proto generated files Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * update comment Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * implementing getOrchestrationRuntimeState Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding unit tests Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * make modtidy-all Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding recursive option in query parameter Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Adding integration test for workflow Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * fix bug in Creating sub-orchestrations Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Setting recursive/terminate purge to default and adding tests for same Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * Removing fix from this PR Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * renaming to Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> * linter fixes Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName parameter to Helm chart Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> * Add priorityClassName to README doc Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> --------- Signed-off-by: Shivam Kumar <shivamkm07@gmail.com> Signed-off-by: Viktor Subota <viktor.subota@c.delinea.com> Co-authored-by: Shivam Kumar <shivamkm07@gmail.com> Co-authored-by: Mukundan Sundararajan <65565396+mukundansundar@users.noreply.github.com> Co-authored-by: Loong Dai <long.dai@intel.com> Co-authored-by: Dapr Bot <56698301+dapr-bot@users.noreply.github.com> Co-authored-by: Viktor Subota <viktor.subota@c.delinea.com> Signed-off-by: Elena Kolevska <elena@kolevska.com>

shivamkm07 added 3 commits January 2, 2024 19:23

Updating protos

751722f

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

enable recursive terminate/purge

ea2dedc

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

Updating dtf-go

5175161

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

shivamkm07 requested review from a team as code owners January 2, 2024 15:14

shivamkm07 mentioned this pull request Jan 2, 2024

Enable recursive terminate/purge dapr/components-contrib#3296

Merged

3 tasks

updating contrib

24f3e7d

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

ItalyPaleAle reviewed Jan 2, 2024

View reviewed changes

pkg/proto/common/v1/common.pb.go Outdated Show resolved Hide resolved

shivamkm07 and others added 4 commits January 2, 2024 22:20

Correct proto generated files

abe6005

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

update comment

41e16a4

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

Merge branch 'master' into cascade_terminatePurgeWf

bb0be0e

implementing getOrchestrationRuntimeState

2d55758

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

shivamkm07 and others added 5 commits January 3, 2024 13:15

Adding unit tests

89e0084

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

Merge branch 'master' into cascade_terminatePurgeWf

56273a1

Merge branch 'master' into cascade_terminatePurgeWf

04ca172

merge master

1d15ad8

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

Merge branch 'master' into cascade_terminatePurgeWf

1f76406

mukundansundar added the autoupdate DaprBot will keep the Pull Request up to date with master branch label Jan 10, 2024

dapr-bot added 2 commits January 10, 2024 09:43

Merge branch 'master' into cascade_terminatePurgeWf

b5ceb30

Merge branch 'master' into cascade_terminatePurgeWf

65bebca

DeepanshuA approved these changes Jan 11, 2024

View reviewed changes

dapr-bot and others added 4 commits January 10, 2024 23:33

Merge branch 'master' into cascade_terminatePurgeWf

eeaed71

Merge branch 'master' into cascade_terminatePurgeWf

d9ff995

Merge branch 'master' into cascade_terminatePurgeWf

7e4c476

make modtidy-all

fb499a6

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

mukundansundar added documentation required This issue needs documentation docs-needed This issue needs documentation. Automatically create a docs issue. labels Jan 12, 2024

shivamkm07 mentioned this pull request Jan 12, 2024

[Workflow]Docs required for Cascade Terminate/Purge Workflow dapr/docs#3949

Closed

dapr-bot and others added 5 commits January 15, 2024 11:42

Merge branch 'master' into cascade_terminatePurgeWf

49db0dd

Merge branch 'master' into cascade_terminatePurgeWf

1a87247

merge master

b456cb0

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

renaming to

614fc7c

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

linter fixes

ba65bde

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

mukundansundar approved these changes Jan 16, 2024

View reviewed changes

dapr-bot added 3 commits January 16, 2024 07:03

Merge branch 'master' into cascade_terminatePurgeWf

b5f8c4b

Merge branch 'master' into cascade_terminatePurgeWf

86f9b63

Merge branch 'master' into cascade_terminatePurgeWf

3dce3f5

cgillum reviewed Jan 16, 2024

View reviewed changes

dapr-bot added 3 commits January 16, 2024 13:42

Merge branch 'master' into cascade_terminatePurgeWf

1d033a0

Merge branch 'master' into cascade_terminatePurgeWf

5150403

Merge branch 'master' into cascade_terminatePurgeWf

87ef171

mukundansundar merged commit f9a2088 into dapr:master Jan 17, 2024
26 of 27 checks passed

mukundansundar mentioned this pull request Jan 18, 2024

[Workflow] Cascading terminate and purge #6393

Closed

This was referenced Jan 31, 2024

Adding recursive terminate/purge in workflow API dapr/docs#3969

Merged

Workflow delete: Remove non_recursive with hardcoded always enabled recursive #7498

Merged

JoshVanL added this to the v1.13 milestone Feb 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cascade terminate/Purge Workflow Support #7340

Cascade terminate/Purge Workflow Support #7340

shivamkm07 commented Jan 2, 2024 •

edited

ItalyPaleAle commented Jan 2, 2024

codecov bot commented Jan 3, 2024 •

edited

shivamkm07 commented Jan 16, 2024

mukundansundar left a comment

cgillum left a comment

cgillum Jan 16, 2024

mukundansundar Jan 16, 2024

DeepanshuA Jan 17, 2024

ItalyPaleAle Jan 17, 2024

cgillum Jan 16, 2024

shivamkm07 Jan 17, 2024

Cascade terminate/Purge Workflow Support #7340

Cascade terminate/Purge Workflow Support #7340

Conversation

shivamkm07 commented Jan 2, 2024 • edited

Description

Issue reference

Checklist

ItalyPaleAle commented Jan 2, 2024

codecov bot commented Jan 3, 2024 • edited

Codecov Report

shivamkm07 commented Jan 16, 2024

mukundansundar left a comment

Choose a reason for hiding this comment

cgillum left a comment

Choose a reason for hiding this comment

cgillum Jan 16, 2024

Choose a reason for hiding this comment

mukundansundar Jan 16, 2024

Choose a reason for hiding this comment

DeepanshuA Jan 17, 2024

Choose a reason for hiding this comment

ItalyPaleAle Jan 17, 2024

Choose a reason for hiding this comment

cgillum Jan 16, 2024

Choose a reason for hiding this comment

shivamkm07 Jan 17, 2024

Choose a reason for hiding this comment

shivamkm07 commented Jan 2, 2024 •

edited

codecov bot commented Jan 3, 2024 •

edited