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

[Reset] Define child handling policies #498

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gow
Copy link
Contributor

@gow gow commented Nov 26, 2024

Added reset policies to handle child workflows.

Why?
Need these to allow resetting of workflows with pending children.

Breaking changes
No

Server PR
N/A

@gow gow requested review from a team as code owners November 26, 2024 23:48
@gow gow requested a review from yycptt November 26, 2024 23:48
enum ResetChildPolicy {
RESET_CHILD_POLICY_UNSPECIFIED = 0;
// Reconnects to the currently running child.
RESET_CHILD_POLICY_RECONNECT = 1;
Copy link
Member

Choose a reason for hiding this comment

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

I thought we initially only support terminateAndRerun for children initiated after the reset point?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. We don't need it immediately. But I need a way to know if the current run is a result of a reset. So wanted to add this.
But on second thought I think we should add an explicit flag in the new run to mark it as a reset-run. Doing that in this PR - temporalio/temporal#6916

@@ -77,6 +77,13 @@ enum ParentClosePolicy {
PARENT_CLOSE_POLICY_REQUEST_CANCEL = 3;
}

// ResetChildPolicy specifies the policy to apply to child workflows when a parent is reset.
enum ResetChildPolicy {
Copy link
Member

Choose a reason for hiding this comment

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

Where is this enum referenced/used in the API?

Copy link
Contributor Author

@gow gow Dec 2, 2024

Choose a reason for hiding this comment

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

I don't have the corresponding change yet. The intent is to use it in ResetWorkflowExecutionRequest .

message ResetWorkflowExecutionRequest {

For now I'm holding off on this. Will update this PR when I get to working on the API.

@gow gow marked this pull request as draft December 2, 2024 19:20
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.

3 participants