Model Generation : Added Support mapping of group field in CRDs #815
Conversation
|
Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack. |
Summary of ChangesHello @CWAbhi, 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 significantly enhances the system's ability to process and categorize Kubernetes Custom Resource Definitions (CRDs) by correctly interpreting their API group information. The core change involves updating the CRD parsing logic to leverage the 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the model generation process by incorporating the group field from CRDs to improve the association of custom resources with their API groups. The changes include updates to CRD parsing logic, component-to-model mapping, and compatibility with both native and custom Kubernetes resources. The changes look good overall, and I have provided a few comments to address potential issues.
| comp.Model.Name = pkg.Name | ||
|
|
||
| // Derive model from the CRD's API group when available; otherwise, fallback to package name | ||
| group := component.ExtractGroupFromAPIVersion(comp.Component.Version) |
There was a problem hiding this comment.
It would be beneficial to add a comment explaining the purpose of extracting the group from the API version here. This will improve the code's readability and maintainability.
| comp.Model.Name = gp.Name | ||
|
|
||
| // Derive model from the CRD's API group when available; otherwise, fallback to package name | ||
| group := component.ExtractGroupFromAPIVersion(comp.Component.Version) |
There was a problem hiding this comment.
It would be beneficial to add a comment explaining the purpose of extracting the group from the API version here. This will improve the code's readability and maintainability.
utils/component/generator_test.go
Outdated
| t.Fatalf("expected fallback model name 'fooBarBaz', got '%s'", name) | ||
| } | ||
| if display != "Foo Bar Baz" { | ||
| t.Fatalf("expected display name 'Foo Bar Baz', got '%s'", display) |
utils/component/generator_test.go
Outdated
| } | ||
| } | ||
|
|
||
| func TestGroupToModel_WithGroup(t *testing.T) { |
There was a problem hiding this comment.
We generally arrange tests which test the same functionality with different cases inside one test function, f.e. TestGroupModel, and test cases are called inside using t.Run, f.e. TestGenerate in the same generator_test.go file.
|
@n2h9 I will look into it |
Signed-off-by: Abhijeet Dey <[email protected]>
Signed-off-by: Abhijeet Dey <[email protected]>
|
@n2h9 Please run the test |
|
@Darshan174 , @aabidsofi19 , @winkletinkle Can you guys review this??? |
|
@leecalcote everything looks good to me , what are your thoughts about this? |
2 participants
Description
This PR fixes #731
Current Behaviour
Previously, the
groupfield was treated as a regular property instead of a key identifier for associating custom resources with their API groups.Enhancement
•Updated CRD parsing logic to recognize and use the group field.
•Implemented component-to-model mapping based on the API group value.
•Ensured compatibility with both native and custom Kubernetes resources.
•Improved maintainability by centralizing group-based mapping logic.
Testing
•Validated functionality with multiple CRDs, ensuring:
•Accurate mapping across different API groups.
•No regression in existing CRD handling.
•Successful parsing and model association for both standard and custom resources.