fix: skip asset download if files are already present#3076
fix: skip asset download if files are already present#3076johanneskoester merged 1 commit intomainfrom
Conversation
WalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant YourClassName
participant FileSystem
User->>YourClassName: Call deploy()
YourClassName->>FileSystem: Check if target_path exists
alt If exists
YourClassName->>FileSystem: Read existing file
YourClassName->>YourClassName: Compute SHA-256 hash of existing file
YourClassName->>YourClassName: Compute SHA-256 hash of new asset
alt If hashes match
YourClassName-->>User: Skip writing asset
else
YourClassName->>FileSystem: Write new asset
end
else
YourClassName->>FileSystem: Write new asset
end
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 (
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
snakemake/assets/__init__.py (1)
121-127: Optimization of asset deployment is effective.The addition of a conditional check to skip the download if the asset file already exists and is up to date is a significant improvement. This change not only saves bandwidth but also reduces unnecessary disk writes, aligning well with the PR's objectives.
However, consider adding a log statement before continuing in line 126 to inform about the skipped download, which could be helpful for debugging purposes.
Consider adding a log statement for better traceability:
+ from snakemake.logging import logger if asset.sha256 == hashlib.sha256(fin.read()).hexdigest(): + logger.info(f"Skipping download for {asset_path} as it is up to date.") continue
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- snakemake/assets/init.py (1 hunks)
Additional context used
Path-based instructions (1)
snakemake/assets/__init__.py (1)
Pattern
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
🤖 I have created a release *beep* *boop* --- ## [8.20.0](v8.19.3...v8.20.0) (2024-09-07) ### Features * allow skipping apptainer and conda tests if not available ([#3074](#3074)) ([c3cd4e0](c3cd4e0)) * MANIFEST.in: include tests/ in sdist ([#3073](#3073)) ([e24f3fc](e24f3fc)) ### Bug Fixes * add subfolders of report template to package data ([14a8f22](14a8f22)) * Inconsistent Linting Output Formatting ([#3064](#3064)) ([90e51ae](90e51ae)) * retry when downloading assets ([58e41b0](58e41b0)) * skip asset download if files are already present ([#3076](#3076)) ([0d3d1e1](0d3d1e1)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
QC
docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).Summary by CodeRabbit
New Features
Bug Fixes