-
Notifications
You must be signed in to change notification settings - Fork 17
New Flink Relations view mode showing toplevel relations + their direct columns #2934
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
This comment has been minimized.
This comment has been minimized.
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 introduces a new Flink Relations view mode for browsing database tables, views, and their columns. The relations view becomes the new default mode, replacing UDFs as the initial view. The implementation includes new models for Flink relations and columns, a system catalog query that unions table and column metadata, and comprehensive test coverage.
Key changes:
- New default view mode showing Flink relations (tables/views) with their columns
- Consolidated system catalog models in
flinkSystemCatalog.ts(moved fromflinkUDF.ts) - New system catalog query combining
INFORMATION_SCHEMA.TABLESandCOLUMNSdata
Reviewed Changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/unit/testResources/makeRelationRow.ts |
New test helper functions for creating mock relation and column data |
tests/unit/testResources/flinkUDF.ts |
Updated import path for consolidated system catalog models |
tests/unit/testResources/flinkRelation.ts |
New test fixtures for relations and columns |
src/viewProviders/multiViewDelegates/flinkUDFsDelegate.ts |
Updated imports for renamed model file |
src/viewProviders/multiViewDelegates/flinkUDFsDelegate.test.ts |
Added extension context setup and updated imports |
src/viewProviders/multiViewDelegates/flinkRelationsDelegate.ts |
New delegate implementing relations view logic |
src/viewProviders/multiViewDelegates/flinkRelationsDelegate.test.ts |
Comprehensive tests for relations delegate |
src/viewProviders/multiViewDelegates/constants.ts |
Added Relations view mode enum value |
src/viewProviders/flinkDatabase.ts |
Integrated relations delegate and set as default view |
src/viewProviders/flinkDatabase.test.ts |
Updated tests with async setup and UDFs mode initialization |
src/viewProviders/baseModels/multiViewBase.ts |
Enhanced getChildren signature to support parent elements |
src/storage/resourceManager.ts |
Updated import for consolidated models |
src/storage/resourceManager.test.ts |
Updated import for consolidated models |
src/models/flinkUDF.ts |
File deleted, moved to flinkSystemCatalog.ts |
src/models/flinkUDF.test.ts |
File deleted, tests moved to flinkSystemCatalog.test.ts |
src/models/flinkSystemCatalog.ts |
New consolidated file with UDF, relation, and column models |
src/models/flinkSystemCatalog.test.ts |
Comprehensive tests for all system catalog models |
src/loaders/utils/udfSystemCatalogQuery.ts |
Updated imports for consolidated models |
src/loaders/utils/udfSystemCatalogQuery.test.ts |
Added explicit type annotations for parameters |
src/loaders/utils/relationsAndColumnsSystemCatalogQuery.ts |
New query builder and parser for relations/columns data |
src/loaders/utils/relationsAndColumnsSystemCatalogQuery.test.ts |
Tests for query generation and response parsing |
src/loaders/ccloudResourceLoader.ts |
Added getFlinkRelations method |
src/loaders/ccloudResourceLoader.test.ts |
Tests for new getFlinkRelations method |
src/extension.ts |
Registered new view commands and updated default mode |
src/commands/flinkUDFs.ts |
Updated imports for consolidated models |
src/commands/flinkDatabaseView.ts |
New command for switching to relations view |
src/commands/flinkDatabaseView.test.ts |
Tests for relations view command |
package.json |
Added relations view mode command and menu entries |
| } from "../../models/flinkSystemCatalog"; | ||
| import type { CCloudFlinkDbKafkaCluster } from "../../models/kafkaCluster"; | ||
|
|
||
| const logger = new Logger("relationsAndColumnsSystemCatalogQuery"); |
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.
This module doing the system catalog query + parsing into model objs is 100% parallel to how we did it for UDFs + UDF parameters over in src/loaders/utils/udfSystemCatalogQuery.ts
See:
This comment has been minimized.
This comment has been minimized.
shouples
left a comment
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.
Good stuff, works great! Minor test suggestion, and I think moving all of the UDF models into models/flinkSystemCatalog might cause a little dev friction but I get the reasoning
|
|
||
| describe("flinkUDF.ts", () => { | ||
| describe("FlinkUdfParameter", () => { | ||
| describe("formatSqlType", () => { |
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.
This external function / its test now moved over into src/utils/flinkTypes[.test].ts. The function is also used by src/models/flinkSystemCatalog.ts
shouples
left a comment
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.
One more follow-up item: #2937
Summary of Changes
INFORMATION_SCHEMA.TABLES(sic) and....COLUMNSto learn about all of the relations (Kafka-backed tables, views, system tables, external tables) in a Flink database, wrapped by newCCloudResourceLoadermethodgetFlinkRelations().src/models/flinkRelation.ts.Optional: Click-testing instructions
RealworldData.Optional: Any additional details or context that should be provided?
Pull request checklist
Please check if your PR fulfills the following (if applicable):
Tests
Release notes