-
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 /api/schemas endpoint to vtadmin-api #7432
[vtadmin-api] Add /api/schemas endpoint to vtadmin-api #7432
Conversation
Signed-off-by: Sara Bee <[email protected]>
aea8170
to
8bf8622
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.
lg2m!
} | ||
|
||
// getSchemas returns all of the schemas across all keyspaces in the given cluster. | ||
func (api *API) getSchemas(ctx context.Context, c *cluster.Cluster, tablets []*vtadminpb.Tablet) ([]*vtadminpb.Schema, error) { |
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.
TIOLI: I know we talked a bit about this file potentially growing very large as the API surface area grows, and these methods don't actually depend on api
for anything, so if you wanted to you could make these top-level private functions in another file, but also this is fine as-is.
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'll leave it for now. I agree with you on one file offering better readability. Even though these methods don't use api
directly, the colocation seems good. One day I'll get used to the verbosity.
tabletSchemas map[string]*tabletmanagerdata.SchemaDefinition | ||
req *vtadminpb.GetSchemasRequest | ||
expected *vtadminpb.GetSchemasResponse | ||
}{ |
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.
Ugh, I am sorry lol. These are great tests though!
Signed-off-by: Sara Bee <[email protected]>
Signed-off-by: Sara Bee <[email protected]>
Signed-off-by: Sara Bee [email protected]
Description
A simplified version of our Slack-internal /api/schemas endpoint. Notably:
"Don't forget" to expand the
api_test.go
diff 😭@ajm188 thanks as always for weathering my
incessant whiningquestions!Queries against local Vitess
curl "http://localhost:14200/api/schemas" | jq .
curl "http://localhost:14200/api/schemas?cluster=nope" | jq .
Related Issue(s)
Checklist
Deployment Notes
N/A
Impacted Areas in Vitess
Components that this PR will affect: