-
Notifications
You must be signed in to change notification settings - Fork 109
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
Capture error messages from git-annex's JSON output #3751
Conversation
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.
I remember adding a test on operations when permissions are wrong (grep for sudo?). Might be a good sport to add a check
@@ -2060,7 +2060,7 @@ def _run_annex_command_json(self, command, | |||
log_online=True | |||
)) | |||
# TODO: refactor to account for possible --batch ones | |||
annex_options = ['--json'] | |||
annex_options = ['--json', '--json-error-messages'] |
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.
We would need to check from which version of annex it was introduced (I still have vague memory asking for it so it might have been not too long ago) and possibly boost our minimal annex version requirement accordingly.
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.
I failed to come up with a test that isnt insanely complex across test setups, otherwise I would have added one. If you consider this a showstopper, please contribute. |
Test is Not a show stopper but very nice to have. Grep for sudo lead me to datalad/datalad/support/tests/test_annexrepo.py Line 2231 in b950066
I could look later when I get to the laptop which line or two to add |
Codecov Report
@@ Coverage Diff @@
## master #3751 +/- ##
==========================================
+ Coverage 82.92% 82.93% +<.01%
==========================================
Files 273 273
Lines 35942 35961 +19
==========================================
+ Hits 29804 29823 +19
Misses 6138 6138
Continue to review full report at Codecov.
|
In order to be able to capture any error messages as part of the JSON-lines output. Precondition for dataladgh-3685
Turn this: ``` % datalad save add(error): that3 (file) ``` into ``` % datalad save add(error): that3 (file) [ that3: setFileMode: permission denied (Operation not permitted)] ``` hence fixes dataladgh-3685
Test is in. Test passes. Merging.... |
Turns this:
into
hence fixes gh-3685
Note that this does not fix the same issue in
add()
, because different code is used (see gh-3714).