Skip to content

Stand alone operations for Nexus#2872

Open
Evanthx wants to merge 6 commits into
masterfrom
sano
Open

Stand alone operations for Nexus#2872
Evanthx wants to merge 6 commits into
masterfrom
sano

Conversation

@Evanthx
Copy link
Copy Markdown
Contributor

@Evanthx Evanthx commented May 7, 2026

What was changed

Why?

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@Evanthx Evanthx requested a review from a team as a code owner May 7, 2026 22:53
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClient.java
* type binding to an {@link UntypedNexusClientHandle} (returned by {@link
* NexusClient#getHandle(String)}) by calling one of the {@link #fromUntyped} factories.
*/
public interface NexusClientHandle<R> extends UntypedNexusClientHandle {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public interface NexusClientHandle<R> extends UntypedNexusClientHandle {
public interface NexusOperationHandle<R> extends UntypedNexusOperationHandle {

Please keep consistent naming with other Handles like https://github.com/temporalio/sdk-java/blob/master/temporal-sdk/src/main/java/io/temporal/client/ActivityHandle.java

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nexus doc had "NexusOperationHandle". I didn't use that name as there was already a NexusOperationHandle class and I didn't want to duplicate it. Even if in a different package, that just seemed confusing.

The reason I went to NexusClient in a lot of class names was to avoid naming collisions like that - again, even if in different packages it seemed confusing, and I wanted some form of consistency. So I named this NexusClientHandle to show that it was linked to the other NexusClient classes.

That being said, I do want to be consistent, but might have to check in with you and talk this one out. Maybe we can make these NexusOperationExecutionHandle and UntypedNexusOperationExecutionHandle, but then we lose the link to the other NexusClient classes - though would we use these for all Nexus operations so maybe we don't need such a link?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed to NexusOperationHandle as per conversation - we feel that the names being in different packages that have different use cases which should never be mixed should be sufficient to avoid confusion, especially as the user will get this returned to them and won't be creating these classes.

Leaving this conversation unresolved to make sure the SDK team sees it and can weigh in!

Comment thread temporal-sdk/src/main/java/io/temporal/client/UntypedNexusClientHandle.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/UntypedNexusClientHandle.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/UntypedNexusServiceClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/UntypedNexusServiceClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClientOperationOptions.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusServiceClient.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusServiceClientImpl.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusServiceClientImpl.java Outdated
Comment thread temporal-sdk/src/test/java/io/temporal/client/nexus/NexusServiceClientTest.java Outdated
Comment thread temporal-sdk/src/test/java/io/temporal/client/nexus/NexusClientHandleTest.java Outdated
Comment thread temporal-sdk/src/test/java/io/temporal/client/nexus/NexusClientTest.java Outdated
Comment thread temporal-sdk/src/test/java/io/temporal/client/nexus/NexusClientHandleTest.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClientOperationOptions.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusClientOperationOptions.java Outdated
Comment thread temporal-sdk/src/main/java/io/temporal/client/NexusServiceClient.java Outdated
@Quinn-With-Two-Ns
Copy link
Copy Markdown
Contributor

Reviewed most of the public API, didn't get to into the tests or implementation for now since some stuff will likely change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants