Add support for upsert memo #2202
Conversation
| @@ -156,6 +156,11 @@ public void upsertTypedSearchAttributes(SearchAttributeUpdate<?>... searchAttrib | |||
| next.upsertTypedSearchAttributes(searchAttributeUpdates); | |||
| } | |||
|
|
|||
| @Override | |||
| public void upsertMemo(Map<String, Object> memo) { | |||
| next.upsertSearchAttributes(memo); | |||
There was a problem hiding this comment.
| next.upsertSearchAttributes(memo); | |
| next.upsertMemo(memo); |
There was a problem hiding this comment.
Tests passed so looks like we must be missing some test coverage?
There was a problem hiding this comment.
Yep added some test coverage for this, will need to think long term if there is some way I can test all the interceptors programatically
| @@ -1186,6 +1186,16 @@ public static void upsertTypedSearchAttributes( | |||
| WorkflowInternal.upsertTypedSearchAttributes(searchAttributeUpdates); | |||
| } | |||
|
|
|||
| /** | |||
| * Updates a Workflow Memos by applying {@code memoUpdates} to the existing Memos set attached to | |||
There was a problem hiding this comment.
Can you clarify how a memo value is removed? Is this done by setting a null object? So can memo contain a null value for a key? In .NET we accept memo update objects to remove any ambiguity. I forget what Go does here.
There was a problem hiding this comment.
Yes by setting a null value same as Go.
There was a problem hiding this comment.
Is there a difference between a null value with key present and an absent key? Specifically, if I do something like upsertMemo(Collections.singletonMap("key-that-exists-from-start", null)), what will the raw memo be if I do describe workflow?
There was a problem hiding this comment.
key-that-exists-from-start would be removed by the server just like Go
There was a problem hiding this comment.
👍 May be worth mentioning in docs that that is how to remove a key
| @@ -156,6 +156,11 @@ public void upsertTypedSearchAttributes(SearchAttributeUpdate<?>... searchAttrib | |||
| next.upsertTypedSearchAttributes(searchAttributeUpdates); | |||
| } | |||
|
|
|||
| @Override | |||
| public void upsertMemo(Map<String, Object> memo) { | |||
There was a problem hiding this comment.
Does Java offer the ability for me to see the whole memo collection inside the workflow? For instance if I wanted to know all memo keys in a workflow, can I? Do we need to add that?
There was a problem hiding this comment.
No and No, at least no one has asked for it and I see no reason this PR would need to add it.
There was a problem hiding this comment.
Works for me. My concern is if/when we do we need to make sure to more explicitly make sure null means remove from a Map POV.
Quinn-With-Two-Ns
force-pushed
the
issue-623
branch
from
1bc8e58 to
cfc0354
Compare
Add support for upsert memo to the Java SDK and test server.
closes #623