# Use --fail-fast in ap_verify to halt execution on first error

XMLWordPrintable

#### Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
1
• Sprint:
AP S22-6 (May), AP F22-1 (June), AP F22-3 (August)
• Team:
• Urgent?:
No

#### Description

I found the --fail-fast option to pipetask, which causes the pipeline execution to fail immediately on error, instead of trying to continue execution. I think this is what we want for ap_verify, as it's purpose is to test our pipeline, and any failures should halt.

Adding "--fail-fast", to line 98 of ap_verify's pipeline_driver.py seems to have done the trick.

Alternately, we could make this a commandline option to ap_verify, if we don't want to always fail immediately (but I think we do?).

#### Activity

Hide
Krzysztof Findeisen added a comment -

I'm sorry, but I still don't understand what you are trying to do (with either change), so I don't believe I'm in a position to approve or disapprove. Perhaps Meredith Rawls has a better understanding of what this would mean for our workflows (if she's not too busy)?

Show
Krzysztof Findeisen added a comment - I'm sorry, but I still don't understand what you are trying to do (with either change), so I don't believe I'm in a position to approve or disapprove. Perhaps Meredith Rawls has a better understanding of what this would mean for our workflows (if she's not too busy)?
Hide
Meredith Rawls added a comment -

I feel like you two disagree on the purpose of ap_verify. Both your views make sense to me... it sounds like John wants "fail fast" to be the ap_verify default to make debugging with it go faster, and Krzysztof is concerned this goes counter to the main purpose of ap_verify as a tool to reproduce runs.

Could we compromise by making "fail fast" an acceptable command-line option for ap_verify (so John can use it for debugging when necessary), but NOT having it be the default (since it's not necessarily desired behavior for checking runs can be reproduced)?

Show
Meredith Rawls added a comment - I feel like you two disagree on the purpose of ap_verify. Both your views make sense to me... it sounds like John wants "fail fast" to be the ap_verify default to make debugging with it go faster, and Krzysztof is concerned this goes counter to the main purpose of ap_verify as a tool to reproduce runs. Could we compromise by making "fail fast" an acceptable command-line option for ap_verify (so John can use it for debugging when necessary), but NOT having it be the default (since it's not necessarily desired behavior for checking runs can be reproduced)?
Hide
Krzysztof Findeisen added a comment -

If Meredith thinks that's best, then I have no objections.

Show
Krzysztof Findeisen added a comment - If Meredith thinks that's best, then I have no objections.
Hide
John Parejko added a comment -

Adding it as a commandline option is less trivial than this commit, so it might be a while before I get to it.

Show
John Parejko added a comment - Adding it as a commandline option is less trivial than this commit, so it might be a while before I get to it.
Hide
Krzysztof Findeisen added a comment -

Following a discussion on #dm-alert-prod about whether ap_verify still has the same purpose it was originally written for (it does not, and I'm the only one who thought otherwise), I've no further objections to the PR as-is.

Show
Krzysztof Findeisen added a comment - Following a discussion on #dm-alert-prod about whether ap_verify still has the same purpose it was originally written for (it does not, and I'm the only one who thought otherwise), I've no further objections to the PR as-is.

#### People

Assignee:
John Parejko
Reporter:
John Parejko
Reviewers:
Krzysztof Findeisen
Watchers:
Eric Bellm, Ian Sullivan, John Parejko, Krzysztof Findeisen, Meredith Rawls