- Notifications
You must be signed in to change notification settings - Fork 842
feat(inspect): exported model size #3126
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
feat(inspect): exported model size #3126
Conversation
…ates Signed-off-by: Dmitry Kalinin <dmitry.kalinin@intel.com>
Signed-off-by: Dmitry Kalinin <dmitry.kalinin@intel.com>
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 adds an exported model size attribute to track the total size in bytes of exported model artifacts. The implementation computes the size after model export and stores it in the database.
Key changes:
- Added a
sizefield to the Model schema to store the total size in bytes of exported model artifacts - Implemented
_compute_export_sizemethod to calculate total size of files/directories in the export path - Created a database migration to add the
sizecolumn to the models table
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| application/backend/src/pydantic_models/model.py | Added size field to Model schema with validation |
| application/backend/src/db/schema.py | Added size column to ModelDB table using BigInteger |
| application/backend/src/services/training_service.py | Implemented _compute_export_size method and integrated size computation into training flow |
| application/backend/src/alembic/versions/7bbe7699b43a_add_model_size_column.py | Database migration to add size column |
| application/backend/tests/unit/services/test_training_service.py | Updated test to mock and verify size computation |
| application/backend/tests/unit/endpoints/test_models.py | Added size field to fixture and verified in API response |
| application/backend/tests/unit/conftest.py | Added size field to model fixture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Dmitry Kalinin <dmitry.kalinin@intel.com>
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 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
maxxgx left a comment
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.
Minor comments. Looks good, thanks!
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.
It's great that you are adding the schema migration script, but since we have not released any version yet, it does not make sense to have a migration at this stage. Rather, you can add the size column to the initial schema 7a213a27d666_initial_schema.py
Signed-off-by: Dmitry Kalinin <dmitry.kalinin@intel.com>
0b08527 into open-edge-platform:feature/geti-inspect
📝 Description
Added exported model size attribute
✨ Changes
Select what type of change your PR is:
✅ Checklist
Before you submit your pull request, please make sure you have completed the following steps:
For more information about code review checklists, see the Code Review Checklist.