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

Add root workflow execution to WorkflowExecutionStartedEventAttributes #366

Merged
merged 1 commit into from
Apr 9, 2024

Conversation

rodrigozhou
Copy link
Contributor

@rodrigozhou rodrigozhou commented Mar 11, 2024

What changed?
Add fields for root workflow execution.

The root workflow execution is defined as follows:

  1. A workflow without parent workflow is its own root workflow.
  2. A workflow that has a parent workflow has the same root workflow as its parent workflow.

Examples:

  • Scenario 1: Workflow W1 starts child workflow W2, and W2 starts child workflow W3.
    • The root workflow of all three workflows is W1.
  • Scenario 2: Workflow W1 starts child workflow W2, and W2 continued as new W3.
    • The root workflow of all three workflows is W1.
  • Scenario 3: Workflow W1 continued as W2.
    • The root workflow of W1 is W1 and the root workflow of W2 is W2.

Why?

Breaking changes

@rodrigozhou rodrigozhou requested review from a team as code owners March 11, 2024 19:58
@dandavison
Copy link
Contributor

Could you add some documentation to the code explaining what the fields are and how they should be used? (Also the PR itself doesn't have any explanation, but that could come from the code comments in a proto API PR).

@rodrigozhou rodrigozhou force-pushed the rodrigozhou/root-wf-exec branch 2 times, most recently from cfb08ad to 921a7ef Compare March 18, 2024 22:42
Copy link
Member

@cretz cretz left a comment

Choose a reason for hiding this comment

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

Approving from API POV, but would like to see reset behavior on this field documented (i.e. that it doesn't change across reset if that's the behavior)

Copy link
Contributor

@dandavison dandavison left a comment

Choose a reason for hiding this comment

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

Thanks for explaining! I added an optional suggestion to make it explicit that if A--CAN-->B then parent(B) = parent(A).

@rodrigozhou rodrigozhou force-pushed the rodrigozhou/root-wf-exec branch 2 times, most recently from 6905dcc to 1032e59 Compare March 21, 2024 17:51
@rodrigozhou
Copy link
Contributor Author

The implementation in the Server is here: temporalio/temporal#5520

@rodrigozhou rodrigozhou merged commit 268fe9c into master Apr 9, 2024
3 checks passed
@rodrigozhou rodrigozhou deleted the rodrigozhou/root-wf-exec branch April 9, 2024 20:11
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.

None yet

6 participants