Skip to content
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

Merged
merged 2 commits into from
Aug 25, 2022
Merged

Support upsert memo #3091

merged 2 commits into from
Aug 25, 2022

Conversation

rodrigozhou
Copy link
Contributor

@rodrigozhou rodrigozhou commented Jul 12, 2022

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.

@rodrigozhou rodrigozhou requested a review from a team as a code owner July 12, 2022 22:01
@alexshtin alexshtin marked this pull request as draft July 12, 2022 22:49
@rodrigozhou rodrigozhou marked this pull request as ready for review August 15, 2022 21:22
@rodrigozhou rodrigozhou force-pushed the upsert-memo branch 7 times, most recently from 4efc28c to 50edfaa Compare August 23, 2022 19:36
}

// check if UpsertedMemo is not nil, then it's not an empty map
if attributes.UpsertedMemo != nil && len(attributes.GetUpsertedMemo().GetFields()) == 0 {
Copy link
Member

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.

Copy link
Contributor Author

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.

@rodrigozhou rodrigozhou merged commit c059c69 into temporalio:master Aug 25, 2022
@rodrigozhou rodrigozhou deleted the upsert-memo branch August 25, 2022 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants