Add size in bytes to backups #5473
Conversation
📝 WalkthroughWalkthroughThe changes introduce a new attribute, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant BackupManager
User->>API: Request backup info
API->>BackupManager: Fetch backup details
BackupManager-->>API: Return backup details with ATTR_SIZE_BYTES
API-->>User: Respond with backup info including size in bytes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
supervisor/backups/backup.py (2)
217-220: Improve size calculation with cachingThe size property has been optimized using
@cached_propertyand now derives its value fromsize_bytes. This change:
- Improves performance by caching the calculated size
- Maintains consistency by using the new
size_bytesproperty as the source of truthConsider adding a docstring to clarify that the size is in megabytes:
@cached_property def size(self) -> float: - """Return backup size.""" + """Return backup size in megabytes.""" return round(self.size_bytes / 1048576, 2) # calc mbyte
217-227: Consider documenting the size handling changesThe introduction of
size_bytesalongside the existingsizeproperty represents an architectural improvement that:
- Provides more precise size information through raw bytes
- Maintains backward compatibility for existing consumers
- Improves performance through caching
Consider documenting these changes in the API documentation to help consumers understand the available size representations and their use cases.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
supervisor/api/backups.py(3 hunks)supervisor/api/const.py(1 hunks)supervisor/backups/backup.py(2 hunks)tests/api/test_backups.py(2 hunks)tests/conftest.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/conftest.py (6)
Pattern */**(html|markdown|md): - For instructional content in documentation, use a direct and authoritative tone. Avoid expressions of politeness such as 'may' or 'please', and ensure the goal of the instruction is fronted.
- Apply the Microsoft Style Guide to ensure documentation maintains clarity and conciseness.
- In step-by-step instructions, front the location phrase in the instructional sentence.
- In step-by-step instructions, front the 'goal' in the instructional sentence.
- In step-by-step instructions, if in doubt what to front, front the 'goal' before the location phrase in the instructional sentence.
- do not hyphenate terms like 'top-right' or 'bottom-left' with 'corner'
Pattern */**(html|markdown|md): - Use bold to mark UI strings.
- If "" are used to mark UI strings, replace them by bold.
Pattern */**(html|markdown|md): - Be brief in your replies and don't add fluff like "thank you for..." and "Please let me know if"
Pattern */**(html|markdown|md): - Use sentence-style capitalization also in headings.
Pattern */**(html|markdown|md): do not comment on HTML used for icons
Pattern */**(html|markdown|md): Avoid flagging inline HTML for embedding videos in future reviews for this repository.
🔇 Additional comments (12)
supervisor/api/const.py (1)
62-62: LGTM: New constant follows conventions
The new constant ATTR_SIZE_BYTES follows the existing naming pattern and is correctly placed in alphabetical order.
supervisor/api/backups.py (3)
57-57: LGTM: Import added to appropriate group
The new constant is correctly imported with other related constants.
149-149: LGTM: Size in bytes added to list response
The raw size in bytes is now included in the backup list response, meeting the PR objective.
221-221: LGTM: Size in bytes added to backup info response
The raw size in bytes is now included in the detailed backup info response, maintaining consistency with the list endpoint.
tests/api/test_backups.py (3)
42-43: LGTM: Size assertions added to info test
The test now properly validates both the formatted size (0.01) and raw size in bytes (10240).
46-66: LGTM: Comprehensive test for backup info endpoint
New test thoroughly validates the backup info endpoint response, including both size representations.
81-82: LGTM: Size assertions added to list test
The test now properly validates both size formats in the list endpoint response.
tests/conftest.py (3)
33-33: LGTM: Import added for new exclude database feature
The addition of ATTR_EXCLUDE_DATABASE to imports is consistent with the new feature for excluding databases from backups.
Line range hint 609-625: LGTM: Consistent mock implementation
The changes to mock_partial_backup mirror those in mock_full_backup, maintaining consistency across test fixtures.
Line range hint 584-600: Verify mock backup file extension
The mock backup file path has been updated to include the .tar extension, which is more accurate. The ATTR_EXCLUDE_DATABASE flag has been added with a default value of False, which is consistent with the feature's implementation.
✅ Verification successful
Mock backup file extension is consistently using .tar
Based on the search results, all mock backup initializations in the test files are using the .tar extension consistently. The changes in the review are aligned with the existing codebase patterns, where mock backups are created with test_backup.tar as the filename.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all backup file paths in tests use .tar extension
# Test: Search for backup file paths in test files
rg -A 2 'Path\(.*test_backup[^.]' tests/
Length of output: 42
Script:
#!/bin/bash
# Description: Search for backup file paths in test files with a broader pattern
# to ensure we catch all variations of backup path construction
# Search for backup path patterns in test files
rg -A 2 'Path.*backup' tests/
# Also search for mock backup initialization patterns
rg -A 2 'Backup\(.*Path' tests/
Length of output: 2244
supervisor/backups/backup.py (2)
9-9: LGTM: Import for performance optimization
The addition of cached_property from functools is appropriate for optimizing frequently accessed properties.
222-227: LGTM: New size_bytes property implementation
The new size_bytes property is well-implemented with:
- Proper error handling for non-existent files
- Efficient use of
stat()for size retrieval - Clear return type annotation
Proposed change
Add raw size in bytes to backup info returned from API rather then a float from converting that to mb. The API should return the raw value, converting to larger units should be done in places that value is displayed to humans.
Type of change
Additional information
Checklist
ruff format supervisor tests)If API endpoints or add-on configuration are added/changed:
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests
Chores