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.

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

feat(executor): upgrade to Snakemake v7.32.4 #81

Merged
merged 1 commit into from
Feb 21, 2024

Conversation

giuseppe-steduto
Copy link
Member

Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
_wait_for_jobs method into a coroutine.

Closes #31
Closes reanahub/reana-client#655

giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Jan 29, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Jan 30, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
Copy link

codecov bot commented Jan 30, 2024

Codecov Report

Attention: 6 lines in your changes are missing coverage. Please review.

Comparison is base (4782678) 3.84% compared to head (4a3f359) 3.82%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #81      +/-   ##
=========================================
- Coverage    3.84%   3.82%   -0.03%     
=========================================
  Files           6       6              
  Lines         182     183       +1     
=========================================
  Hits            7       7              
- Misses        175     176       +1     
Files Coverage Δ
reana_workflow_engine_snakemake/executor.py 0.00% <0.00%> (ø)

Dockerfile Outdated
Comment on lines 96 to 98
# hadolint ignore=DL3013
RUN if [ "${DEBUG}" -gt 0 ]; then pip install --no-cache-dir -e ".[debug,xrootd]"; else pip install --no-cache-dir ".[xrootd]"; fi;

Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to move this after the installation of reana-commons?

Copy link
Member Author

Choose a reason for hiding this comment

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

When building locally in debug mode and installing a version of reana-commons that is only available locally (not released on PyPi yet), this step was failing because it could not find the required reana-commons version.
Moving it after the installation of reana-commons solves the issue.

Copy link
Member

Choose a reason for hiding this comment

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

Any reasons why you needed to change the version of r-commons locally? Just to understand what is the use case

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just usually what I do when updating r-commons and some other component at the same time, especially because I can quickly check if the changes were propagated correctly by just looking at the installed r-commons version

Copy link
Member

Choose a reason for hiding this comment

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

FWIW it would be nice to keep the same order for all the REANA components, i.e. either make this change everywhere, or just keep the original order here too.

Personally when I was testing your branch, I just downgraded the setup.py line to require 0.9.5 by a local edit, and tested it that way.

IMO the old order was acceptable, because we usually only bump r-commons version when the new r-commons is released on PyPI, so the branch remains in a "easily testable" locally for a long time, so the above setup.py micro-editing is not even needed.

That said, we could also change the order if it make things easier -- and there should be no danger of overriding some dependencies in that way -- but then let's do it for all the components systematically, for the overall consistency, and in a nicely separate commit, for better housekeeping.

setup.py Outdated Show resolved Hide resolved
giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Jan 30, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Feb 13, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
Dockerfile Outdated
Comment on lines 106 to 108
# Create and switch to REANA user to be able to create snakemake-specific
# directories in the home folder.
RUN useradd reana --uid 1000 --create-home
USER reana

Copy link
Member

Choose a reason for hiding this comment

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

After discussing with Tibor, given that the UID of the user can be changed with WORKFLOW_RUNTIME_USER_UID, maybe it's better if we directly create the directory in the Dockerfile with mkdir /.cache/snakemake and then we set the env variable XDG_CACHE_HOME to /.cache? In this way, independently by which user is used, the directory always exists

giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Feb 19, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
Dockerfile Outdated
@@ -102,6 +102,10 @@ RUN pip check
ENV TERM=xterm \
PYTHONPATH=/workdir

# Create and set cache directory to be used by Snakemake
RUN mkdir -p /.cache/snakemake && chmod 644 /.cache
Copy link
Member

Choose a reason for hiding this comment

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

The above does not work for me. The following does:

diff --git a/Dockerfile b/Dockerfile
index c73f1a2..d8b036a 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -103,7 +103,7 @@ ENV TERM=xterm \
     PYTHONPATH=/workdir

 # Create and set cache directory to be used by Snakemake
-RUN mkdir -p /.cache/snakemake && chmod 644 /.cache
+RUN mkdir -p /.cache/snakemake && chmod ug+rwx /.cache/snakemake
 ENV XDG_CACHE_HOME=/.cache

 # Set image labels

giuseppe-steduto added a commit to giuseppe-steduto/reana-workflow-engine-snakemake that referenced this pull request Feb 21, 2024
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
Amend the overridden executor to reflect the changes in the new version
of Snakemake, in particular with regard to the change of the
`_wait_for_jobs` method into a coroutine.

Closes reanahub#31
Closes reanahub/reana-client#655
@tiborsimko tiborsimko merged commit 4a3f359 into reanahub:master Feb 21, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

python 3.11: cannot submit snakemake workflows setup: upgrade to latest snakemake version
3 participants