-
Notifications
You must be signed in to change notification settings - Fork 207
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
Replace all vtctlclient usage for vreplication commands #1619
Conversation
✅ Deploy Preview for vitess ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
a0a68ef
to
03eedea
Compare
@@ -89,7 +89,6 @@ As a result, connection pools should be sized mindful of the capacity of the und | |||
#### `--db_allprivs_user` | |||
|
|||
* (default 'vt_allprivs') | |||
* Created on demand by `vtctlclient ExecuteFetchAsAllPrivs` |
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.
Why are we not keeping this for vtctldclient
,
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.
There's no ExecuteFetchAsAllPrivs
vtctldclient command. I'm not sure that we need one either.
/cc @ajm188
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.
there's also no equivalent RPC for vtctld server. this was deliberate, as me, @deepthi and others agreed it was no longer needed
@@ -31,7 +31,7 @@ All of the command options and parameters are listed in our [reference page for | |||
Let's start by loading some sample data: | |||
|
|||
```bash | |||
$ mysql < ../common/insert_commerce_data.sql |
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.
Should we remove the $
in all shell snippet?
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 necessary, but the general standard (markdown linter e.g.) is that you do NOT include the prompt if the command is not run and thus the output not shown (but instead you're only showing the command that the reader should run). That's why I removed it here, as we do not include the command output.
03eedea
to
da26e25
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
da26e25
to
da9b567
Compare
Those will have to be done separately Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
cef99d2
to
71ce78d
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
9a78349
to
51846fe
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
d1a02e7
to
9dc4e14
Compare
d516944
to
0c38eb2
Compare
0c38eb2
to
9970fad
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
the Materialize, Mount, and Migrate commands are not documented yet. I will open a separate PR to rebuild the reference docs. Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
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, thanks for the ton of work required to do this!
2e19dfd
to
84ccc40
Compare
Signed-off-by: Matt Lord <mattalord@gmail.com>
Signed-off-by: Matt Lord <mattalord@gmail.com>
Fixes: vitessio/vitess#14206