fix: correctly handle meta-wrapper tag replacement depending on the used snakemake-wrapper release#3826
Conversation
📝 WalkthroughWalkthroughModified wrapper tag handling in Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller
participant modules_py as modules.get_wrapper_tag
Note over modules_py: input meta_wrapper (string)
Caller->>modules_py: call get_wrapper_tag(meta_wrapper)
alt meta_wrapper is a URL
modules_py-->>Caller: return None
else meta_wrapper is not a URL
modules_py->>modules_py: extract first path segment as tag
alt tag parses as version >= 8.0.0
modules_py-->>Caller: return None
else
modules_py-->>Caller: proceed with existing tag parsing/return
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py⚙️ CodeRabbit configuration file
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2024-10-06T14:09:54.370ZApplied to files:
📚 Learning: 2024-10-06T14:09:26.494ZApplied to files:
📚 Learning: 2024-11-12T12:08:20.342ZApplied to files:
🧬 Code graph analysis (1)src/snakemake/modules.py (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (44)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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 (2)
src/snakemake/modules.py(1 hunks)src/snakemake/wrapper.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
⚙️ CodeRabbit configuration file
**/*.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.
Files:
src/snakemake/modules.pysrc/snakemake/wrapper.py
**/wrapper.py
⚙️ CodeRabbit configuration file
Do not complain about use of undefined variable called
snakemake.
Files:
src/snakemake/wrapper.py
🧠 Learnings (2)
📚 Learning: 2024-10-06T14:09:54.370Z
Learnt from: johanneskoester
Repo: snakemake/snakemake PR: 3117
File: tests/test_wrapper/Snakefile:11-11
Timestamp: 2024-10-06T14:09:54.370Z
Learning: Changes made within test cases, such as in `tests/test_wrapper/Snakefile`, are for testing purposes and do not require updates to the project documentation.
Applied to files:
src/snakemake/modules.pysrc/snakemake/wrapper.py
📚 Learning: 2024-10-06T14:09:26.494Z
Learnt from: johanneskoester
Repo: snakemake/snakemake PR: 3117
File: snakemake/deployment/conda.py:0-0
Timestamp: 2024-10-06T14:09:26.494Z
Learning: In `snakemake/deployment/conda.py`, the exception handling in the `_get_version` method is intentional. The method raises a `WorkflowError` when the version cannot be determined, and this behavior is desired. Do not suggest modifying this exception handling.
Applied to files:
src/snakemake/modules.py
🧬 Code graph analysis (1)
src/snakemake/modules.py (1)
src/snakemake/wrapper.py (1)
is_url(50-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (44)
- GitHub Check: tests (10, ubuntu-latest, py311)
- GitHub Check: tests (10, ubuntu-latest, py312)
- GitHub Check: tests (8, macos-latest, py313)
- GitHub Check: tests (6, ubuntu-latest, py313)
- GitHub Check: tests (10, ubuntu-latest, py313)
- GitHub Check: tests (10, windows-2022, py313)
- GitHub Check: tests (9, windows-2022, py313)
- GitHub Check: tests (9, ubuntu-latest, py313)
- GitHub Check: tests (9, ubuntu-latest, py312)
- GitHub Check: tests (8, ubuntu-latest, py312)
- GitHub Check: tests (8, ubuntu-latest, py313)
- GitHub Check: tests (8, windows-2022, py313)
- GitHub Check: tests (9, ubuntu-latest, py311)
- GitHub Check: tests (7, ubuntu-latest, py313)
- GitHub Check: tests (7, ubuntu-latest, py311)
- GitHub Check: tests (5, ubuntu-latest, py313)
- GitHub Check: tests (7, windows-2022, py313)
- GitHub Check: tests (7, ubuntu-latest, py312)
- GitHub Check: tests (6, windows-2022, py313)
- GitHub Check: tests (6, ubuntu-latest, py311)
- GitHub Check: tests (8, ubuntu-latest, py311)
- GitHub Check: tests (6, ubuntu-latest, py312)
- GitHub Check: tests (5, windows-2022, py313)
- GitHub Check: tests (2, ubuntu-latest, py313)
- GitHub Check: tests (5, ubuntu-latest, py311)
- GitHub Check: tests (4, ubuntu-latest, py313)
- GitHub Check: tests (5, ubuntu-latest, py312)
- GitHub Check: tests (4, windows-2022, py313)
- GitHub Check: tests (4, macos-latest, py313)
- GitHub Check: tests (4, ubuntu-latest, py312)
- GitHub Check: tests (3, ubuntu-latest, py312)
- GitHub Check: tests (3, windows-2022, py313)
- GitHub Check: tests (3, ubuntu-latest, py313)
- GitHub Check: tests (4, ubuntu-latest, py311)
- GitHub Check: tests (1, ubuntu-latest, py311)
- GitHub Check: tests (3, ubuntu-latest, py311)
- GitHub Check: tests (1, ubuntu-latest, py312)
- GitHub Check: tests (1, macos-latest, py313)
- GitHub Check: tests (2, windows-2022, py313)
- GitHub Check: tests (1, ubuntu-latest, py313)
- GitHub Check: tests (1, windows-2022, py313)
- GitHub Check: tests (2, ubuntu-latest, py312)
- GitHub Check: tests (2, ubuntu-latest, py311)
- GitHub Check: apidocs
🔇 Additional comments (1)
src/snakemake/wrapper.py (1)
17-17: LGTM! Named group enables version extraction in modules.py.The addition of the named capturing group
(?P<ver>...)maintains backward compatibility while enablingmodules.pyto extract and compare version numbers. The pattern correctly excludes the optional "v" prefix from the captured group, which is appropriate for parsing withpackaging.version.Version.
🤖 I have created a release *beep* *boop* --- ## [9.13.5](v9.13.4...v9.13.5) (2025-11-04) ### Bug Fixes * cache wrapper files and wait for them in case of shared filesystem for sources ([#3809](#3809)) ([498fff7](498fff7)) * correctly handle meta-wrapper tag replacement depending on the used snakemake-wrapper release ([#3826](#3826)) ([8d46a4a](8d46a4a)) * ensure that flags are properly considered for input files before applying path modifiers (i.e. default storage providers) ([#3813](#3813)) ([d3bfe32](d3bfe32)) * ensure that tokens are not leaked when paths or uris of source files are logged ([#3821](#3821)) ([a217e50](a217e50)) * print secs as numeric in jsonl benchmark format ([#3814](#3814)) ([395a5e6](395a5e6)) * revert breaking change in 9.11.9 disallowing empty input files even when unused ([#3810](#3810)) ([83c05cc](83c05cc)) * shorten report ids (thus dir names) in order to avoid issues with path length on windows ([#3822](#3822)) ([b24d971](b24d971)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
…sed snakemake-wrapper release (snakemake#3826) ### Description <!--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 * **Bug Fixes** * Stop erroring when a wrapper reference is a URL; such references are now ignored instead of causing failures. * **Refactor** * Improved wrapper-tag handling: newer-style wrappers (version 8.0.0+) and certain tag formats are now treated as no-op for tag replacement, improving compatibility with recent wrapper versions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
🤖 I have created a release *beep* *boop* --- ## [9.13.5](snakemake/snakemake@v9.13.4...v9.13.5) (2025-11-04) ### Bug Fixes * cache wrapper files and wait for them in case of shared filesystem for sources ([snakemake#3809](snakemake#3809)) ([498fff7](snakemake@498fff7)) * correctly handle meta-wrapper tag replacement depending on the used snakemake-wrapper release ([snakemake#3826](snakemake#3826)) ([8d46a4a](snakemake@8d46a4a)) * ensure that flags are properly considered for input files before applying path modifiers (i.e. default storage providers) ([snakemake#3813](snakemake#3813)) ([d3bfe32](snakemake@d3bfe32)) * ensure that tokens are not leaked when paths or uris of source files are logged ([snakemake#3821](snakemake#3821)) ([a217e50](snakemake@a217e50)) * print secs as numeric in jsonl benchmark format ([snakemake#3814](snakemake#3814)) ([395a5e6](snakemake@395a5e6)) * revert breaking change in 9.11.9 disallowing empty input files even when unused ([snakemake#3810](snakemake#3810)) ([83c05cc](snakemake@83c05cc)) * shorten report ids (thus dir names) in order to avoid issues with path length on windows ([snakemake#3822](snakemake#3822)) ([b24d971](snakemake@b24d971)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Description
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
Bug Fixes
Refactor