-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
ui: Populate database filter dropdown in stmts page with SHOW DATABASES
sql-over-http call
#93657
Conversation
9b250d6
to
5c51452
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.
just a couple nits and fix CI failures for src/views/statements/statements.spec.tsx
and src/app.spec.tsx
:)
Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @gtr)
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
line 91 at r1 (raw file):
// selectDatabases returns the array of all databases in the cluster, // regardless of whether they have statement statistics present in the data.
Could probably just have:
// selectDatabases returns the array of all databases in the cluster.
pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx
line 222 at r1 (raw file):
// selectDatabases returns the array of all databases in the cluster, // regardless of whether they have statement statistics present in the data.
Ditto.
5c51452
to
6c1e1e5
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.
TFTR!
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @THardy98)
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.selectors.ts
line 91 at r1 (raw file):
Previously, THardy98 (Thomas Hardy) wrote…
Could probably just have:
// selectDatabases returns the array of all databases in the cluster.
Done.
pkg/ui/workspaces/db-console/src/views/statements/statementsPage.tsx
line 222 at r1 (raw file):
Previously, THardy98 (Thomas Hardy) wrote…
Ditto.
Done.
fba9a06
to
7fac45b
Compare
bors r+ |
bors retry |
Already running a review |
…SES` sql-over-http call Fixes: cockroachdb#70461. Previously, the databases filter dropdown was populated by the `StatementsResponse` API call. This would result in some databases for which we do not receive any stmts to be ignored. According to above issue, the database filter-drop down should always be populated with cluster databases even when there are no statements or transactions for them. This commit populates the database filter dropdown using the `getDatabasesList()` API call which itself executes the `SHOW DATABASES` SQL query. Release note (ui change): The databases filter dropdown in the stmts page now uses the `getDatabasesList()` API call, resulting in all cluster databases showing up.
7fac45b
to
5d6236e
Compare
Canceled. |
bors r+ |
Build succeeded: |
Fixes: #70461.
Previously, the databases filter dropdown was populated by the
StatementsResponse
API call.This would result in some databases forwhich we do not receive any stmts to be ignored.According to above
issue, the database filter - drop down should always be populated with
cluster databases even when there are no statements or transactions for
them.This commit populates the database filter dropdown using the
getDatabasesList()
API call which itself executes theSHOW DATABASES
SQL query.
Creating a new empty database from the SQL shell:
Clicking the "Databases" dropdown:
Release note(ui change): The databases filter dropdown in the stmts
page now uses the
getDatabasesList()
API call, resulting in allcluster databases showing up.