Repository navigation
Conversation
|
|
||
| // An operation was started asynchronously. The token can be used for cancellation. | ||
| message AsyncSuccess { | ||
| string operation_token = 1; |
There was a problem hiding this comment.
I was under the impression that operation token was only for handler-server and caller-server side, not caller client side which operates on operation IDs
There was a problem hiding this comment.
This is for exposing direct calls to a handler, bypassing the durable caller machinery. The semantics are different.
There was a problem hiding this comment.
I don't understand. Where are the RPC calls for standalone Nexus operations from a client that are durable and take operation IDs and such? The name of these RPCs makes it seem like those. Or are standalone Nexus operations as being designed internally not occurring as designed?
There was a problem hiding this comment.
So both PRs have StartNexusOperation and mean different things, though both PR descriptions acknowledge the ambiguity.
I would recommend not using the term Execution to differentiate. I do think the outbound caller form should use Execution, but I think the "direct" form here should have a more qualifying name. I assume this is mostly for server-to-server because most people will want durable and therefore even if they are calling a handler on their own cluster, they'll use the durable/visibility-based form from the other PR, correct?
You're going to have to have an adjective to refer to this form of API regardless, so I recommend baking that adjective into the RPC names as you will with docs or anything else. Direct may be ok, but other options may make sense too, but we need some term to refer to these in both the RPC name and in conversation/docs IMO.
There was a problem hiding this comment.
The use cases are:
- Exposing system endpoint Nexus APIs to SDK clients (allows us to go all in on Nexus for public APIs, each API would automatically work within and outside of a workflow)
- Building interfaces with workers as a backend (e.g. for the UI and CLI)
I intentionally used the API names from the Nexus handler specification since there's a 1-1 mapping with those APIs.
There was a problem hiding this comment.
I think we need a term for this approach and the RPCs should include that term to avoid ambiguity with the other ones. Execution isn't enough IMO to differentiate (and won't be in CLI, docs, or anywhere else).
There was a problem hiding this comment.
We are not planning on exposing these methods to the CLI or docs BTW. This is an internal detail that would be used inside the SDK. We have been using the terms Durable and Direct or Execution and Request.
I'm not sure what to call this. Maybe:
StartDirectNexusOperation?CancelDirectNexusOperation?
Got better ideas?
There was a problem hiding this comment.
Alternatives:
SendStartNexusOperationRequestSendCancelNexusOperationRequest
There was a problem hiding this comment.
I like StartNexusOperation vs StartNexusOperationExecution: clear and elegant IMO.
Add StartNexusOperation and CancelNexusOperation to the WorkflowService for invoking Nexus operations directly, without a durable caller-side operation. - Responses mirror temporal.api.nexus.v1.StartOperationResponse: sync result, async start, or a Temporal failure. Links use temporal.api.common.v1.Link. - Add Callback.Ephemeral. An ephemeral callback is never persisted and turns the request that carries it into a long poll. The outcome is returned synchronously if the operation completes before the long-poll deadline; otherwise the request returns as an async start and the caller re-issues it with the same request ID. - Expose PollActivityExecution as a system Nexus operation. The request namespace must be empty when invoked via Nexus. - Exempt the direct-call RPCs from the temporal-resource-id annotation requirement.
ac2f894 to
a44fe3c
Compare
| // Also exposed as an asynchronous operation on the system Nexus endpoint. When invoked via Nexus, the request's | ||
| // namespace must be empty; the caller's namespace is used. | ||
| rpc PollActivityExecution (PollActivityExecutionRequest) returns (PollActivityExecutionResponse) { | ||
| option (nexusannotations.v1.operation).tags = "exposed"; |
There was a problem hiding this comment.
This must be reverted before committing. I'm not proposing to add this API as a Nexus API now, I just needed an "async" API for the prototype.
…Failure - The StartNexusOperation and CancelNexusOperation HTTP routes no longer include the namespace, which is only applicable to the system Nexus endpoint and is passed in the body. - Handler errors are returned as gRPC errors with a NexusHandlerFailure detail carrying the handler error, whose retry behavior clients should honor.
…yService.GetResult ActivityService is a new Nexus-only service on the system Nexus endpoint. Its GetResult operation outputs the activity's result payload as is, and fails or is canceled with the activity's failure. That's the same completion a durable callback attached to the activity delivers, so workflows can call the operation too, not just direct callers. PollActivityExecution is no longer exposed via Nexus. The Nexus RPC YAML generator now derives type refs from the file that defines each message, so operations can use messages from other packages, e.g. GetResult outputs a common.v1.Payload.
bergundy
left a comment
There was a problem hiding this comment.
activity service was added for the demo, not to be merged.
What changed?
Added two RPCs for making direct Nexus requests using namespace credentials:
StartNexusOperationandCancelNexusOperation.Unlike standalone Nexus operations (
StartNexusOperationExecution, etc.), direct calls do not create a durable caller-side operation. They go directly to the handler registered for the given endpoint. Initially, only the system Nexus endpoint (__temporal_system) will be supported.temporal.api.nexus.v1.StartOperationResponse. It is either a sync result, an async start, or a TemporalFailurewithApplicationFailureInfoorCanceledFailureInfo. Links usetemporal.api.common.v1.Link.NexusHandlerFailureerror detail carries the handler error. Clients should honor the handler error's retry behavior rather than retrying by status code.namespaceonly applies to the system Nexus endpoint, where it is the namespace the operation runs in. It is passed in the request body, not in the HTTP route.Callback.Ephemeral, which is set viaStartNexusOperationRequest.completion_callback.temporal.api.activityservice.v1.ActivityService, a Nexus service exposed on the system Nexus endpoint. It is not served over gRPC.GetResultoperation waits for a standalone activity to complete. Its output is the activity's result payload, as encoded by the activity. If the activity fails, times out, or is terminated, the operation fails with the activity's failure as the cause. If the activity is canceled, the operation is canceled.nexus/activity-service.wit, and added the service to the generated nexus-rpc YAML.ActivityService.GetResultfrom thetemporal-resource-idlint. The direct calls' routing target is derived from the operation input (start) or the operation token (cancel).The HTTP routes (
/nexus-requests/endpoints/{endpoint}/services/{service}/operations/{operation}[/cancel]) qualify these APIs as "requests" (nexus-requests) to distinguish them from standalone Nexus operation routes (nexus-operations).Why?
This lets callers outside of a workflow invoke system Nexus operations with minimal overhead, including long-running ones. An async system operation called from a workflow completes via a durable callback. Called from outside a workflow, the same operation is awaited with an ephemeral callback.
A separate service per archetype keeps each operation's implementation in one place on the server, instead of several libraries implementing operations of
WorkflowService.Notes
GetResultreturnstemporal.api.common.v1.Payloaddirectly, sobuf.yamlignoresRPC_RESPONSE_STANDARD_NAMEforactivityservice/v1/service.proto.api-linter.yamldisables the HTTP annotation and Get method rules for the service.payloadoutput as a system-encodedPayloadproto. ForGetResult, they need to decode the output with the user's data converter instead.make system-nexus-witfails on the existingnotification-service.wit(0.2.8 lacksadd-message --file, and 0.2.12 rejects@nexus.docon an interface). The ActivityService step runs after it, so I ran that step separately. Its output doesn't change when regenerated.Server PR
Not yet.