-
Notifications
You must be signed in to change notification settings - Fork 425
feat: enhanced extension #7227
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
base: main
Are you sure you want to change the base?
feat: enhanced extension #7227
Conversation
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 an enhanced extension mechanism to support enterprise features in a more flexible way. The changes replace the previous plugin-based approach with a factory-based extension system that allows for better separation between open-source and enterprise code.
- Adds a new
Extensiontype parameterization toGreptimeOptionsfor custom extension support - Replaces plugin-based trigger DDL manager initialization with a factory-based approach
- Removes enterprise feature flag requirements from catalog information schema tables
Reviewed Changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/cmd/src/options.rs | Adds generic extension parameter E to GreptimeOptions and introduces EmptyOptions for non-enterprise builds |
| src/cmd/src/extension/*.rs | New extension module with factory traits for standalone, frontend, and metasrv components |
| src/meta-srv/src/metasrv/builder.rs | Updates builder to accept and use extension for trigger DDL manager creation |
| src/meta-srv/src/bootstrap/*.rs | Adds extension module and updates metasrv_builder signature to accept extension |
| src/common/meta/src/ddl_manager.rs | Renames method from with_trigger_ddl_manager to with_trigger_ddl_manager_opt |
| src/cmd/src/standalone.rs | Updates to use extension-based factory for trigger DDL manager and information schema tables |
| src/cmd/src/frontend.rs | Updates to use extension-based factory for information schema tables |
| src/cmd/src/metasrv.rs | Adds extension parameter to build methods |
| src/cmd/src/flownode.rs | Removes enterprise Components struct (no longer needed) |
| src/cmd/src/datanode.rs | Adds extension generic parameter to types and methods |
| src/catalog/src/*.rs | Removes enterprise feature flags from information schema table factory support |
| src/cmd/tests/load_config_test.rs | Updates test code to use EmptyOptions generic parameter |
| src/cmd/Cargo.toml | Removes catalog/enterprise from enterprise feature dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Copilot reviewed 25 out of 26 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a832fba to
bd8b0b9
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.
Pull Request Overview
Copilot reviewed 26 out of 28 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Seems "extension" is functionally overlapping with existed "plugins". Unless there are some hard problems that current plugins can't resolve while extension can, I'm inclined to optimize plugins instead. WDYT @sunng87 @fengjiachun |
|
Yes, is there any chance we can reuse plugin mechanism, or just use wrapper in downstream repos for this? |
70e544c to
a84164d
Compare
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
What's changed and what's your intention?
This pr mainly change
INFORMATION_SCHEMA_ALERTS_TABLE_ID.#[cfg(feature = "enterprise")]fromInformationSchemaTableFactoryrelated.extensionfield forGreptimeOptionsstruct which stores some configuration related to extensions. The usage detail seetest_read_config_file_with_extension.Extensionstruct for flownode, frontend, standalone etc, which make extensions strongly typed.componentsfield of the Flownode and Standalone instances, which breaks encapsulation.PR Checklist
Please convert it to a draft if some of the following conditions are not met.