Conversation
|
Make sure to link this in the README https://github.com/temporalio/samples-java/blob/main/README.md |
| 5. **Client options and interceptors** — set the identity and data converter, and register two | ||
| logging interceptors. | ||
|
|
||
| > [!WARNING] |
There was a problem hiding this comment.
NIT: I would put this at the top
|
|
||
| ### Running | ||
|
|
||
| Start a Temporal server with the standalone-Nexus dynamic configs enabled: |
There was a problem hiding this comment.
We should call out the version of the dev server users need
| package io.temporal.samples.nexusstandalone.service; | ||
|
|
||
| // A helper method to generate workflow IDs. | ||
| public final class GreetingIds { |
There was a problem hiding this comment.
We don't need this helper, just inline this
| * environment variables) to point at a different server or namespace — for example a Temporal Cloud | ||
| * namespace with an API key. | ||
| */ | ||
| public class ClientOptions { |
There was a problem hiding this comment.
I'd just inline this like all the other samples
| public static WorkflowClient getWorkflowClient() { | ||
| ClientConfigProfile profile; | ||
| try { | ||
| String configFilePath = |
There was a problem hiding this comment.
Why do we have special client config loading code? Other samples just do
ClientConfigProfile profile;
try {
profile = ClientConfigProfile.load();
} catch (IOException e) {
throw new RuntimeException("Failed to load client configuration", e);
}
| NexusServiceClient<GreetingNexusService> greetingClient = | ||
| nexusClient.newNexusServiceClient(GreetingNexusService.class, ENDPOINT_NAME); | ||
|
|
||
| demonstrateExecute(greetingClient); |
There was a problem hiding this comment.
Please inline all these functions, they only have one caller and make the code harder to follow
|
|
||
| // executeAsync(...) is the same but returns a CompletableFuture instead of blocking. | ||
| CompletableFuture<GreetingOutput> future = | ||
| nexusClient.executeAsync( |
There was a problem hiding this comment.
I would show execute here to keep it simpler
| nexusClient.newNexusServiceClient(GreetingNexusService.class, ENDPOINT_NAME); | ||
|
|
||
| demonstrateExecute(greetingClient); | ||
| demonstrateCancel(greetingClient); |
There was a problem hiding this comment.
I don't think we demonstrated cancel or terminate in other SDK language samples. Not strongly opposed, but it is a lot? Whatever we decided to do we should be consistent
| NexusServiceClient<GreetingNexusService> greetingClient = | ||
| nexusClient.newNexusServiceClient(GreetingNexusService.class, ENDPOINT_NAME); | ||
|
|
||
| demonstrateExecute(greetingClient); |
There was a problem hiding this comment.
Another thing we discussed with Product we want to show is getting a handle to an operation that has already been started, would be good to show that when we show execute
|
|
||
| demonstrateExecute(greetingClient); | ||
| demonstrateCancel(greetingClient); | ||
| demonstrateTerminate(greetingClient, client); |
There was a problem hiding this comment.
I'd also demonstrate start
There was a problem hiding this comment.
using the workflow operation specifically. I would show starting and then getting the result in two steps. That is by far the most common use of Nexus so we should show that IMO
| demonstrateCancel(greetingClient); | ||
| demonstrateTerminate(greetingClient, client); | ||
| demonstrateVisibility(nexusClient); | ||
| demonstrateClientOptionsAndInterceptors(stubs, namespace); |
There was a problem hiding this comment.
Interceptors are a very advanced option I would leave them off this sample
| logger.info( | ||
| "Operation id={} final status: {}", | ||
| handle.getNexusOperationId(), | ||
| awaitTerminalStatus(handle, Duration.ofSeconds(10))); |
There was a problem hiding this comment.
We should not be encouraging users to manually poll describe, users can just get the result of the operation through the handle
| private static final Logger logger = LoggerFactory.getLogger(StandaloneClientStarter.class); | ||
|
|
||
| // Must match the Nexus endpoint configured on the server (see README). | ||
| public static final String ENDPOINT_NAME = "nexusstandalone-endpoint"; |
There was a problem hiding this comment.
nit
| public static final String ENDPOINT_NAME = "nexusstandalone-endpoint"; | |
| public static final String ENDPOINT_NAME = "nexus-standalone-operation-endpoint"; |
| // A per-run suffix appended to workflow-backed operation names so their backing workflow IDs are | ||
| // unique on each run. Without this, re-running against the same server (no restart) would reuse | ||
| // deterministic workflow IDs from the previous run and collide. | ||
| private static final String RUN_ID = UUID.randomUUID().toString().substring(0, 8); |
There was a problem hiding this comment.
Run ID already has a meaning in Temporal as a server generated UUID I wouldn't overload the term here. No strong opinion on the term other then not using an existing term
| Create the namespace and the Nexus endpoint: | ||
|
|
||
| ```bash | ||
| temporal operator namespace create --namespace default |
There was a problem hiding this comment.
with the dev server
Sample code for Standalone Nexus operations. This cannot be checked in before the matching SDK changes - temporalio/sdk-java#2872