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

Make jobs automatically resubmit for a much wider range of exit codes #2170

Merged
merged 5 commits into from Feb 5, 2023

Conversation

ewels
Copy link
Member

@ewels ewels commented Jan 24, 2023

now 130..145

Discussion in #configs channel on the nf-core Slack.

'cc @pontus @pcantalupo @muffato @FelixKrueger

'cc @nf-core/core for visibility as this is a fairly wide ranging change (though hopefully not controversial)

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@codecov
Copy link

codecov bot commented Jan 24, 2023

Codecov Report

Merging #2170 (6ff70af) into dev (110cec8) will increase coverage by 0.08%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##              dev    #2170      +/-   ##
==========================================
+ Coverage   71.54%   71.62%   +0.08%     
==========================================
  Files          77       77              
  Lines        8367     8375       +8     
==========================================
+ Hits         5986     5999      +13     
+ Misses       2381     2376       -5     
Impacted Files Coverage Δ
nf_core/bump_version.py 89.47% <ø> (ø)
nf_core/create.py 65.01% <ø> (+0.38%) ⬆️
nf_core/download.py 52.72% <ø> (ø)
nf_core/launch.py 63.26% <ø> (ø)
nf_core/lint/__init__.py 74.18% <ø> (ø)
nf_core/lint/files_unchanged.py 72.61% <ø> (ø)
nf_core/lint/template_strings.py 87.50% <ø> (ø)
nf_core/list.py 79.82% <ø> (ø)
nf_core/modules/bump_versions.py 63.90% <ø> (ø)
nf_core/utils.py 82.38% <ø> (+0.24%) ⬆️
... and 8 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

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

sounds reasonable

@muffato
Copy link
Member

muffato commented Jan 24, 2023

I noticed that 104 is not in the list anymore. Does anyone remember why it was needed and if it's still relevant ?

@ewels you were in fact involved in 104 getting into the pipeline template ;) #2170, which is a port from nf-core/rnaseq#56

@ewels
Copy link
Member Author

ewels commented Jan 25, 2023

Oof, good spot + detective work @muffato! No I have no memory of why it was there. Presumably saw it somewhere..

Had a quick look in Slack and didn't find a lot, but one or two messages / google suggest that it could crop up as an out-of-memory exit code sometimes. I guess we can add it back here 👍🏻

Anyone got some quick-fire groovy skills to add it into the array in a nice succinct way? Does 104 + 130..145 or something work?

@ewels
Copy link
Member Author

ewels commented Jan 30, 2023

Haven't properly tested the last commit but should hopefully be ok to merge now 👍🏻

Copy link
Member

@muffato muffato left a comment

Choose a reason for hiding this comment

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

A minor change to match the changelog with the code

CHANGELOG.md Outdated Show resolved Hide resolved
Co-authored-by: Matthieu Muffato <cortexspam-github@yahoo.fr>
@ewels
Copy link
Member Author

ewels commented Feb 4, 2023

Black released a new stable update, which is why everything is breaking. Re-ran Black here to fix the tests.

@ewels ewels merged commit 6f79b56 into nf-core:dev Feb 5, 2023
@ewels ewels deleted the errorStrategy_exitCodes branch February 5, 2023 13:21
@mirpedrol mirpedrol mentioned this pull request Feb 9, 2023
4 tasks
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

6 participants