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.

Sign up for GitHub

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

Clarify run ID may not be current in ResetWorkflowExecutionRequest #473

Merged
merged 4 commits into from
Nov 12, 2024
Merged

Clarify run ID may not be current in ResetWorkflowExecutionRequest #473

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e62b931
Clarify run ID may not be current in ResetWorkflowExecutionRequest
bergundy Nov 5, 2024
7bed81e
Update temporal/api/workflowservice/v1/request_response.proto
bergundy Nov 8, 2024
fed0036
Gen openapi
bergundy Nov 8, 2024
d1a1979
Incorporate Dan's suggestion
bergundy Nov 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion openapi/openapiv2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5504,7 +5504,8 @@
"type": "string"
}
},
"description": "Identifies a specific workflow within a namespace. Practically speaking, because run_id is a\nuuid, a workflow execution is globally unique. Note that many commands allow specifying an empty\nrun id as a way of saying \"target the latest run of the workflow\"."
"description": "The workflow ID and optional run ID to reset to.\nThe current run will be terminated even if it has a different run ID than the one provided in this\nrequest.",
"title": "The workflow ID and optional run ID to reset to.\nThe current run will be terminated even if it has a different run ID than the one provided in this\nrequest."
},
"reason": {
"type": "string"
Expand Down
7 changes: 6 additions & 1 deletion openapi/openapiv3.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7498,7 +7498,12 @@ components:
namespace:
type: string
workflowExecution:
$ref: '#/components/schemas/WorkflowExecution'
allOf:
- $ref: '#/components/schemas/WorkflowExecution'
description: |-
The workflow ID and optional run ID to reset to.
The current run will be terminated even if it has a different run ID than the one provided in this
request.
reason:
type: string
workflowTaskFinishEventId:
Expand Down
3 changes: 3 additions & 0 deletions temporal/api/workflowservice/v1/request_response.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,9 @@ message SignalWithStartWorkflowExecutionResponse {

message ResetWorkflowExecutionRequest {
string namespace = 1;
// The workflow ID and optional run ID to reset to.
dandavison marked this conversation as resolved.
Show resolved Hide resolved
// The current run will be terminated even if it has a different run ID than the one provided in this
// request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably multiple runs might be terminated, if the request identifies a run far back in the chain?

Maybe some wording like the following?

The run identified by the request, and any subsequent runs, will be terminated by the reset, and a new run will be created that becomes the current run. If the request does not identify a run, then it is interpreted as identifying the current run.

Copy link
Member Author

Choose a reason for hiding this comment

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

Presumably multiple runs might be terminated, if the request identifies a run far back in the chain?

No, there can only be one current run. Only the current one will be terminated.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me know if that makes sense or you still want clarification.

Copy link
Contributor

Choose a reason for hiding this comment

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

Haha yes, what I said made zero sense.

It's not a big problem but I'm still finding it slightly unclear. Here's another attempt:

// The workflow to reset. If this contains a run ID then the workflow will be reset back to the
// provided event ID in that run. Otherwise it will be reset to the provided event ID in the
// current run. In all cases the current run will be terminated and a new run started.

temporal.api.common.v1.WorkflowExecution workflow_execution = 2;
string reason = 3;
// The id of a `WORKFLOW_TASK_COMPLETED`,`WORKFLOW_TASK_TIMED_OUT`, `WORKFLOW_TASK_FAILED`, or
Expand Down
Loading