Add Temporal Operation Handler#1503
Conversation
VegetarianOrc
force-pushed
the
amazzeo/temporal-nexus-operation-handler
branch
from
26c52b8 to
b0dd232
Compare
… the same type signature on a service that the client is not bound to
…at verfiy the start failure case and concurrent starts
| _input: None, | ||
| ) -> nexus.TemporalOperationResult[None]: | ||
| return await client.start_workflow( | ||
| BlockingWorkflow.run, id=f"blocking-{uuid.uuid4}" |
There was a problem hiding this comment.
Should there be parenthesis after the uuid4? Same for two cases just below, lines 78 and 81.
|
|
||
| match operation_token.type: | ||
| case OperationTokenType.WORKFLOW: | ||
| await self.cancel_workflow_run(ctx, operation_token.workflow_id) |
There was a problem hiding this comment.
If this doesn't match, should we raise an error or something to let the user know? I'm not sure of the desired behavior if that's the case.
| self._started_async = True | ||
| try: | ||
| yield | ||
| except: |
| namespace = token_details.get("ns") | ||
| if not isinstance(namespace, str) or not namespace: | ||
| raise TypeError( | ||
| f"invalid token: expected namespace to be a string or null, got {type(namespace)}" |
There was a problem hiding this comment.
The error message says null is OK, but null isn't a string so will fail the if statement, won't it?
| class OperationTokenType(IntEnum): | ||
| """Type discriminator for Nexus operation tokens.""" | ||
|
|
||
| WORKFLOW = 1 |
There was a problem hiding this comment.
Should there be an UNSPECIFIED of 0? The other int enums have it - it looks like default from protobuf is zero so if it isn't specified for some reason it will be set to zero
|
|
||
| def __init__(self) -> None: | ||
| """Initialize the client wrapper from the active Nexus operation context.""" | ||
| self._temporal_context = _TemporalStartOperationContext.get() |
There was a problem hiding this comment.
Should you make sure the get returned a valid context?
| def _reserve_async_start(self) -> Iterator[None]: | ||
| if self._started_async: | ||
| raise HandlerError( | ||
| "Only one async operation can be started per operation handler invocation. Use TemporalNexusClient.client for additional workflow interactions", |
There was a problem hiding this comment.
If you can only call one async per operation, should this check be in the operation? Otherwise I can just get the client and call multiple async operations, can't I?
2 participants
What was changed
Added support for Temporal-backed Nexus operation handlers.
This introduces
@temporalio.nexus.temporal_operation,TemporalNexusClient, andTemporalOperationResult, allowing Nexus operation handlers to either return a synchronous result or start a Temporal workflow as the async backing operation. The branch also adds token parsing support for generic operation tokens, cancellation support for Temporal-backed workflow operations, Nexus workflow client overloads for the new operation shape, and focused runtime/type coverage.Why?
This gives Nexus operation handlers a first-class way to interact with Temporal while preserving Nexus async operation semantics, workflow linking/callback behavior, cancellation handling, and typed workflow-start ergonomics.
Checklist
How was this tested:
Any docs updates needed?
Yes. The Nexus docs should cover
@temporalio.nexus.temporal_operation,TemporalNexusClient.start_workflow,TemporalOperationResult.sync, and async workflow-backed operation behavior.