BF: save: Honor on_failure #3470
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Ran into this when going down a rabbit hole for gh-3468. |
Codecov Report
@@ Coverage Diff @@
## master #3470 +/- ##
==========================================
+ Coverage 91.26% 91.27% +0.01%
==========================================
Files 272 272
Lines 34903 34910 +7
==========================================
+ Hits 31853 31865 +12
+ Misses 3050 3045 -5
Continue to review full report at Codecov.
|
yarikoptic
reviewed
Jun 4, 2019
save() calls status() with the default on_failure value of "continue",
which means that---regardless of the on_failure value specified for
save()---status() always raises an IncompleteResultsError exception
when a failure occurs and then save() aborts. Make save() call
status() with on_failure="ignore" so that the *save* caller gets to
decide what to do in the case of a failure.
Note that this results in some double reporting:
[ERROR ] path not underneath this dataset [status(...)]
status(error): ../a [path not underneath this dataset]
But if save() were to just swallow the result so that only the log
message is shown, an on_failure value of "continue" or "stop" would
not raise an IncompleteResultsError exception or exit on the command
line with a non-zero status.
mih
reviewed
Jun 24, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
save() calls status() with the default on_failure value of "continue",
which means that---regardless of the on_failure value specified for
save()---status() always raises an IncompleteResultsError exception
when a failure occurs and then save() aborts. Make save() call
status() with on_failure="ignore" so that the save caller gets to
decide what to do in the case of a failure.
Note that this results in some double reporting:
But if save() were to just swallow the result so that only the log
message is shown, an on_failure value of "continue" or "stop" would
not raise an IncompleteResultsError exception or exit on the command
line with a non-zero status.