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
Add SetWorkflowExecution persistence API #2523
Conversation
* Add SetWorkflowExecution persistence API for allow overwriting a mutable state * Update tests
// dbVersion is for CAS, so the db record version will be set to `resetWorkflow.DBRecordVersion` | ||
// while CAS on `resetWorkflow.DBRecordVersion - 1` |
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.
update comment to not use resetWorkflow?
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.
sure
batch := d.Session.NewBatch(gocql.LoggedBatch) | ||
|
||
shardID := request.ShardID | ||
workflow := request.SetWorkflowSnapshot |
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.
nit: i would use snapshot as this local variable name.
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.
the GitHub phone app collapse the review comments so did not notice this after merging the PR, will address the comments in the next PR
@@ -75,7 +75,7 @@ func applyWorkflowMutationTx( | |||
workflowMutation.DBRecordVersion, | |||
); err != nil { | |||
switch err.(type) { | |||
case *p.WorkflowConditionFailedError: | |||
case *p.WorkflowConditionFailedError, *p.ConditionFailedError: |
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.
wait, did we missed this?
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.
Not a big issue, only the returned error type is not expected.
request.RangeID, | ||
) | ||
|
||
conflictRecord := make(map[string]interface{}) |
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.
nit: conflictRecords?
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 is actually one row, the iter can return other rows if multiple rows are involved
What changed?
Why?
More functionality
How did you test it?
New tests
Potential risks
N/A
Is hotfix candidate?
No