feat!: update configuration file #2369
Conversation
42e2558 to
d16b324
Compare
Update source code `Kind` to `Type`. It's only changes within our code. Changes to yaml tag (that will affect users) will be done in later PRs. This is a breaking change since it updates telemetry's span attribute from `source_kind` to `source_type`. Related #817 Future updates will include: * Updating a preprocessing function to convert config file from v1 to v2 * Update unmarshal function for ToolsFile to convert config file (test will fail since the yaml tag is not yet updated). * Update yaml tag (test will pass).
Add preprocessing function during parsing of tools file to convert v2 tools file. Related #817 Future updates will include: * Update unmarshal function for ToolsFile to convert config file (test will fail since the yaml tag is not yet updated). * Update yaml tag (test will pass). --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Averi Kitsch <akitsch@google.com>
Update `parseToolsFile()` to use the file conversion function. Update unmarshalling function since we will be unmarshalling per-doc (separated by `---`). `AuthSources` will be converted to `AuthServices` during the file conversion stage. Hence, removing it from `ToolsFile` struct. ToolsFile v2 will not support `AuthSources`. Double checked that all docs do not reference to `AuthSources` anymore; new users is expected to be using `AuthServices` instead. This PR will not pass the unit test since the updates for resources's yaml tag will be in subsequent PRs. Breaking it down to keep review simpler. Related #817 Upcoming PR: * Update yaml tag
…2348) Update yaml tag in source and auth services from `kind` to `type`. Test will still fail since we have not yet updated tools.
Update yaml tag in tools from `kind` to `type`. Upcoming PRs: * Update config in `/tests` and `/docs` Will resolve other unit tests failure in the final PR to keep things simpler.
Update "kind" to "type" in `/tests` folder.
Update docs and `cmd/root_test.go`.
Fix previously failing test. During unmarshal, if the resources is not presented, return nil instead of empty struct. Also added a note in docs to view the configuration file v1 version.
d16b324 to
8584676
Compare
Summary of ChangesHello @Yuan325, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a breaking change to the Toolbox configuration file format, refactoring it to enhance resource isolation and facilitate atomic, incremental updates. The primary change involves transitioning from a single-document YAML structure with top-level resource keys to a multi-document format where each resource is defined with explicit Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This PR introduces a significant and breaking change to the configuration file format, moving to a more structured v2 format. The changes are extensive, touching documentation, core logic for config parsing, and tests.
The implementation of the v1-to-v2 conversion layer in cmd/root.go is well-handled, including backward compatibility for authSources. The refactoring in internal/server/config.go to a centralized UnmarshalResourceConfig function is a good improvement for handling the new multi-document format.
The documentation has been updated thoroughly across the board to reflect the new format and the kind vs type terminology change. The tests have also been updated to cover the new format and conversion logic.
I have one minor suggestion for a typo in the documentation. Overall, this is a solid and well-executed refactoring.
| type: postgres-list-triggers | ||
| source: postgres-source | ||
| description: | | ||
| Lists all non-internal triggers in a database. Returns trigger name, schema name, table name, wether its enabled or disabled, timing (e.g BEFORE/AFTER of the event), the events that cause the trigger to fire such as INSERT, UPDATE, or DELETE, whether the trigger activates per ROW or per STATEMENT, the handler function executed by the trigger and full definition. |
There was a problem hiding this comment.
There's a typo in the description. "wether" should be "whether".
| Lists all non-internal triggers in a database. Returns trigger name, schema name, table name, wether its enabled or disabled, timing (e.g BEFORE/AFTER of the event), the events that cause the trigger to fire such as INSERT, UPDATE, or DELETE, whether the trigger activates per ROW or per STATEMENT, the handler function executed by the trigger and full definition. | |
| Lists all non-internal triggers in a database. Returns trigger name, schema name, table name, whether its enabled or disabled, timing (e.g BEFORE/AFTER of the event), the events that cause the trigger to fire such as INSERT, UPDATE, or DELETE, whether the trigger activates per ROW or per STATEMENT, the handler function executed by the trigger and full definition. |
This PR introduces a significant update to the Toolbox configuration file format, which is one of the primary breaking changes required for the implementation of the Advanced Control Plane.
Summary of Changes
The configuration schema has been updated to enforce resource isolation and facilitate atomic, incremental updates.
User Impact
Existing tools.yaml configuration files are now in an outdated format. Users must eventually update their files to the new YAML format.
Mitigation & Compatibility
Backward compatibility is maintained during this transition to ensure no immediate user action is required for existing files.