Add MeshModel summary filter API integration and unit tests#918
Add MeshModel summary filter API integration and unit tests#918Amr-Shams wants to merge 6 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 enhances the MeshModel registry API by introducing new capabilities for retrieving aggregated summary data for components and relationships. It provides a standardized approach to query and summarize registry entities, offering insights into their distribution across various dimensions. This improvement aims to facilitate better data analysis and management within the MeshModel ecosystem. 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
|
There was a problem hiding this comment.
Code Review
This pull request effectively integrates new MeshModel summary filters into the registry API and includes comprehensive unit tests, which is great. The refactoring of error handling into a dedicated package is a good move for code organization. I've provided a couple of suggestions to refactor the new GetSummary methods to reduce code duplication, which will enhance long-term maintainability.
|
Were now using schemas driven development and IMo such struct should be implemented in meshery/schemas |
| return nil | ||
| } | ||
|
|
||
| func (relationshipFilter *RelationshipSummaryFilter) GetSummary(db *database.Handler) (*RelationshipSummary, error) { |
There was a problem hiding this comment.
Where this method will be used ?
There was a problem hiding this comment.
it is a cross repo issue, here you can check the (meshery/meshery#17457)
u mean the new filter ? |
I think all structs created should go there to have a unique source of truth but perhaps it needs to be validated. cc: @leecalcote, @aabidsofi19 any thoughts ? |
|
@lekaf974 absolutely right . All structs models, requests payload, filters , response should be defined and generated using openapi schemas |
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| type relationshipSummaryRecord struct { |
There was a problem hiding this comment.
As mentionned in other comment this struct should be added in meshery/schemas repo
There was a problem hiding this comment.
Yeah fair point, but this one is test-only, not an API struct.
relationshipSummaryRecord is just a small DB seed helper for unit tests.
After switching to generated schema types, using RelationshipDefinition directly in test migrate/seed was failing because of nested ModelReference parsing in GORM.
So this isn’t a new runtime model, and nothing API-related is being defined here.
The actual filter/response shapes are in meshery/schemas.
|
@lekaf974 i have open a pr, and once the schemes is approved i will push the local changes here |
a9591a4 to
4b2d740
Compare
Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
…ount Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
Signed-off-by: Amr-Shams <amr.shams2015.as@gmail.com>
4b2d740 to
ab92b53
Compare
Description
This PR integrates the new MeshModel summary filters into the registry API and adds unit tests for summary aggregation behavior.
This PR fixes #917
Notes for Reviewers
Signed commits