Skip to content

Cascade terminate/purge support in GrpcDurableTaskClient - #262

Merged
Jacob Viau (jviau) merged 12 commits into
microsoft:mainfrom
shivamkm07:cascade_terminatePurge
Feb 2, 2024
Merged

Cascade terminate/purge support in GrpcDurableTaskClient#262
Jacob Viau (jviau) merged 12 commits into
microsoft:mainfrom
shivamkm07:cascade_terminatePurge

Conversation

@shivamkm07

@shivamkm07Shivam Kumar (shivamkm07) commented Jan 24, 2024

Copy link
Copy Markdown
Contributor

This PR adds support to recursively terminate/purge sub-orchestrations in GrpcDurableTaskClient. It also sets the recursive behavior to be false by default.

Closes: #260

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

@jviauJacob Viau (jviau) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As we now have multiple parameters for terminate, I recommend we do the following:

  1. Add TerminateInstanceOptions
  2. Make the current abstract method virtual. Default implementation will call one of the new methods we add.
  3. Add TerminateInstanceOptions accepting virtual methods
  4. Update our clients to override the new options-receiving method.
/// add xmldocpublicrecordTerminateInstanceOptions{publicobject?Output{get;init;}publicboolRecursive{get;init;}}

new methods:

publicvirtualTaskTerminateInstanceAsync(stringinstanceId,TerminateInstanceOptions?options,CancellationTokencancellation)=>thrownewNotSupportedException($"{this.GetType()} does not support orchestration termination.");// this is the formerly abstract methodpublicvirtualTaskTerminateInstanceAsync(stringinstanceId,object?output=null,CancellationTokencancellation=default){TerminateInstanceOptions?options=outputisnull?null:new(){Output=output};returnthis.TerminateInstanceAsync(instanceId,options,cancellation);}

Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
@shivamkm07

Shivam Kumar (shivamkm07) commented Jan 25, 2024

Copy link
Copy Markdown
ContributorAuthor

As we now have multiple parameters for terminate, I recommend we do the following:

  1. Add TerminateInstanceOptions
  2. Make the current abstract method virtual. Default implementation will call one of the new methods we add.
  3. Add TerminateInstanceOptions accepting virtual methods
  4. Update our clients to override the new options-receiving method.
/// add xmldocpublicrecordTerminateInstanceOptions{publicobject?Output{get;init;}publicboolRecursive{get;init;}}

new methods:

publicvirtualTaskTerminateInstanceAsync(stringinstanceId,TerminateInstanceOptions?options,CancellationTokencancellation)=>thrownewNotSupportedException($"{this.GetType()} does not support orchestration termination.");// this is the formerly abstract methodpublicvirtualTaskTerminateInstanceAsync(stringinstanceId,object?output=null,CancellationTokencancellation=default){TerminateInstanceOptions?options=outputisnull?null:new(){Output=output};returnthis.TerminateInstanceAsync(instanceId,options,cancellation);}

I am okay with this change. Just should the new TerminateInstanceOptions method be an abstract method and both GrpcDurableTaskClient and ShimDurableTaskClient implement this method? This is what seems to be followed in other DurableTaskClient methods..

Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
@shivamkm07

Copy link
Copy Markdown
ContributorAuthor

As we now have multiple parameters for terminate, I recommend we do the following:

  1. Add TerminateInstanceOptions
  2. Make the current abstract method virtual. Default implementation will call one of the new methods we add.
  3. Add TerminateInstanceOptions accepting virtual methods
  4. Update our clients to override the new options-receiving method.
/// add xmldocpublicrecordTerminateInstanceOptions{publicobject?Output{get;init;}publicboolRecursive{get;init;}}

new methods:

publicvirtualTaskTerminateInstanceAsync(stringinstanceId,TerminateInstanceOptions?options,CancellationTokencancellation)=>thrownewNotSupportedException($"{this.GetType()} does not support orchestration termination.");// this is the formerly abstract methodpublicvirtualTaskTerminateInstanceAsync(stringinstanceId,object?output=null,CancellationTokencancellation=default){TerminateInstanceOptions?options=outputisnull?null:new(){Output=output};returnthis.TerminateInstanceAsync(instanceId,options,cancellation);}

Jacob Viau (@jviau) Pushed the suggested changes. Please review.

Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Core/TerminateInstanceOptions.cs Outdated
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Core/DurableTaskClient.cs Outdated
Comment threadsrc/Client/Grpc/GrpcDurableTaskClient.cs Outdated
Comment threadsrc/Client/OrchestrationServiceClientShim/ShimDurableTaskClient.cs Outdated
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>

@jviauJacob Viau (jviau) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only a few small comments of recursive default behavior.

Comment threadsrc/Client/Core/PurgeInstanceOptions.cs Outdated
Comment threadsrc/Client/Core/TerminateInstanceOptions.cs Outdated
Comment threadsrc/Shared/Grpc/ProtoUtils.cs
Comment threadsrc/Client/Grpc/GrpcDurableTaskClient.cs Outdated
Signed-off-by: Shivam Kumar <shivamkm07@gmail.com>
Sign up for freeto 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.

Support Cascade Terminate/Purge

3 participants

@shivamkm07@cgillum@jviau