-
Notifications
You must be signed in to change notification settings - Fork 375
[WX-1361] Remove Confusing Error Message #7449
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
Conversation
aednichols
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work finding a concise solution!
Co-authored-by: Adam Nichols <anichols@broadinstitute.org>
jgainerdewar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job finding such a small and tidy solution!
| returnCode: Option[Int] = None, | ||
| jobExecutionMap: JobExecutionMap = Map.empty | ||
| jobExecutionMap: JobExecutionMap = Map.empty, | ||
| omitFromWorkflowLevelFailures: Boolean = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick that you can feel free to ignore if you disagree - IMO boolean arguments where true represents negation require readers to think too hard. I think it's easier to understand if it's includeInWorkflowLevelFailures: Boolean = true.
Description
When Cromwell restarts during a failure, it must fail the 'next' upcoming tasks in order to cleanly terminate the workflow. During this process, it was logging an error that isn't really an error (and was very confusing to users).
This PR:
Release Notes Confirmation
CHANGELOG.mdCHANGELOG.mdin this PRCHANGELOG.mdbecause it doesn't impact community usersTerra Release Notes