Skip to content
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.

Sign up for GitHub

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

fix(snakemake): update to v7.32.4 to make Python 3.11 clients compatible #435

Merged
merged 1 commit into from
Feb 1, 2024
Merged

fix(snakemake): update to v7.32.4 to make Python 3.11 clients compatible #435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion reana_commons/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ def default_workspace():
REANA_WORKFLOW_ENGINES = ["yadage", "cwl", "serial", "snakemake"]
"""Available workflow engines."""

REANA_DEFAULT_SNAKEMAKE_ENV_IMAGE = "docker.io/snakemake/snakemake:v6.8.0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should rather be a feature, not a fix. While it's technically true that we are fixing things for py311 clients, we are also upgrading Snakemake to a new major version, which might affect all clients for all other Python versions. So we should rather bring attention to that. I would suggest putting something like feat(snakemake): upgrade to Snakemake 7.32.4 for the release announcement headline.

REANA_DEFAULT_SNAKEMAKE_ENV_IMAGE = "docker.io/snakemake/snakemake:v7.32.4"
"""Snakemake default job environment image."""

REANA_JOB_CONTROLLER_CONNECTION_CHECK_SLEEP = float(
Expand Down
24 changes: 16 additions & 8 deletions reana_commons/snakemake.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def _create_snakemake_dag(
if workdir:
workflow.workdir(workdir)

workflow.include(snakefile=snakefile, overwrite_first_rule=True)
workflow.include(snakefile=snakefile, overwrite_default_target=True)
workflow.check()

# code copied and adapted from `snakemake.workflow.Workflow.execute()`
Expand All @@ -113,16 +113,20 @@ def files(items):
else:

def files(items):
relpath = (
lambda f: f
if os.path.isabs(f) or f.startswith("root://")
else os.path.relpath(f)
)
def relpath(f):
return (
f
if os.path.isabs(f) or f.startswith("root://")
else os.path.relpath(f)
)

return map(relpath, filterfalse(workflow.is_rule, items))

if not kwargs.get("targets"):
targets = (
[workflow.first_rule] if workflow.first_rule is not None else list()
[workflow.default_target]
if workflow.default_target is not None
else list()
)

prioritytargets = kwargs.get("prioritytargets", [])
Expand Down Expand Up @@ -157,7 +161,11 @@ def files(items):
omitrules=omitrules,
)

workflow.persistence = Persistence(dag=dag)
if hasattr(workflow, "_persistence"):
workflow._persistence = Persistence(dag=dag)
else:
# for backwards compatibility (Snakemake < 7 for Python 3.6)
workflow.persistence = Persistence(dag=dag)
Comment on lines +164 to +168
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed to also support Python 3.6, that uses a Snakemake version (6.15) in which the new default_target directive is present, but internally persistence had not been replaced by _persistence yet in the snakemake.Workflow class. I think this approach of directly checking the attribute (rather than checking, say, the Python version) is correct, but I'm more than happy to get advice on how to do this better, if you think there's a more appropriate way!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good and I prefer it over checking Snakemake/Python version

dag.init()
dag.update_checkpoint_dependencies()
dag.check_dynamic()
Expand Down
8 changes: 4 additions & 4 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@
"yadage": ["adage~=0.10.1", "yadage~=0.20.1", "yadage-schemas~=0.10.6"],
"cwl": ["cwltool==3.1.20210628163208"],
"snakemake": [
"snakemake==6.8.0 ; python_version<'3.12'",
"snakemake==7.9.0 ; python_version>='3.12'",
"snakemake==6.15.5 ; python_version<'3.7'", # Snakemake v7 requires Python 3.7+
"snakemake==7.32.4 ; python_version>='3.7'",
"tabulate<0.9",
],
"snakemake_reports": [
"snakemake[reports]==6.8.0 ; python_version<'3.12'",
"snakemake[reports]==7.9.0 ; python_version>='3.12'",
"snakemake==6.15.5 ; python_version<'3.7'",
"snakemake==7.32.4 ; python_version>='3.7'",
"pygraphviz<1.8",
"tabulate<0.9", # tabulate 0.9 crashes snakemake, more info: https://github.com/snakemake/snakemake/issues/1899
],
Expand Down
1 change: 1 addition & 0 deletions tests/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ def dummy_snakefile():
"results/foo.txt",
"results/bar.txt",
"results/baz.txt"
default_target: True

rule foo:
input:
Expand Down
4 changes: 0 additions & 4 deletions tests/test_snakemake.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@

def test_snakemake_load(tmpdir, dummy_snakefile):
"""Test that Snakemake metadata is loaded properly."""
if sys.version_info.major == 3 and sys.version_info.minor in (11, 12):
pytest.xfail(
"Snakemake features of reana-client are not supported on Python 3.11"
)
Comment on lines -20 to -23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be removed from tests in reana-client

workdir = tmpdir.mkdir("sub")
# write Snakefile
p = workdir.join("Snakefile")
Expand Down
Loading