Add UpsertMemo API #883
Add UpsertMemo API #883
Conversation
rodrigozhou
requested review from
mfateev,
Sushisource,
alexshtin,
Spikhalskiy,
bergundy,
cretz and
mmcshane
as code owners
| func mergeMemo(current, upsert *commonpb.Memo) *commonpb.Memo { | ||
| if current == nil || len(current.Fields) == 0 { | ||
| if upsert == nil || len(upsert.Fields) == 0 { | ||
| return nil |
There was a problem hiding this comment.
If an empty memo was on workflow info, this has now made it nil
There was a problem hiding this comment.
In theory, the new memo cannot be nil or an empty map. See validateAndSerializeMemo which is executed before merging the memo.
| } | ||
|
|
||
| func mergeMemo(current, upsert *commonpb.Memo) *commonpb.Memo { | ||
| if current == nil || len(current.Fields) == 0 { |
There was a problem hiding this comment.
Could also just len(current.GetFields()) == 0
|
Didn't look at the code but something I wish we'd done with search attributes in TS is delay sending the command until the end of an "activation", so if a user calls |
|
@rodrigozhou note that I moved the original issue to sdk-features and marked this PR as not closing for that issue, we need to implement in all SDKs. |
rodrigozhou
force-pushed
the
upsert-memo
branch
from
8c6fd96 to
405b67a
Compare
|
The CI failure is because the CI used the docker-compose repo to start the temporal server, which doesn't include EDIT: skipped |
rodrigozhou
force-pushed
the
upsert-memo
branch
from
e95dbc8 to
1bdf276
Compare
rodrigozhou
force-pushed
the
upsert-memo
branch
from
1bdf276 to
bba5194
Compare
| if strictMode && !isMemoMatched(eventAttributes.UpsertedMemo, commandAttributes.UpsertedMemo) { | ||
| return false | ||
| } |
There was a problem hiding this comment.
StrictMode was there for historical reason. But I don't think they are needed anymore. We probably should remove strictMode. @cretz
(No change requested for this PR.)
rodrigozhou
force-pushed
the
upsert-memo
branch
from
bea6fb7 to
713ef57
Compare
What was changed
Adding an API for
UpsertMemo. It merges with current memo and creates a commandModifyWorkflowPropertieswith the updated memo.This change depends on temporalio/api#218.
Why?
Feature request: temporalio/features#119
Checklist