From 6b77c02be9446745f3b7d6ecf62fdb5c75eea7dd Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Tue, 25 Apr 2023 14:30:27 -0400 Subject: [PATCH] comment --- ..._3f0872f13160a77a82bd5a95fc3f397d._comment | 9 ++++++- ..._0217c8acd2a9eb3bba1fc497856cd96a._comment | 27 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_4_0217c8acd2a9eb3bba1fc497856cd96a._comment diff --git a/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_3_3f0872f13160a77a82bd5a95fc3f397d._comment b/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_3_3f0872f13160a77a82bd5a95fc3f397d._comment index 1fa3b718b2..b48dd361f7 100644 --- a/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_3_3f0872f13160a77a82bd5a95fc3f397d._comment +++ b/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_3_3f0872f13160a77a82bd5a95fc3f397d._comment @@ -12,6 +12,13 @@ caused it, and so an exit status is not enough information. So the additional json would need to include the filename that didn't exist. +[this closed todo](https://git-annex.branchable.com/projects/datalad/bugs-done/copy_does_not_reflect_some_failed_copies_in_--json_output/) +argues persuasively that --json-error-messages cannot be extended to +include these errors as-is. In particular, putting them in the current +json would mean outputting a record without a "key" field in situations +where that field is always present, so json consumers might +misbehave without it. + bpoldrack makes a good point [here](https://github.com/datalad/datalad/pull/6510#issuecomment-1057320621): @@ -24,7 +31,7 @@ bpoldrack makes a good point So eg: - {"exception":"UNIQUEID", "file", "foo", "error-messages":["foo not found"]} + {"exception":"UNIQUEID", "file":"foo", "error-messages":["foo not found"]} That seems about right to me, and it future proofs git-annex to be able to report other exceptions in json output later on. diff --git a/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_4_0217c8acd2a9eb3bba1fc497856cd96a._comment b/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_4_0217c8acd2a9eb3bba1fc497856cd96a._comment new file mode 100644 index 0000000000..d977b33910 --- /dev/null +++ b/doc/todo/api_for_telling_when_nonexistant_or_non_git_files_passed/comment_4_0217c8acd2a9eb3bba1fc497856cd96a._comment @@ -0,0 +1,27 @@ +[[!comment format=mdwn + username="joey" + subject="""comment 4""" + date="2023-04-25T17:49:23Z" + content=""" +What should enable that new json output for exceptions, +--json-error-messages or a new option like --json-exceptions? + +The risk is that, since this is a new object type, it breaks a json +consumer that expects to find a `{"command":...}` object. + +I do think that git-annex should be free to add new fields to existing json +objects. But adding a new object type seems considerably more risky. +git-annex has never done that before (eg --json-progress outputs the +"command" object with updated values). + +I took a look at datalad's parsing of json from git-annex, to see what it might +do if it got a new object type unexpectedly. I'm not convinced it wouldn't +misbehave. For example, in `datalad/interface/results.py` `annexjson2result`: + + res['status'] = 'ok' if d.get('success', False) is True else 'error' + +Without a "status" field this will yield "error" which could go on to +cause unexpected behavior + +So, I think this would need to be a new --json-exceptions option. +"""]]