on import from cloud, save resource to disk directly #8930
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes focus on modifying the file downloading mechanism in the cloud storage classes within the CVAT engine. The primary modification involves replacing the Changes
Poem
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
cvat/apps/engine/cloud_provider.py(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: isort
cvat/apps/engine/cloud_provider.py
[error] 13-16: Imports are incorrectly sorted. The import statement 'from typing import Any, Callable, Optional, TypeVar, BinaryIO' needs to be reordered to 'from typing import Any, BinaryIO, Callable, Optional, TypeVar'
🪛 GitHub Actions: Pylint
cvat/apps/engine/cloud_provider.py
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: build
- GitHub Check: testing
- GitHub Check: generate_github_pages
- GitHub Check: Analyze (python)
- GitHub Check: Linter
- GitHub Check: Analyze (javascript)
🔇 Additional comments (7)
cvat/apps/engine/cloud_provider.py (7)
170-170: LGTM! Good abstraction design.The new abstract method provides a clear contract for subclasses to implement direct streaming functionality.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
173-179: LGTM! Good backward compatibility.The reimplemented download_fileobj maintains backward compatibility while utilizing the new streaming functionality.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
180-187: LGTM! Robust error handling.The download_file implementation includes proper directory creation and cleanup on failure.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
567-572: LGTM! Efficient implementation using boto3.The implementation correctly uses boto3's download_fileobj with proper transfer configuration.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
768-774: LGTM! Efficient implementation using Azure SDK.The implementation correctly uses Azure's download_blob with proper concurrency control.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
878-880: LGTM! Efficient implementation using Google Cloud Storage SDK.The implementation correctly uses GCS's download_blob_to_file method.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
1023-1023: LGTM! Memory-efficient implementation.The function now uses download_file to save directly to disk, avoiding unnecessary memory usage.
🧰 Tools
🪛 GitHub Actions: Pylint
[error] Pylint analysis failed due to incompatibility with Python type aliases. The error occurs because the version of pylint (2.14.5) does not support 'typealias' syntax, which is present in the file.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8930 +/- ##
===========================================
+ Coverage 73.82% 73.86% +0.03%
===========================================
Files 412 412
Lines 44262 44258 -4
Branches 3993 3993
===========================================
+ Hits 32678 32689 +11
+ Misses 11584 11569 -15
|
…s not supported by pylint 2.14.5
# Conflicts: # .github/workflows/pylint.yml
|
Motivation and context
when importing backups or datasets from a cloud, they first are loaded to memory and only then are saved to the disk.
If a dataset or backup is larger then available RAM, the worker terminates
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit