[DO NOT MERGE] Standalone Nexus Operations #685
Conversation
| // A unique identifier for this start request. Typically UUIDv4. | ||
| string request_id = 3; | ||
|
|
||
| string operation_id = 4; |
There was a problem hiding this comment.
Probably deserves a comment that this is only a caller-side ID, distinct from the operation ID that the handler will generate
There was a problem hiding this comment.
The handler doesn't generate an operation ID but I agree it's worth calling out.
|
|
||
| // The number of attempts made to start/deliver the operation request. | ||
| // This number represents a minimum bound since the attempt is incremented after the request completes. | ||
| int32 attempt = 9; |
There was a problem hiding this comment.
This is attempt to deliver the start request. Will we support overall operation retry in the future? Will this name be confusing if we do? Maybe we should call it start_attempt so that people will not confuse it with activity attempt which has a different meaning.
There was a problem hiding this comment.
I want to keep this for consistency with PendingNexusOperationInfo.
| string request_id = 19; | ||
|
|
||
| // Operation token. Only set for asynchronous operations after a successful StartOperation call. | ||
| string operation_token = 20; |
There was a problem hiding this comment.
I thought we said we didn't want to expose this to callers? They should only have one way of referencing their operations: their caller-side operation ID.
There was a problem hiding this comment.
It's still worth exposing this information as we do for workflow callers.
| // The identity of the client who initiated this request. | ||
| string identity = 2; | ||
| // A unique identifier for this start request. Typically UUIDv4. | ||
| string request_id = 3; |
There was a problem hiding this comment.
Deserves a comment that this is the request ID for this caller-side request. StartOperation requests sent to the handler will use a different request ID, generated by the caller server.
| // The run ID of the operation, useful when run_id was not specified in the request. | ||
| string run_id = 1; | ||
|
|
||
| // Stage to wait for. The operation may be in a more advanced stage when the poll is unblocked. |
There was a problem hiding this comment.
I'm confused, is this the stage the original request sent? Or does it represent the current stage of the operation?
There was a problem hiding this comment.
The current stage. Let me fix the docstring.
| body: "*" | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
Won't we need DeleteNexusOperation as well? Otherwise we won't be able to clean up operations after namespace deletion.
| // Updated on terminal status. | ||
| int64 state_transition_count = 10; | ||
| // Updated once on scheduled and once on terminal status. | ||
| int64 state_size_bytes = 11; |
There was a problem hiding this comment.
Is this intentionally a field only present in list? It was mentioned for standalone activities that everything in list was expected to be in describe.
Also, for standalone activities it was mentioned there would be a tool that would make sure everything in list was also in describe result. Can we prioritize that? It's a lot of effort for me to have to continually confirm our assertion on every PR and find these issues since we chose not to reuse types.
There was a problem hiding this comment.
Just want to call this out that we don't have this guarantee for schedules or batch which are much older archetypes: https://github.com/temporalio/api/blob/master/temporal/api/schedule/v1/message.proto https://github.com/temporalio/api/blob/master/temporal/api/workflowservice/v1/request_response.proto#L1715-L1751.
I don't think this guarantee needs to be high priority but we should keep track of it because I do think that it is nice to have. Ideally the SDKs would allow the types to have completely different fields, there's no need to reuse the models here.
There was a problem hiding this comment.
Right, but this guarantee/promise was made as part of not reusing models knowing the SDK will need this guarantee. Was not expecting a "nice to have" guarantee when the promise/guarantee was made.
There was a problem hiding this comment.
Let's take this offline.
There was a problem hiding this comment.
Discussed offline, we will write a tool soon.
3 participants
What changed?
Open questions
PollNexusOperation(Execution?)for consistency withPollWorkflowExecutionUpdate, which is inconsistent with the standalone activity version of this API (we will want to make all three consistent IMHO).Server PR