Implement operator commands for Standalone Activities - #3013
Open
GregoryTravis wants to merge 65 commits into
Open
Implement operator commands for Standalone Activities#3013GregoryTravis wants to merge 65 commits into
GregoryTravis wants to merge 65 commits into
Conversation
# Conflicts: # temporal-sdk/src/test/java/io/temporal/client/functional/StandaloneActivityTest.java
maciejdudko
requested changes
Aug 18, 2026
maciejdudko
left a comment
maciejdudko
left a comment
Contributor
There was a problem hiding this comment.
Looking good, although there are a few design choices that warrant more discussion.
Ported from sdk-python#1782, written against our current API shape. Replaces describePayloadFieldsAreOptIn, describeReadsInputAndOutcome and describeReadsFailureOutcome with one test covering all three, and more: - a new HeartbeatFailIncrementActivity heartbeats, fails once, then succeeds, so a single describe carries input, result, heartbeat details and a last failure at the same time. The three tests it replaces each used a different activity, so no describe ever held them together. - pins hasLastFailure true while getOutcomeFailure is null on a succeeded activity that failed once, the terminal-versus-attempt distinction that was untested. - asserts the accessors are absent, not merely that has* is false. The activity takes and returns Integer rather than int: MethodExtractor cannot probe a method reference with primitive types.
The four api#792 flags were covered only functionally, so nothing proved the SDK actually sets them on DescribeActivityExecutionRequest — a default-on bug would have looked identical from observable state. Three cases against a stubbed client: defaults ask for nothing, all four are forwarded, and asking for one does not set the others. Brings Java level with Python and Go.
The stripping in RootActivityClientInvoker existed with no coverage. It only matters against a server that ignores the opt-ins, which no functional test can produce, so it needs a stub that returns every payload field regardless of what was asked for. Three cases: nothing requested strips all four, everything requested keeps all four, and stripping is per field.
It asserted that a single set option produces a mask naming exactly that option. The mask tests in every SDK already assert the mask names exactly what changed, and a one-option case catches nothing the multi-option case misses. Java was the only SDK with it, so removing it is also parity rather than porting three near-duplicates.
RestoreOriginalOptions reuses the update-options interceptor rather than having one of its own, distinguished purely by the restore flag with an empty mask. An interceptor watching option updates would otherwise silently miss restores, and nothing pinned that. Ported from the Python interceptor suite, which was the only one asserting it.
Java clears the same way Go does, by passing a zero value rather than a dedicated sentinel: Duration.ZERO is non-null so the path reaches the mask, and the server normalizes a zero timeout back to unset. Null keeps the path out of the mask entirely. Neither behaviour was covered. Brings Java level with Go and Python.
GregoryTravis
changed the title
maciejdudko
requested changes
Sep 5, 2026
|
|
||
| @Override | ||
| public String toString() { | ||
| return "PauseActivityOptions{" + "reason='" + reason + "'" + '}'; |
Contributor
There was a problem hiding this comment.
Nit:
Suggested change
| return "PauseActivityOptions{" + "reason='" + reason + "'" + '}'; | |
| return "PauseActivityOptions{" + "reason='" + reason + "'}"; |
Comment on lines
+10
to
+17
| /** | ||
| * Options for {@link UntypedActivityHandle#updateOptions(ActivityOptionsUpdate<?>...)}. | ||
| * | ||
| * <p>Only the fields that are explicitly set are sent to the server; a derived field mask ensures | ||
| * that unset fields are left unchanged (a partial update). | ||
| */ | ||
| @Experimental | ||
| public final class ActivityExecutionOptions { |
Contributor
There was a problem hiding this comment.
Documentation is inaccurate with the new design.
Comment on lines
+19
to
+21
| public static Builder newBuilder() { | ||
| return new Builder(); | ||
| } |
Contributor
There was a problem hiding this comment.
With the new design, this class doesn't need a builder. Instead it should have a package-visibility constructor that takes proto ActivityOptions.
Contributor
There was a problem hiding this comment.
I think it would be less confusing if ActivityOptionsKey was an inner class of ActivityOptionsUpdate, and the static constants in ActivityOptionsKeys were also moved into ActivityOptionsUpdate. I'd also remove the methods valueSet and valueUnset from ActivityOptionsUpdate and keep them only on ActivityOptionsKey. (and rename them to set and unset for brevity.)
Comment on lines
+444
to
+445
| private final ActivityOptions activityOptions; | ||
| private final FieldMask updateMask; |
Contributor
There was a problem hiding this comment.
UpdateActivityOptionsInput should receive List<ActivityOptionsUpdate> so that it can be intercepted, and the computation of activityOptions and updateMask should happen in RootActivityClientInvoker.
Comment on lines
+222
to
+225
| switch (key.getName()) { | ||
| case "task_queue.name": | ||
| options.setTaskQueue(TaskQueue.newBuilder().setName((String) value).build()); | ||
| break; |
Contributor
There was a problem hiding this comment.
The logic how to set the field in proto should be a method of ActivityOptionsKey itself, so that we don't have to match on the exact string value of the path. (Each key constant would have its own implementation.)
| @Experimental | ||
| public final class ActivityOptionsKey<T> { | ||
|
|
||
| private final String name; |
Contributor
There was a problem hiding this comment.
Java SDK hasn't adopted the CHANGELOG.md file yet. Remove this.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement operator commands for standalone activities
Adds pause, unpause, reset, and update-options to standalone activities, plus
the describe surface needed to observe their effects.
Standalone activities already supported start, result, describe, cancel, and
terminate. This adds the four operator commands the server exposes for them, so
an operator can hold, resume, restart, and retune a running activity without
going through a workflow.
Describe: payload fields are opt-in
DescribeActivityExecutionRequestgates four payload-bearing fields behindper-call flags (api#792). All four are now plumbed through
DescribeActivityOptionsand default to false.