Add support for metadata to test server #2441
Conversation
| * @return Workflow ID of the root Workflow | ||
| */ | ||
| @Nullable | ||
| String getRootWorkflowId(); |
There was a problem hiding this comment.
This is a bit awkward since this can only be null on old servers that didn't have this value.
There was a problem hiding this comment.
Makes sense, may be worth documenting that here in the javadoc. I do appreciate that this is not using Optional.
| * @return Workflow ID of the root Workflow | ||
| */ | ||
| @Nullable | ||
| String getRootWorkflowId(); |
There was a problem hiding this comment.
Makes sense, may be worth documenting that here in the javadoc. I do appreciate that this is not using Optional.
| /** Get the fixed summary for this workflow execution. */ | ||
| @Experimental | ||
| @Nullable | ||
| public String getStaticSummary() { |
There was a problem hiding this comment.
Might consider memoizing this (e.g. backed by an AtomicReference). If not, at least worth clarifying in the javadoc each invocation does data conversion.
There was a problem hiding this comment.
We generally don't ever memoize conversion for these type of things, same for memo. I'll note in the docs
There was a problem hiding this comment.
I wonder if it's worth a more generic implementation of command-to-event user metadata propagation like server instead of the only known current use cases. This would make it more future proof. I understand it'd change the code a bit since there is no generic place where an event is created from a command nor is there a common place to affect an event after it is created. Up to you.
There was a problem hiding this comment.
Yeah possibly, child workflow would still need some special logic, but yeah this should be generic
Quinn-With-Two-Ns
force-pushed
the
issue-2430
branch
from
c90dad2 to
763c7ff
Compare
This PR does a few things:
closes: