Add ChildWorkflowOptions support to WorkflowImplementationOptions (#2) - #2887
Merged
Quinn-With-Two-Ns merged 1 commit intoAug 27, 2026
Merged
Conversation
|
|
|
|
porunov
force-pushed
the
feature/child-workflow-options
branch
from
May 25, 2026 21:04
bd8e2d9 to
3295d53
Compare
maciejdudko
previously requested changes
Jun 1, 2026
maciejdudko
left a comment
maciejdudko
left a comment
Contributor
There was a problem hiding this comment.
Hi @porunov, thank you for your contribution. The code looks good, but the tests need to be improved before we can merge this - see comment.
Comment on lines
+46
to
+47
| @Test | ||
| public void testDefaultChildWorkflowOptionsApplied() { |
Contributor
There was a problem hiding this comment.
This test does not actually assert that the right options were used. Please rewrite it so that the child workflow only succeeds if the default options were correctly applied. I think checking its memo would be most straight forward.
Please also add a test that verifies default, not per-type child workflow options work too.
Contributor
Author
There was a problem hiding this comment.
Thank you @maciejdudko for pointing at it. There was a bug when the default options would overwrite per type options, but it's now fixed. Also added the tests as you requested. All tests are passing now. Let me know please if there are any other concerns with this PR.
voplica-git
force-pushed
the
feature/child-workflow-options
branch
from
June 26, 2026 10:29
3295d53 to
948ecc8
Compare
porunov
force-pushed
the
feature/child-workflow-options
branch
from
July 7, 2026 12:28
948ecc8 to
101bb90
Compare
Contributor
Author
|
@maciejdudko I've rebased in from |
…plementationOptions Fixes temporalio#2790 Allow predefining ChildWorkflowOptions on WorkflowImplementationOptions, mirroring the existing ActivityOptions, LocalActivityOptions and NexusServiceOptions support: - Add setChildWorkflowOptions(Map) and setDefaultChildWorkflowOptions() builder methods, matching getters, and equals/hashCode/toString/ toBuilder support on WorkflowImplementationOptions - Expose the options through SyncWorkflowContext, served directly from the immutable WorkflowImplementationOptions - Add ChildWorkflowOptions.Builder#mergeChildWorkflowOptions(): non-null override fields win, except contextPropagators lists are concatenated (matching ActivityOptions.Builder#mergeActivityOptions), the mutually exclusive searchAttributes/typedSearchAttributes are merged as one logical field so the merged options never carry both flavors, and VERSIONING_INTENT_UNSPECIFIED is treated as unset - Resolve the predefined options for both typed and untyped child stubs (WorkflowInternal.newChildWorkflowStub and newUntypedChildWorkflowStub, so DynamicWorkflow parents are covered too) with the following precedence, highest to lowest, merged field by field: options passed to the stub creation method > per-type options (setChildWorkflowOptions) > default options (setDefaultChildWorkflowOptions) - Keep child stub creation cost unchanged: the interface metadata is computed once and passed into ChildWorkflowInvocationHandler instead of being reflectively re-derived - Keep the previous public WorkflowImplementationOptions constructor as a backward-compatible delegating overload - Document the new behavior on the Workflow child stub factory methods and warn against predefining workflowId, which would be applied to every child of the matching scope Fix child workflow options precedence: the predefined options were merged in the wrong order, so when no options were passed to newChildWorkflowStub the default options overrode the per-type options. The default options have the lowest precedence and must never override per-type options. The merge order is now correct, and the javadocs on setChildWorkflowOptions and setDefaultChildWorkflowOptions describe the actual precedence. Tests: - Verify the applied options through the child's memo in DefaultChildWorkflowOptionsSetOnWorkflowTest (covering default, per-type, explicit and field-level merge precedence for both typed and untyped stubs), so a test only passes if the expected options actually took effect - Add an exhaustive unit test for ChildWorkflowOptions#mergeChildWorkflowOptions that exercises every field, plus dedicated tests for the search attribute flavors, context propagator concatenation and unspecified versioning intent Signed-off-by: Oleksandr Porunov <alexandr.porunov@gmail.com>
Quinn-With-Two-Ns
force-pushed
the
feature/child-workflow-options
branch
from
August 27, 2026 03:22
101bb90 to
020e5c2
Compare
Quinn-With-Two-Ns
approved these changes
Aug 27, 2026
Quinn-With-Two-Ns
dismissed
maciejdudko’s stale review
August 27, 2026 03:23
Other SDK engineer taking over review
Quinn-With-Two-Ns
enabled auto-merge (squash)
August 27, 2026 03:24
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
4 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.
Fixes #2790
Tests are added to:
ChildWorkflowOptionsInWorkflowImplementationOptionsTest.javaPotential documentation place: https://docs.temporal.io/develop/java/workflows/child-workflows (it could be something similar as activity options documentation).
Disclaimer: Code was Generated by Opus 4.6 in Copilot. Reviewed manually.