Skip to content

fix: forwarding --keep-incomplete to cluster executor#1951

Merged
johanneskoester merged 1 commit intosnakemake:mainfrom
prs513rosewood:prs513rosewood/forward-keep-incomplete
Nov 8, 2022
Merged

fix: forwarding --keep-incomplete to cluster executor#1951
johanneskoester merged 1 commit intosnakemake:mainfrom
prs513rosewood:prs513rosewood/forward-keep-incomplete

Conversation

@prs513rosewood
Copy link
Contributor

@prs513rosewood prs513rosewood commented Nov 3, 2022

Description

The option --keep-incomplete was not correctly forwarded in the case of cluster execution, causing jobs to clean up output files in case of failure, despite the main snakemake process being called with --keep-incomplete. Below is a MWE:

rule test:
    output:
        "file.out"
    shell:
        "touch {output[0]}"

Run with snakemake -j1 --cluster "bash -x" --keep-incomplete file.out to see that --keep-incomplete is absent from the snakemake call in the generated jobscript.

I've looked at adding a regression test, but I'm not sure how to do that in the snakemake tests, which is why the box below is unchecked. Any advice is appreciated.

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • 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).

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 3, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@johanneskoester johanneskoester merged commit 2894c7d into snakemake:main Nov 8, 2022
@johanneskoester
Copy link
Contributor

johanneskoester commented Nov 8, 2022

Thanks a lot!

@github-actions github-actions bot mentioned this pull request Nov 8, 2022
Merged
@prs513rosewood prs513rosewood deleted the prs513rosewood/forward-keep-incomplete branch November 8, 2022 13:59
@dariober
Copy link
Contributor

dariober commented Nov 9, 2022

I have nothing to contribute other than saying that --cluster "bash -x" is a really cool way to mock cluster execution (do I understand it right?). It would be nice to make it an FAQ or a Q&A on StackOverflow for future reference. Thanks!

johanneskoester pushed a commit that referenced this pull request Nov 10, 2022
@github-actions
chore(main): release 7.18.2 (#1950)
a5c3523 
🤖 I have created a release *beep* *boop*
---


##
[7.18.2](v7.18.1...v7.18.2)
(2022-11-10)


### Bug Fixes

* Change ratelimiter dependency to throttler
([#1958](#1958))
([50b8f16](50b8f16))
* fixed problem with leaked modifications when inheriting multiple times
from the same rule
([#1957](#1957))
([2475cbc](2475cbc))
* forwarding --keep-incomplete to cluster executor
([#1951](#1951))
([2894c7d](2894c7d))
* show input files on job error
([#1949](#1949))
([ad21631](ad21631))

---
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>
wleepang pushed a commit to wleepang/snakemake that referenced this pull request Jun 27, 2023
Squashed commit of the following:
9e81306 
commit 7ee6b53fc76769d68f941a081401c914507939c2
Author: Pang, Lee <pwyming@.amazon.com>
Date:   Sun Jun 25 20:58:16 2023 -0700

    enable using containers with rules

commit 7ca2cba3d1b84cd73d0d886a95f03ec81b2ff173
Author: Pang, Lee <pwyming@.amazon.com>
Date:   Fri May 26 15:16:06 2023 -0700

    use workflow derived container image for job def

commit f8638d60da664300f5c2c8d803d6c75075d4851d
Author: Pang, Lee <pwyming@.amazon.com>
Date:   Fri May 26 15:05:19 2023 -0700

    bump snakemake container to v7.26.0 from ECR Public

commit 0f53f9b8f2e7bfdd716d9430e704014dd823b2e5
Author: Pang, Lee <pwyming@.amazon.com>
Date:   Fri May 26 14:58:46 2023 -0700

    always use EFS with AWS Batch

commit bf1a0e8bdc4d7968bbb2654730ecf2027d1318fe
Author: Pang, Lee <pwyming@.amazon.com>
Date:   Fri May 26 14:20:38 2023 -0700

    remove redundant arguments to workflow.execute

commit b66cf19361c90bcb7b2afea8d0279ef870cf885d
Author: Matthew [C] Carpenter <carpedma@amazon.com>
Date:   Thu Dec 8 16:01:02 2022 +0000

    Resolved atomic changes from:

    commit e5dee15ea6fab0eab873e9a5d1b7851a9fa7ab85
    Author: Matthew [C] Carpenter <carpedma@amazon.com>
    Date:   Tue Dec 6 11:04:03 2022 +0000

        use snakemake/snakemake:v7.18.2 as task queue container image

        SIM: https://i.amazon.com/codon-178
        cr: https://code.amazon.com/reviews/CR-80987193

    commit d74678aa8414192c93b729d5f255f70bfcb0fb17
    Author: Matthew [C] Carpenter <carpedma@amazon.com>
    Date:   Mon Dec 5 20:46:17 2022 +0000

        wip

    Assuming rebase on history ending at:

    commit 1ecd9370c52a6658ae7cbb95c77d34bfea08fd1c
    Author: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
    Date:   Thu Nov 10 20:36:09 2022 +0100

        chore(main): release 7.18.2 (snakemake#1950)

        🤖 I have created a release *beep* *boop*
        ---
        [7.18.2](snakemake/snakemake@v7.18.1...v7.18.2)
        (2022-11-10)

        * Change ratelimiter dependency to throttler
        ([snakemake#1958](snakemake#1958))
        ([50b8f16](snakemake@50b8f16))
        * fixed problem with leaked modifications when inheriting multiple times
        from the same rule
        ([snakemake#1957](snakemake#1957))
        ([2475cbc](snakemake@2475cbc))
        * forwarding --keep-incomplete to cluster executor
        ([snakemake#1951](snakemake#1951))
        ([2894c7d](snakemake@2894c7d))
        * show input files on job error
        ([snakemake#1949](snakemake#1949))
        ([ad21631](snakemake@ad21631))

        ---
        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>

    commit 6780fdfbdf7df569f14766de3013292c7caf9c26
    Author: Peter Van Dyken <87136354+pvandyken@users.noreply.github.com>
    Date:   Thu Nov 10 14:33:59 2022 -0500

        fix: Change ratelimiter dependency to throttler (snakemake#1958)

        ratelimiter used outdated asyncio code that no longer works in Python
        3.11. Throttler is more up to date

        As part of the change, _wait_for_jobs in all ClusterExecutors have been
        made async to be compatible with throttler. The Documentation on cluster
        design has been updated accordingly.

        Resolves snakemake#1952

        <!--Add a description of your PR here-->
        <!-- 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).

commit 44387f366b85c4e68ad70869355290e10b1cc6ae
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Wed Apr 6 15:35:04 2022 -0700

    Fixes bad if check for assigning the resource requirements

commit 4288b2523f0a91f8ea65a4f76829959b0f3d6bb7
Author: Mark Schreiber <mrschre@amazon.com>
Date:   Wed Apr 6 18:10:31 2022 +0000

    commit: d748e303e47e7926cee808b4b6e9a1d6081d5342
    Author: Mark Schreiber <mrschre@amazon.com>
    Date: 2022-04-06T17:37:20.000Z

    Add executor.cancel() call to sigterm handler

commit 320bd826f877643facd83336ccb5209f04a72283
Author: Gurinder <gwalia@amazon.com>
Date:   Wed Feb 23 15:51:19 2022 -0800

    fix sm image version

commit 90588a645ca877c0ea3e6fde2757835f5eb8cbf4
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Tue Feb 15 15:00:28 2022 -0800

    Addresses PR comments from Gurinder

commit 03a47410ce94755050f9a25f2afb9d6a37275647
Author: Gurinder <gwalia@amazon.com>
Date:   Wed Feb 9 15:27:56 2022 -0800

    lock snakemake to version 6.15.1

commit 3e8a38f17cf81077cc8b5234430f55035dbc13bc
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Thu Feb 3 10:34:55 2022 -0800

    Adds a list output commands to snakemake

commit c4eff8bc33ebd6bb6357a749c9da4fd89960a741
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Wed Feb 2 09:50:04 2022 -0800

    Fixes how the snakemake project runs EFS code

commit e9714c7a7be51c3d9caeeb3960a498caf0ccb1f0
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Fri Jan 28 14:03:32 2022 -0800

    Fixes tags arg

commit ad0b20cd26637b4138950d0bf95d20962156c24d
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Thu Dec 23 15:10:10 2021 -0800

    Initial aws_batch commit

commit 6a441b73ef2bde2963eaa222afae63d425409c1f
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Thu Jan 27 14:18:41 2022 -0800

    Changes how tags are defined

commit 097049e3db5ae476e8a61593adc57afd4a55f0f3
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Thu Jan 27 11:07:44 2022 -0800

    Commits the workflow zip to unblock co-worker

commit 2a0f9f63ef52bc39586c9873f60a5b8cae76354c
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Wed Jan 26 00:27:42 2022 +0000

    Fixes bug with startup script

commit 427d54f5ae17a080a0ee27c66da7778c1c12414f
Author: Gurinder <gwalia@amazon.com>
Date:   Mon Jan 3 11:26:45 2022 -0800

    Updated aws dockerfile to only have repository code + fixed buildspec for codebuild

commit 0f474381629230c841769253a297ce0b870468d4
Author: Gurinder <gwalia@amazon.com>
Date:   Thu Dec 30 17:21:34 2021 -0800

    Added aws_dockerfile and buildspec to create image to pull into snakemake base image

commit 5d3b5263a50f8f81da82dd703d80146b802b8499
Author: Elliot Smith <elliotsm@amazon.com>
Date:   Thu Dec 23 15:10:10 2021 -0800

    Initial aws_batch commit
@nsiccha
Copy link

nsiccha commented Oct 13, 2023

Not sure whether I should open another issue, but when using slurm I'm having the problem which was fixed here.

I.e. when running e.g. snakemake --keep-going --keep-incomplete --slurm -j1024 --default-resources runtime=10 mem_mb=1000 cpus_per_task=1 with snakemake version 7.32.4 snakemake keeps deleting all output files in a group if a single job failed in that group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johanneskoester johanneskoester johanneskoester approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@prs513rosewood @johanneskoester @dariober @nsiccha