Temporal Nexus Python proposal #97
Conversation
dandavison
force-pushed
the
nexus-python
branch
3 times, most recently
from
8bb5eff to
fbf98df
Compare
dandavison
force-pushed
the
nexus-python
branch
from
fbf98df to
71ad9da
Compare
dandavison
force-pushed
the
nexus-python
branch
from
71ad9da to
568a4bb
Compare
| @workflow.defn | ||
| class EchoCallerWorkflow: | ||
| def __init__(self): | ||
| self.nexus_client = NexusClient( |
There was a problem hiding this comment.
| self.nexus_client = NexusClient( | |
| self.nexus_client = workflow.NexusClient( |
| self.nexus_client = NexusClient( | ||
| MyNexusService, # or string name "my-nexus-service", | ||
| "my-nexus-endpoint-name", | ||
| schedule_to_close_timeout=timedelta(seconds=30), |
There was a problem hiding this comment.
I think this needs to be per-operation kwarg. Java uniquely does some of these at the service/stub level because of how it proxies, but all other SDKs can do operation level.
| ```python | ||
| import nexusrpc.handler | ||
|
|
||
| @nexusrpc.handler.service(interface=interface.MyNexusService, name="my-service") # import-time check that interface was implemented |
There was a problem hiding this comment.
Can we use a different name for the decorator besides service? I know being in handler resolves the ambiguity, but I still think we should avoid ambiguous names across the two modules of the nexusrpc project. Also, similar to operation below, when we talk about "service" we are talking about the contract, not ambiguously talking about the contract or implementation. Maybe service_impl or service_handler or something.
| ```python | ||
| import nexusrpc.handler | ||
|
|
||
| @nexusrpc.handler.service(interface=interface.MyNexusService, name="my-service") # import-time check that interface was implemented |
There was a problem hiding this comment.
Not sure interface is a good name here. Maybe it doesn't need to be a kwarg at all?
| ```python | ||
| import nexusrpc.handler | ||
|
|
||
| @nexusrpc.handler.service(interface=interface.MyNexusService, name="my-service") # import-time check that interface was implemented |
There was a problem hiding this comment.
The name parameter here seems like something that should be on the service side not the impl side. It is needed by callers too so it is part of the contract.
| - `StartOperationOptions` | ||
|
|
||
| ```python | ||
| @dataclass |
There was a problem hiding this comment.
Pedantic, but recommend frozen here
| class MyNexusService: # instantiated when instantiating worker | ||
| # User is free to define custom constructor | ||
|
|
||
| @nexusrpc.handler.operation(name="my-invalid-python-identifier") |
There was a problem hiding this comment.
Like services, names of operations are part of the contract and therefore need to be on the contract side not the impl side
| - In the shorthand form, the decorator takes in the `start` handler method and returns a factory method `(Service) -> Operation` (that factory method takes care of creating an operation with the specified start method). Thus at run-time, the method on their service has a signature that differs from what they wrote under the decorator. Decorators in Python are used to effect fairly radical transformations: for example `@property`, or `@contextlib.contextmanager` in the standard library. But the main point in defense of the radicalness of this decorator transformation is that the user is not expected to call this method: it is called by the framework. | ||
| - The decorator propagates type annotations on the start method defined on the service to become run-time type annotation objects on the methods defined on the operation [[nexus-sdk PR](https://github.com/nexus-rpc/sdk-python/blob/e318efeb5b615a7dab916af1c5f01fe97ffff841/src/nexusrpc/handler.py#L294), [sdk-python PR](https://github.com/temporalio/sdk-python/blob/08430098aac4d92c3744b1245c5585819abadad3/temporalio/nexus/handler.py#L155-L169)]. This allows, e.g. a Nexus worker to instantiate the correct Python type from payloads received over the wire. | ||
|
|
||
| **[ALTERNATIVES CONSIDERED]** |
There was a problem hiding this comment.
Not sure this is an alternative. Isn't what is below effectively what @nexusrpc.handler.sync_operation is sugar for?
| @temporalio.nexus.handler.workflow_operation | ||
| async def hello( | ||
| self, input: HelloInput, options: nexusrpc.handler.StartOperationOptions | ||
| ) -> nexusrpc.handler.StartOperationResult[HelloOutput]: |
There was a problem hiding this comment.
Could have a more specific, clearer subtype to return here, e.g. temporalio.nexus.handler.WorkflowOperationHandler[HelloOutput]
| ```python | ||
| @nexusrpc.handler.service(interface=interface.MyNexusService) | ||
| class MyNexusService: | ||
| @temporalio.nexus.handler.workflow_operation |
There was a problem hiding this comment.
Safe to assume this is just shorthand for something like:
def hello(self) -> WorkflowOperationHandler[HelloInput, HelloOutput]:
return temporalio.nexus.handler.WorkflowOperationHandler(HelloWorkflow.run)And I could do that myself if I didn't want the decorator (e.g. maybe doing something a bit more dynamic)?
This is an early-stage design document for how Nexus functionality will be exposed in the Nexus Python SDK and in the Temporal Python SDK. The design is still evolving, and feedback is welcome.
The design proposed here is being prototyped in this draft PR.
Rendered markdown