Conversation
spkane31
changed the title
bergundy
left a comment
bergundy
left a comment
There was a problem hiding this comment.
The comments I put on the PauseActivityExecution request applies to all of the new APIs.
| }; | ||
| option (temporal.api.protometa.v1.request_header) = { | ||
| header: "temporal-resource-id" | ||
| value: "workflow:{workflow_id}" |
There was a problem hiding this comment.
This header is conditional based on whether the API is targeting a workflow or an activity.
There was a problem hiding this comment.
See what RespondActivityTaskCompletedById did here.
| post: "/namespaces/{namespace}/activities/{activity_id}/pause" | ||
| body: "*" | ||
| additional_bindings { | ||
| post: "/api/v1/namespaces/{namespace}/activities/{activity_id}/pause" | ||
| body: "*" |
There was a problem hiding this comment.
You're missing the workflow HTTP endpoint. You forgot to copy them from here: https://github.com/temporalio/api/pull/640/changes/cdfef8a8a17582faa966a55d360b6ec2cfd8a251..aa94d3c75ee0759e460fabcf2f7862d74b079055#diff-bded41be6e20a31fc62611d9e8ff0725e1646c662caf8b9ce4e333461b94fbf5.
bergundy
left a comment
bergundy
left a comment
There was a problem hiding this comment.
Approved but please do not merge until you have a working server implementation just to be sure that nothing is missed.
| string namespace = 1; | ||
|
|
||
| // If provided, targets a workflow activity for the given workflow ID. | ||
| // If empty, target a standalone activity. |
There was a problem hiding this comment.
| // If empty, target a standalone activity. | |
| // If empty, targets a standalone activity. |
| // If empty, target a standalone activity. | ||
| string workflow_id = 2; | ||
| // The ID of the activity to target. Must be provided for a standalone activity. | ||
| // Mutually exclusive with workflow_activity_type. |
There was a problem hiding this comment.
Looks like mention of workflow_activity_type in comments is stale -- it doesn't exist in any of them.
| // Mutually exclusive with workflow_activity_type. |
| // Resource ID for routing. Contains "workflow:{workflow_id}" for workflow activities or "activity:{activity_id}" for standalone activities. | ||
| string resource_id = 9; |
There was a problem hiding this comment.
Is resource_id something stale that you've moved away from in favor of the multiple business_id logic in the generated history client routing options?
| string workflow_id = 2; | ||
| // The ID of the activity to target. Must be provided for a standalone activity. | ||
| // Mutually exclusive with workflow_activity_type. | ||
| string activity_id = 3; |
There was a problem hiding this comment.
Is activity_id required now that we don't have workflow_activity_type?
|
|
||
| // Resource ID for routing. Contains "workflow:{workflow_id}" for workflow activities or "activity:{activity_id}" for standalone activities. | ||
| string resource_id = 10; | ||
| } |
There was a problem hiding this comment.
Is it a design decision that there's no "reason" for Reset?
3 participants
What changed?
Add
{Pause/Unpause/Reset/Update}ActivityExecutionRPCs and corresponding request/response pairs that will target both workflow embedded and standalone activities. These all strictly target a single activity by ID, if a workflow_id is specified on the request the operation targets a workflow activity.Why?
Unified support for activity operations.
Breaking changes
None
Server PR
Does not break server