-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Snakemake upgrade #471
base: master
Are you sure you want to change the base?
Snakemake upgrade #471
Conversation
a4ab1b0
to
20a01a4
Compare
20a01a4
to
46bd3b6
Compare
46bd3b6
to
c243ae1
Compare
c243ae1
to
88cc0dd
Compare
reana_commons/snakemake.py
Outdated
) as snakemake_api: | ||
try: | ||
workflow_api = snakemake_api.workflow( | ||
resource_settings=ResourceSettings(nodes=300), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the nodes settings, we already have SNAKEMAKE_MAX_PARALLEL_JOBS
environmental variable that could be used instead of the hard-coded value of 300, even though this might "fully' work only when the validation would be moved to the server-side.
Still, you could preventively introduce in reana_commons/config.py
something like:
SNAKEMAKE_MAX_PARALLEL_JOBS = int(os.getenv("SNAKEMAKE_MAX_PARALLEL_JOBS", "300"))
"""Snakemake maximum number of jobs that can run in parallel."""
which might be better that hard-coding the value of 300.
(Similarly as we do in reana_workflow_engine_snakemake/config.py
.)
setup.py
Outdated
# see https://github.com/snakemake/snakemake/issues/2657 | ||
"snakemake @ git+https://github.com/mdonadoni/snakemake.git@cea31624976989ad0645eb2e1751260d32259506", # branch `7.32.4-python3.12` | ||
"pulp>=2.7.0,<2.8.0", | ||
"snakemake==8.23.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already Snakemake 8.24.0 out.
reana_commons/snakemake.py
Outdated
workdir=workdir, | ||
) | ||
# Try to create dag and it throws an error in case of a failure. | ||
# Seems to be enough but might not be an extensive validation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps amend the docstring to:
Seems to be enough for the first validation. We may move to snakemake --dry-run
when the validation process will be fully moved to the server-side.
88cc0dd
to
dfa4283
Compare
dfa4283
to
804e084
Compare
804e084
to
e23bba8
Compare
6ab9917
to
5d54f47
Compare
5d54f47
to
939f501
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #471 +/- ##
==========================================
- Coverage 44.23% 41.79% -2.45%
==========================================
Files 28 28
Lines 1978 2010 +32
==========================================
- Hits 875 840 -35
- Misses 1103 1170 +67
|
939f501
to
150afd2
Compare
No description provided.