-
Notifications
You must be signed in to change notification settings - Fork 13
feat: add endpoint to connect to db code snippets #2198
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
Conversation
@@ -48,7 +44,6 @@ export function SchemaTree(props: SchemaTreeProps) { | |||
{currentData: actionsSchemaData, isFetching: isActionsDataFetching}, | |||
] = tableSchemaDataApi.useLazyGetTableSchemaDataQuery(); | |||
|
|||
const [querySettings] = useQueryExecutionSettings(); |
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.
It was used only in hook dependencies
// Name: ydb_vla_dev02 | ||
// Title: YDB DEV VLA02 | ||
const clusterName = name ?? clusterNameFromQuery ?? undefined; | ||
const clusterTitle = title ?? clusterName; |
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 little refactoring to make code more clear
// If there is enpdoint from props, we don't need to request tenant data | ||
// Also we should not request tenant data if we are in single cluster mode | ||
// Since there is no ControlPlane data in this case | ||
const shouldRequestTenantData = database && !endpointFromProps && !singleClusterMode; |
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 left endpoint
in props to make possible to use dialog in Databases table, where we know all the params from table data and do not need to request them additionally
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.
Pull Request Overview
This PR adds a new endpoint for connecting to DB code snippets and updates various API calls to use an object parameter for the endpoint and tenant queries. Key changes include restructuring API call signatures, updating tenant and cluster reducers to support the new endpoint parameter, and enhancing the ConnectToDB components with a new utility function to prepare endpoints.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/store/reducers/tenants/tenants.ts | Updated getTenants call to pass parameters as an object |
src/store/reducers/tenant/tenant.ts | Modified getTenantInfo to support an optional clusterName parameter |
src/store/reducers/cluster/cluster.ts | Refactored cluster information retrieval and introduced separate title variable |
src/services/api/viewer.ts | Adjusted getTenants signature to use object-based parameters |
src/services/api/meta.ts | Updated getTenants to accept both clusterName and databaseName |
src/containers/Tenant/ObjectSummary/SchemaTree/SchemaTree.ts | Removed an unused hook and updated dependencies |
src/containers/Tenant/Diagnostics/TenantOverview/TenantOverview.tsx | Added clusterName to tenant query |
src/containers/Header/Header.tsx | Updated header breadcrumbs to use a resolved cluster title |
src/components/ConnectToDB/utils.ts | Added a utility function to remove URL search parameters and trailing slashes |
src/components/ConnectToDB/snippets.ts | Updated getSnippetCode to call prepareEndpoint for improved endpoint handling |
src/components/ConnectToDB/ConnectToDBDialog.tsx | Refactored dialog to conditionally query tenant data and wrap snippet in a loader |
src/components/ConnectToDB/test/utils.test.ts | Added tests for the new prepareEndpoint utility |
Comments suppressed due to low confidence (1)
src/store/reducers/cluster/cluster.ts:149
- [nitpick] The local variable 'clusterName' is derived from multiple sources which may be ambiguous; consider renaming it to a more descriptive name like 'resolvedClusterName' for clarity.
const clusterName = name ?? clusterNameFromQuery ?? undefined;
Closes #1810
Stand - you should click on "Connect" button in the top right corner to see the Dialog with endpoint: https://nda.ya.ru/t/6-FHwxii7DsYUf
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
😟 No changes in tests. 😕
Bundle Size: 🔺
Current: 83.37 MB | Main: 83.36 MB
Diff: +4.75 KB (0.01%)
ℹ️ CI Information