feat(schemas): add MeshModel component and relationship summary filter schemas#616
feat(schemas): add MeshModel component and relationship summary filter schemas#616Amr-Shams wants to merge 2 commits intomeshery:masterfrom
Conversation
Summary of ChangesHello, 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 MeshModel API by introducing dedicated schemas for summarizing and filtering components and relationships. By adopting a schema-first approach, it ensures consistency and facilitates automated type generation for downstream consumers. Additionally, it improves the robustness of the Go codebase by adding explicit validation methods for numerous enum types. Highlights
Changelog
Activity
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 introduces new schemas for component and relationship summaries, and subsequently regenerates the corresponding Go and TypeScript models. The changes are predominantly auto-generated. My review has identified a recurring issue with code duplication in the generated ...Summary structs across both Go and TypeScript. Specifically, anonymous structs are being generated instead of reusing the already defined ...GroupEntry structs. This impacts the maintainability of the code. The review comments below provide specific suggestions to rectify this, which will likely necessitate adjustments in the code generation process or the source schema definitions.
| type RelationshipSummary struct { | ||
| ByKind *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byKind,omitempty" yaml:"byKind,omitempty"` | ||
| ByModel *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byModel,omitempty" yaml:"byModel,omitempty"` | ||
| BySubType *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"bySubType,omitempty" yaml:"bySubType,omitempty"` | ||
| ByType *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byType,omitempty" yaml:"byType,omitempty"` | ||
| Total int64 `json:"total" yaml:"total"` | ||
| } |
There was a problem hiding this comment.
The anonymous structs used for ByKind, ByModel, BySubType, and ByType are identical to the RelationshipGroupEntry struct. To improve code reuse and maintainability, consider using the named RelationshipGroupEntry struct here. This would make the RelationshipSummary struct definition cleaner and avoid code duplication.
type RelationshipSummary struct {
ByKind *[]RelationshipGroupEntry `json:"byKind,omitempty" yaml:"byKind,omitempty"`
ByModel *[]RelationshipGroupEntry `json:"byModel,omitempty" yaml:"byModel,omitempty"`
BySubType *[]RelationshipGroupEntry `json:"bySubType,omitempty" yaml:"bySubType,omitempty"`
ByType *[]RelationshipGroupEntry `json:"byType,omitempty" yaml:"byType,omitempty"`
Total int64 `json:"total" yaml:"total"`
}| // ComponentSummary defines model for ComponentSummary. | ||
| type ComponentSummary struct { | ||
| ByCategory *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byCategory,omitempty" yaml:"byCategory,omitempty"` | ||
| ByModel *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byModel,omitempty" yaml:"byModel,omitempty"` | ||
| ByRegistrant *[]struct { | ||
| Count int32 `json:"count" yaml:"count"` | ||
| Key string `json:"key" yaml:"key"` | ||
| } `json:"byRegistrant,omitempty" yaml:"byRegistrant,omitempty"` | ||
| Total int64 `json:"total" yaml:"total"` | ||
| } |
There was a problem hiding this comment.
The anonymous structs used for ByCategory, ByModel, and ByRegistrant are identical to the ComponentGroupEntry struct. To improve code reuse and maintainability, consider using the named ComponentGroupEntry struct here. This would make the ComponentSummary struct definition cleaner and avoid code duplication.
type ComponentSummary struct {
ByCategory *[]ComponentGroupEntry `json:"byCategory,omitempty" yaml:"byCategory,omitempty"`
ByModel *[]ComponentGroupEntry `json:"byModel,omitempty" yaml:"byModel,omitempty"`
ByRegistrant *[]ComponentGroupEntry `json:"byRegistrant,omitempty" yaml:"byRegistrant,omitempty"`
Total int64 `json:"total" yaml:"total"`
}| RelationshipSummary: { | ||
| /** Format: int64 */ | ||
| total: number; | ||
| byModel?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| byKind?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| byType?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| bySubType?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| }; |
There was a problem hiding this comment.
The inline type definitions for byModel, byKind, byType, and bySubType are identical to RelationshipGroupEntry. To improve type safety and maintainability, you should reuse the RelationshipGroupEntry type here.
RelationshipSummary: {
/** Format: int64 */
total: number;
byModel?: components["schemas"]["RelationshipGroupEntry"][];
byKind?: components["schemas"]["RelationshipGroupEntry"][];
byType?: components["schemas"]["RelationshipGroupEntry"][];
bySubType?: components["schemas"]["RelationshipGroupEntry"][];
};| ComponentSummary: { | ||
| /** Format: int64 */ | ||
| total: number; | ||
| byModel?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| byCategory?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| byRegistrant?: { | ||
| key: string; | ||
| /** Format: int32 */ | ||
| count: number; | ||
| }[]; | ||
| }; |
There was a problem hiding this comment.
The inline type definitions for byModel, byCategory, and byRegistrant are identical to ComponentGroupEntry. To improve type safety and maintainability, you should reuse the ComponentGroupEntry type here.
ComponentSummary: {
/** Format: int64 */
total: number;
byModel?: components["schemas"]["ComponentGroupEntry"][];
byCategory?: components["schemas"]["ComponentGroupEntry"][];
byRegistrant?: components["schemas"]["ComponentGroupEntry"][];
};
|
Good work! You might like to present your work in the upcoming, weekly Meshery development meeting (meshery.io/calendar). If so, I encourage you to add this item in the meeting agenda (meet.meshery.io/dev-minutes), attend, present, and receive feedback. |
|
@Amr-Shams merge conflict need to be addressed. |
well do my best there, thanks 🥇 |
…r schemas Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
Amr-Shams
force-pushed
the
feat/meshmodel-summary-filter-schemas
branch
from
4c40070 to
39e5453
Compare
2 participants
Description
Adds schema-first API shapes for MeshModel summary filters and summary responses.
Added schemas
Enums
by_model,by_category,by_registrantby_model,by_kind,by_type,by_subtypeWhy
Moves filter/request/response types to schemas as the source of truth so downstream repos consume generated types.
Notes for Reviewers
This PR fixes #615
Signed commits