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: Adding --cluster-cancel and --cluster-cancel-nargs #1395

Merged
merged 4 commits into from Feb 17, 2022

Conversation

holtgrewe
Copy link
Contributor

Description

This PR adds the optional arguments --cluster-cancel and --cluster-mcancel. The user can specify commands for terminating jobs by their job ID, either one at a time (--cluster-cancel) or multiple at once (--cluster-mcancel). Testing this was somewhat tricky because you have to simulate senting SIGINT to the job so I ended extending tests/common.py a bit.

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

Addresses: #570

@holtgrewe holtgrewe linked an issue Feb 14, 2022 that may be closed by this pull request
@johanneskoester johanneskoester changed the title Adding --cluster-cancel and --cluster-mcancel feat: Adding --cluster-cancel and --cluster-mcancel Feb 14, 2022
Copy link
Contributor

@johanneskoester johanneskoester left a comment

Choose a reason for hiding this comment

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

Thanks a lot, very nice work! The Snakefile of your testcase is missing. Apart from that, a few requests below. And one "code smell" in the sonarcloud report.

snakemake/executors/__init__.py Outdated Show resolved Hide resolved
snakemake/executors/__init__.py Outdated Show resolved Hide resolved
tests/common.py Outdated Show resolved Hide resolved
@holtgrewe holtgrewe force-pushed the cluster-cancel branch 2 times, most recently from 5d739a4 to a8bbbdf Compare February 15, 2022 09:21
@holtgrewe
Copy link
Contributor Author

Sorry for the many pushes but I had a hard time uncovering why the CI was failing and it was running locally (as so often, timing issues because of fewer cores in CI than in dev). I'm positive that this is ready for re-review 👓 🔍 🔬

@holtgrewe
Copy link
Contributor Author

🎉

@holtgrewe holtgrewe mentioned this pull request Feb 15, 2022
2 tasks
@holtgrewe holtgrewe changed the title feat: Adding --cluster-cancel and --cluster-mcancel feat: Adding --cluster-cancel and --cluster-cancel-nargs Feb 15, 2022
@sonarcloud
Copy link

sonarcloud bot commented Feb 17, 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

Copy link
Contributor

@johanneskoester johanneskoester left a comment

Choose a reason for hiding this comment

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

Excellent, awesome and clean work, as always!

@johanneskoester johanneskoester merged commit 0593de1 into snakemake:main Feb 17, 2022
cpauvert pushed a commit to cpauvert/snakemake that referenced this pull request Feb 17, 2022
)

* Adding support for --cluster-[m]cancel

* Adding support for --cluster-[m]cancel.

* Adressing points from code review

* [ci skip] fix typos

Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
johanneskoester added a commit that referenced this pull request Feb 18, 2022
* Remove the decode attributes of source files fix #1393

* Adding a test with report generation without md5sum check

* Formatting with black

* feat: Adding --cluster-cancel and --cluster-cancel-nargs (#1395)

* Adding support for --cluster-[m]cancel

* Adding support for --cluster-[m]cancel.

* Adressing points from code review

* [ci skip] fix typos

Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>

Co-authored-by: Johannes Köster <johannes.koester@tu-dortmund.de>
Co-authored-by: Manuel Holtgrewe <manuel.holtgrewe@bihealth.de>
Co-authored-by: Johannes Köster <johannes.koester@uni-due.de>
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.

Canceling jobs through Snakemake profiles?
2 participants