-
Notifications
You must be signed in to change notification settings - Fork 913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support upsert memo #3091
Support upsert memo #3091
Conversation
f8d1085
to
41f4dc4
Compare
4efc28c
to
50edfaa
Compare
50edfaa
to
3ef1495
Compare
} | ||
|
||
// check if UpsertedMemo is not nil, then it's not an empty map | ||
if attributes.UpsertedMemo != nil && len(attributes.GetUpsertedMemo().GetFields()) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This: attributes.UpsertedMemo != nil
is always true
here. You can remove this check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, at this moment yes, but the condition above, as new attributes are added, it would become something like if attributes.UpsertedMemo == nil || attributes.something == nil || ...
. So, this condition here makes sense to check again.
What changed?
Adding support to upsert memo in a similar fashion as upsert search attributes.
This change depends on temporalio/api#218.
Why?
Add ability to add/update memo fields.
Feature request: temporalio/features#119
How did you test it?
Added unit tests. Tested locally and verified the values in ES document was correct.
Potential risks
No risks. This PR doesn't change any existing code, only adds new ones (minus a few minor refactors like renaming functions).
Is hotfix candidate?
No.