-
Notifications
You must be signed in to change notification settings - Fork 882
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
Consolidate migration folder in OSS (NEW) #7100
base: main
Are you sure you want to change the base?
Conversation
if !isNotFoundServiceError(err) { | ||
a.logger.Error("force-replication failed to generate replication task", tag.WorkflowNamespaceID(request.NamespaceID), tag.WorkflowID(we.WorkflowId), tag.WorkflowRunID(we.RunId), tag.Error(err)) | ||
var executionCandidates []definition.WorkflowKey | ||
if request.EnableParentInfo { |
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.
Based on our previous discussion on the old pr, we do not plan to expose the enablePatentInfo here. I plan to remove line 504 and all related tests. Let me know if my understanding is correct
[Consolidate migration folder in OSS](#6893 (comment))
@yux0 @hehaifengcn
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.
can we not move code related to enablePatentInfo over to oss? either remove it from saas-temporal first or do it in this PR.
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.
Sg. removed the enablePatentInfo and related tests
1, Consolidate force replication, handover wf and catchup wf in OSS with control plane.
2, We copied all 7 files from cp and remove the prefix saas.
What changed?
Why?
How did you test it?
Potential risks
Documentation
Is hotfix candidate?