-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
VTAdmin(API): Add workflow start/stop endpoint #16658
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16658 +/- ##
==========================================
- Coverage 68.93% 68.90% -0.04%
==========================================
Files 1562 1562
Lines 200767 200831 +64
==========================================
- Hits 138407 138376 -31
- Misses 62360 62455 +95 ☔ View full report in Codecov by Sentry. |
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.
Nice work, @beingnoble03 ! I only had one small request about the span name. If you can change that — unless I'm missing something and you or @rohit-nayak-ps disagree — then I'll go ahead and merge it.
Thanks!
go/vt/vtadmin/api.go
Outdated
@@ -1687,6 +1693,73 @@ func (api *API) GetWorkflowStatus(ctx context.Context, req *vtadminpb.GetWorkflo | |||
}) | |||
} | |||
|
|||
// StartWorkflow is part of the vtadminpb.VTAdminServer interface. | |||
func (api *API) StartWorkflow(ctx context.Context, req *vtadminpb.StartWorkflowRequest) (*vtctldatapb.WorkflowUpdateResponse, error) { | |||
span, ctx := trace.NewSpan(ctx, "API.WorkflowUpdate") |
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.
We should use the API call name for the trace span here and for stop as well. WorkflowUpdate has its own trace span. Then the action for the span is also obvious (now it's not clear what we're updating in the span info).
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.
Oh! Thanks for noticing. I forgot to change here.
Signed-off-by: Noble Mittal <[email protected]>
5238d0f
to
da45f9e
Compare
Description
This PR adds
start
andstop
VReplication workflow endpoints in the VTAdmin-API. These are to be further used in VTAdmin-Web.Related Issue(s)
Checklist
Deployment Notes