-
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
Add vtctldclient missing cmds #17442
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[email protected]>
Signed-off-by: Matt Lord <[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 #17442 +/- ##
==========================================
- Coverage 67.68% 67.61% -0.07%
==========================================
Files 1583 1585 +2
Lines 254321 254558 +237
==========================================
- Hits 172131 172120 -11
- Misses 82190 82438 +248 ☔ View full report in Codecov by Sentry. |
9824c10
to
ec59343
Compare
Signed-off-by: Matt Lord <[email protected]>
ec59343
to
851ccac
Compare
Signed-off-by: Matt Lord <[email protected]>
8c5f3e9
to
05c4c1d
Compare
Signed-off-by: Matt Lord <[email protected]>
} | ||
|
||
message ValidatePermissionsKeyspaceResponse { | ||
} |
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.
Does this not contain any results?
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.
Yeah, it returns an error if there's a validation failure:
Lines 3259 to 3269 in 76abae4
func commandValidatePermissionsKeyspace(ctx context.Context, wr *wrangler.Wrangler, subFlags *pflag.FlagSet, args []string) error { | |
if err := subFlags.Parse(args); err != nil { | |
return err | |
} | |
if subFlags.NArg() != 1 { | |
return fmt.Errorf("the <keyspace name> argument is required for the ValidatePermissionsKeyspace command") | |
} | |
keyspace := subFlags.Arg(0) | |
return wr.ValidatePermissionsKeyspace(ctx, keyspace) | |
} |
We could change that, of course, but here I'm primarily just porting these missing commands over.
// testCopySchemaShards tests that schema from source is correctly applied to destination | ||
func testCopySchemaShards(t *testing.T, source string, shard int) { |
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 removed the CopySchemaShard
client command tests since this client command is no longer used anywhere that I could find so we can kill it off.
Description
This implements client commands in
vtctldclient
that were never ported during the client migration. These are things that were identified while moving all endtoend tests to usevtctldclient
exclusively in #17441.We will (after merging in
main
once #17441 is merged) also then replace all remainingvtctlclient
usage in the e2e tests with usage of these newvtctldclient
commands.Related Issue(s)
Checklist