Conversation
| @Nonnull Deadline deadline, | ||
| Class<R> resultClass, | ||
| @Nullable Type resultType, | ||
| @Nullable NexusSerializationContext serializationContext) { |
There was a problem hiding this comment.
We shouldn't need to pass this through the interceptor, we don't do this for other SerializationContext. We can generally just construct it just in time before use
There was a problem hiding this comment.
Actually I. will slightly retract my comment since this is just on GetNexusOperationResultInput
There was a problem hiding this comment.
I am debating a if it would be better to expand it here instead of passing the context but not sure TBH
There was a problem hiding this comment.
I can, but there are two calls to this method and both have a serialization context. So it feels odd to take a class and pass through each individual field of it instead of the class, just so that the method can then create a new copy of that class from the individual fields. Then if we ever add a field that gets weird again.
There was a problem hiding this comment.
Not sure it's the exact same situation, but I felt it was the better choice to expand it in Python as well when reviewing this PR temporalio/sdk-python#1828 from @JoshuaFrenchwood.
There was a problem hiding this comment.
Thinking about this ... the places making the call are internal and do create the context, so unwrapping it or not just isn't a big deal honestly, so I just did it.
But it makes me wonder - what made this stand out to you two? Not asking to disagree but I wanted to understand - normally I'd go for earlier construction and fewer arguments, it surfaces errors earlier and is more readable. What am I missing that made both of you lean the other way?
|
Overall makes sense I think we just need some integration tests |
The logic for this code change was originally in Python:
temporalio/sdk-python#1828
This is a port of that logic into the Java SDK.
What was changed
Added NexusSerializationContext for nexus callers and sync handlers. This allows data and failure converters to use nexus endpoint, service and operation to be used for encoding and decoding.
Why?
This enables codecs to select serialization behavior or encryption keys by Nexus endpoint, service, or operation.
For example, workflows calling two Nexus endpoints can encrypt each endpoint’s payloads with a different key while ensuring that inputs, results, and failures are decoded with the converter selected for the corresponding operation.
How was this tested:
Unit testing