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

fix: Repair MREs from #823 #1203

Merged
merged 12 commits into from Feb 21, 2022
Merged

fix: Repair MREs from #823 #1203

merged 12 commits into from Feb 21, 2022

Conversation

epruesse
Copy link
Contributor

@epruesse epruesse commented Oct 2, 2021

Description

see #823

Three MREs:

  • Checkpoints with temp() output lead to incomplete DAG execution
  • Checkpints with pipe() lead to incomplete DAG execution
  • Checkpoint target rules removed before 100% message displayed

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

Maarten-vd-Sande and others added 6 commits Sep 22, 2021
- Can't have the b.txt in first text be temp and expect it present, it
will correctly be deleted.
- Add third MRE which is targeting more of a log issue
The main scheduler loop terminates if there are no more runnable
jobs. This tests if there still are jobs that should be run, but that
we cannot seem to get to. If so, print an error message, print the
files affected, and exit 1. The latter is important so Snakemake can,
as hotfix, be rerun in a loop until it's actually completed all
tasks.
The aggressive early deletion of temp files must ignore
checkpoints. Other rules depend on a checkpoint only via the first
output file of the checkpoint (flagged string) as a stub. All other
outputs marked as temp would be deleted (and have been before this
patch). Since those files are missing, the dependent rules will never
be runnable and the workflow ends before all targets have been built.
@epruesse epruesse requested a review from johanneskoester as a code owner Oct 2, 2021
@epruesse epruesse changed the title Fix 823 (partially at least) fix: Repair MREs from #823 Oct 2, 2021
@epruesse
Copy link
Contributor Author

epruesse commented Oct 2, 2021

@johanneskoester Snakemake exits when no more runnable or running jobs exist. One indicator of the 823 issue is that there still are jobs that should be run left. I'm checking for this here, printing an error and exiting with 1. You may want to change the issue url to point to an issue of your own, where you've got the top post to inform about the bug. I was able to fix the MREs posted in the issue, but I feel like this isn't the last we've heard of this.

Copy link
Contributor

@johanneskoester johanneskoester left a comment

Thanks a lot! Very good catch.

@sonarcloud
Copy link

sonarcloud bot commented Oct 26, 2021

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

@sonarcloud
Copy link

sonarcloud bot commented Feb 21, 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 b007979 into snakemake:main Feb 21, 2022
7 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.

None yet

3 participants