[main] Add --abort-on-failure flag to rootcp and some sanity checks#22555
[main] Add --abort-on-failure flag to rootcp and some sanity checks#22555silverweed wants to merge 2 commits into
Conversation
vepadulano
left a comment
vepadulano
left a comment
There was a problem hiding this comment.
I like the idea! I left a couple of minor suggestions, also a test would be nice to have.
silverweed
force-pushed
the
rootcp_strict
branch
from
22a1703 to
408bcbc
Compare
Test Results 22 files 22 suites 3d 13h 23m 36s ⏱️ For more details on these failures, see this check. Results for commit d87406c. ♻️ This comment has been updated with latest results. |
| if (args_.fStrict) { | ||
| Err() << "node failed to be copied. Aborting.\n"; | ||
| return false; | ||
| } else { | ||
| Warn() << "node will be skipped. Use --abort-on-failure if you want to abort on failure.\n"; | ||
| return true; |
There was a problem hiding this comment.
Are we guaranteed an error message from CopyNode that identify the failed node? (Other wise consider adding an identifier to the Error/Warning).
There was a problem hiding this comment.
Yes, every return false; in CopyNode is accompanied by an error/warning with the specifics
| --recreate recreate the destination file. | ||
| -r, --recursive recurse inside directories | ||
| --replace replace object if already existing | ||
| -a, --abort-on-failure abort if any object fails to be copied (instead of skipping such objects) |
There was a problem hiding this comment.
Does the output get deleted or is there an incomplete file left? (Should the choice between the two be left to the user?)
There was a problem hiding this comment.
Good point, it currently does not get removed but it probably should. I don't think we should add a flag to tweak that.
Otherwise it may fail to run if the file is already there
silverweed
force-pushed
the
rootcp_strict
branch
from
408bcbc to
d87406c
Compare
silverweed
changed the title
3 participants
Adding the option to abort the entire copy if any object fails to be copied.