feat: do not fail if --cache is active but no SNAKEMAKE_OUTPUT_CACHE env var is defined. Instead, print a warning that explains the options.#3270
Conversation
…env var is defined. Instead, print a warning that explains the options.
|
Warning Rate limit exceeded@johanneskoester has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe pull request introduces a new exception Changes
Sequence DiagramsequenceDiagram
participant Workflow
participant StorageProvider
participant OutputFileCache
participant ExceptionHandler
Workflow->>StorageProvider: Check storage settings
alt Valid Storage Settings
StorageProvider-->>OutputFileCache: Initialize StorageOutputFileCache
else Invalid Storage Settings
StorageProvider->>ExceptionHandler: Raise MissingOutputFileCachePathException
ExceptionHandler-->>Workflow: Handle missing cache path error
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 (
|
|
Please format your code with black: |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
snakemake/caching/__init__.py (1)
Line range hint
6-15: Refactor imports to satisfy Black formatting and remove unusedloggerimport.According to the pipeline failure logs, the import block requires reformatting to meet Black’s grouping standards. Additionally, static analysis flagged
loggeras unused in this file.Below is a proposed diff to address both concerns, grouping imports consistently and removing
logger:-import os -from abc import ABCMeta, abstractmethod -from snakemake.jobs import Job -from snakemake.io import apply_wildcards -from snakemake.exceptions import MissingOutputFileCachePathException, WorkflowError, CacheMissException -from snakemake.caching.hash import ProvenanceHashMap -from snakemake.logging import logger +import os +from abc import ABCMeta, abstractmethod +from snakemake.caching.hash import ProvenanceHashMap +from snakemake.exceptions import ( + CacheMissException, + MissingOutputFileCachePathException, + WorkflowError, +) +from snakemake.io import apply_wildcards +from snakemake.jobs import Job🧰 Tools
🪛 Ruff (0.8.2)
13-13:
snakemake.logging.loggerimported but unused; consider removing, adding to__all__, or using a redundant alias(F401)
🧹 Nitpick comments (3)
snakemake/caching/__init__.py (1)
25-25: Chain exception viaraise ... from ...for clarity.When re-raising an exception in an
exceptclause, it’s often beneficial to preserve the trace context usingraise ... from err. This helps distinguish errors in exception handling from the original exception:- except KeyError: - raise MissingOutputFileCachePathException() + except KeyError as err: + raise MissingOutputFileCachePathException() from err🧰 Tools
🪛 Ruff (0.8.2)
25-25: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
snakemake/exceptions.py (1)
606-608: Consider adding a docstring for clarity.Although this dedicated exception class is valid as is, adding a short docstring can help future maintainers understand its purpose:
class MissingOutputFileCachePathException(Exception): + """ + Raised when Snakemake attempts to use caching + but no cache path is specified via environment variable. + """ passsnakemake/workflow.py (1)
127-127: Remove or utilize thesnakemake.apiimport.Static analysis indicates that
snakemake.apiis imported but not used. Consider removing it if it’s unnecessary:-from snakemake import api, caching, sourcecache +from snakemake import caching, sourcecache🧰 Tools
🪛 Ruff (0.8.2)
127-127:
snakemake.apiimported but unusedRemove unused import:
snakemake.api(F401)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
snakemake/caching/__init__.py(2 hunks)snakemake/cli.py(2 hunks)snakemake/exceptions.py(1 hunks)snakemake/workflow.py(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- snakemake/cli.py
🧰 Additional context used
📓 Path-based instructions (3)
snakemake/exceptions.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 the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
snakemake/workflow.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 the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
snakemake/caching/__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 the self argument of methods.
Do not suggest type annotation of the cls argument of classmethods.
Do not suggest return type annotation if a function or method does not contain a return statement.
🪛 Ruff (0.8.2)
snakemake/workflow.py
127-127: snakemake.api imported but unused
Remove unused import: snakemake.api
(F401)
snakemake/caching/__init__.py
13-13: snakemake.logging.logger imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
25-25: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
🪛 GitHub Actions: CI
snakemake/caching/__init__.py
[error] 6-15: Code formatting does not meet Black standards. File needs to be reformatted to fix import statement grouping.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: apidocs
🔇 Additional comments (2)
snakemake/workflow.py (2)
57-57: No concerns with the updated exception import.The addition of
MissingOutputFileCachePathExceptioninto the import list is consistent with its usage.
689-707: Implementation aligns with the PR’s core objective.This new try-except block gracefully handles a missing cache path by warning the user instead of failing, matching the PR’s goal to skip failing if
--cacheis enabled withoutSNAKEMAKE_OUTPUT_CACHEset.
|
🤖 I have created a release *beep* *boop* --- ## [8.28.0](v8.27.1...v8.28.0) (2025-02-12) ### Features * do not fail if --cache is active but no SNAKEMAKE_OUTPUT_CACHE env var is defined. Instead, print a warning that explains the options. ([#3270](#3270)) ([9610f7c](9610f7c)) ### Bug Fixes * do not use outdated metadata for rerun triggers (only warn about it) ([#3259](#3259)) ([d766a48](d766a48)) * ensure that exceptions print storage queries instead of local copies of remote files ([#3258](#3258)) ([e5d8ec1](e5d8ec1)) * fix error message of evaluate helper function ([#3282](#3282)) ([9483a64](9483a64)) * Revert cleaning of env vars in apptainer ([#3285](#3285)) ([e79a51b](e79a51b)) ### Performance Improvements * compare checksums of input files <= 1MB (before (10KB) ([#3267](#3267)) ([ba017bb](ba017bb)) * query updated input files in parallel ([#3266](#3266)) ([bc4fcee](bc4fcee)) ### Documentation * Adds instructions for using syntax highlighting with lazy.nvim ([#3246](#3246)) ([7a75043](7a75043)) * Fix typos in basic API example ([#3277](#3277)) ([8782219](8782219)) --- 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>
…env var is defined. Instead, print a warning that explains the options. (snakemake#3270) <!--Add a description of your PR here--> ### QC <!-- Make sure that you can tick the boxes below. --> * [x] The PR contains a test case for the changes or the changes are already covered by an existing test case. * [x] The documentation (`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). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit ## Release Notes - **New Features** - Introduced a new exception `MissingOutputFileCachePathException` for more precise error handling in output file caching - **Documentation** - Updated help text for `--cache` argument to reference the correct environment variable - **Bug Fixes** - Improved error handling for scenarios involving missing output file cache paths - Enhanced workflow robustness when initializing storage cache settings <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [8.28.0](snakemake/snakemake@v8.27.1...v8.28.0) (2025-02-12) ### Features * do not fail if --cache is active but no SNAKEMAKE_OUTPUT_CACHE env var is defined. Instead, print a warning that explains the options. ([snakemake#3270](snakemake#3270)) ([9610f7c](snakemake@9610f7c)) ### Bug Fixes * do not use outdated metadata for rerun triggers (only warn about it) ([snakemake#3259](snakemake#3259)) ([d766a48](snakemake@d766a48)) * ensure that exceptions print storage queries instead of local copies of remote files ([snakemake#3258](snakemake#3258)) ([e5d8ec1](snakemake@e5d8ec1)) * fix error message of evaluate helper function ([snakemake#3282](snakemake#3282)) ([9483a64](snakemake@9483a64)) * Revert cleaning of env vars in apptainer ([snakemake#3285](snakemake#3285)) ([e79a51b](snakemake@e79a51b)) ### Performance Improvements * compare checksums of input files <= 1MB (before (10KB) ([snakemake#3267](snakemake#3267)) ([ba017bb](snakemake@ba017bb)) * query updated input files in parallel ([snakemake#3266](snakemake#3266)) ([bc4fcee](snakemake@bc4fcee)) ### Documentation * Adds instructions for using syntax highlighting with lazy.nvim ([snakemake#3246](snakemake#3246)) ([7a75043](snakemake@7a75043)) * Fix typos in basic API example ([snakemake#3277](snakemake#3277)) ([8782219](snakemake@8782219)) --- 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
Release Notes
New Features
MissingOutputFileCachePathExceptionfor more precise error handling in output file cachingDocumentation
--cacheargument to reference the correct environment variableBug Fixes