-
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: Support for Workflow Actions #16816
VTAdmin: Support for Workflow Actions #16816
Conversation
Signed-off-by: Noble Mittal <[email protected]>
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 #16816 +/- ##
==========================================
- Coverage 69.41% 69.36% -0.05%
==========================================
Files 1570 1570
Lines 203929 204053 +124
==========================================
- Hits 141551 141544 -7
- Misses 62378 62509 +131 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
…kflow Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
Signed-off-by: Noble Mittal <[email protected]>
267f345
to
bb02cc3
Compare
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.
Looks good! I played around with it and it works as expected. The pop-up window on complete that presents you with the relevant options is really nice.
The only surprise is that we do not have this for Reshard too since it's otherwise roughly equivalent support-wise on main.
@@ -384,6 +386,7 @@ func (api *API) Handler() http.Handler { | |||
router.HandleFunc("/migration/{cluster_id}/{keyspace}/launch", httpAPI.Adapt(vtadminhttp.LaunchSchemaMigration)).Name("API.LaunchSchemaMigration").Methods("PUT", "OPTIONS") | |||
router.HandleFunc("/migration/{cluster_id}/{keyspace}/retry", httpAPI.Adapt(vtadminhttp.RetrySchemaMigration)).Name("API.RetrySchemaMigration").Methods("PUT", "OPTIONS") | |||
router.HandleFunc("/migrations/", httpAPI.Adapt(vtadminhttp.GetSchemaMigrations)).Name("API.GetSchemaMigrations") | |||
router.HandleFunc("/movetables/{cluster_id}/complete", httpAPI.Adapt(vtadminhttp.MoveTablesComplete)).Name("API.MoveTablesComplete") |
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.
Any reason we're doing movetables and not reshard? Looks like this PR branch does not appear to have the Reshard work from #16903, so that may be why. Can we add it here so that we have some uniformity? We can also do it in a follow-up if needed.
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.
Same for Materialize, which we'll have shortly: #16941
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.
I think complete
can be done for Reshard. I can do that in a follow-up PR.
Signed-off-by: Noble Mittal <[email protected]>
Updated the branch with latest changes, and tested the changes locally after the merge. |
Description
This PR adds support for workflow
complete
,switchtraffic
,reversetraffic
andcancel
operation from VTAdmin.Related Issue(s)
Screenshots
Checklist
Deployment Notes