BF(UX): do not keep logging ERROR possibly present in progress records #5936
Conversation
This file contains hidden or 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
Initially observed while giving a live demo (kheh kheh -- always the best time to find bugs) on DANDI datasets such as http://github.com/dandisets/000029, e.g.: (git-annex)lena:/tmp/000029[tags/0.210806.2112^0]git $> datalad get sub-anm36996* [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found [ERROR ] download failed: Not Found Total: 0%| | 0.00/20.7M [00:12<?, ? Bytes/s] Get sub-anm369964/sub-anm .. 9964_behavior+ecephys.nwb: 41%|█▋ | 3.11M/7.66M [00:10<00:15, 295k Bytes/s] which is due to the fact that the first URL in git-annex for the files is no longer available, so annex falls to the second available URL, but it keeps repeating the same list error-messages in its --json-error-messages: $> 'git' 'annex' 'get' '--json' '--json-error-messages' '--json-progress' '-c' 'annex.dotfiles=true' '--' 'sub-anm369962/sub-anm369962_behavior+ecephys.nwb' {"byte-progress":4989429,"action":{"command":"get","note":"from web...","input":["sub-anm369962/sub-anm369962_behavior+ecephys.nwb"],"key":"SHA256E-s6644036--d01c43576e3aeceed53a052defa8bbbf15b885146f518acf63d228cee44576ac.nwb","error-messages":[" download failed: Not Found"],"file":"sub-anm369962/sub-anm369962_behavior+ecephys.nwb"},"total-size":6644036,"percent-progress":"75.1%"} {"byte-progress":5546485,"action":{"command":"get","note":"from web...","input":["sub-anm369962/sub-anm369962_behavior+ecephys.nwb"],"key":"SHA256E-s6644036--d01c43576e3aeceed53a052defa8bbbf15b885146f518acf63d228cee44576ac.nwb","error-messages":[" download failed: Not Found"],"file":"sub-anm369962/sub-anm369962_behavior+ecephys.nwb"},"total-size":6644036,"percent-progress":"83.48%"} ... {"byte-progress":6451701,"action":{"command":"get","note":"from web...","input":["sub-anm369962/sub-anm369962_behavior+ecephys.nwb"],"key":"SHA256E-s6644036--d01c43576e3aeceed53a052defa8bbbf15b885146f518acf63d228cee44576ac.nwb","error-messages":[" download failed: Not Found"],"file":"sub-anm369962/sub-anm369962_behavior+ecephys.nwb"},"total-size":6644036,"percent-progress":"97.11%"} {"command":"get","note":"from web...\nchecksum...","success":true,"input":["sub-anm369962/sub-anm369962_behavior+ecephys.nwb"],"key":"SHA256E-s6644036--d01c43576e3aeceed53a052defa8bbbf15b885146f518acf63d228cee44576ac.nwb","error-messages":[" download failed: Not Found"],"file":"sub-anm369962/sub-anm369962_behavior+ecephys.nwb"} Not sure yet if there could be a json record from annex which would have "action" but without byte-progress reporting, so decided to just condition on it being not a progress report for logging an error (work @mih is doing might eventually remove all those ERROR logs anyways). With this patch we would just get the cleaner $> datalad get sub-anm369962/sub-anm369962_behavior+ecephys.nwb get(ok): sub-anm369962/sub-anm369962_behavior+ecephys.nwb (file) [download failed: Not Found] where this non-consequential/non-informative and possibly confusing to the user error is reported, but how could we address that (e.g. not report any errors if command eventually succedeed?) is a separate question. PS. relevant issues in dandisets, which whenever addressed would eliminate this problem, and thus would cause this issue to not reproduce using aforementioned commands: - dandi/dandisets#78 - dandi/dandisets#79
yarikoptic
added
the
merge-if-ok
Codecov Report
@@ Coverage Diff @@
## maint #5936 +/- ##
===========================================
- Coverage 90.28% 55.63% -34.66%
===========================================
Files 300 297 -3
Lines 42516 42480 -36
===========================================
- Hits 38387 23632 -14755
- Misses 4129 18848 +14719
Continue to review full report at Codecov.
|
Member
Author
|
travis -- unrelated error, restarted that one run. appveyor -- |
Member
Author
|
ok, no objections stated, and it is imho quite straightforward and goes along with the idea of reduced logging, so I will proceed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Initially observed while giving a live demo (kheh kheh -- always the best time
to find bugs) on DANDI datasets such as
http://github.com/dandisets/000029, e.g.:
which is due to the fact that the first URL in git-annex for the files is
no longer available, so annex falls to the second available URL, but it keeps
repeating the same list error-messages in its --json-error-messages:
Not sure yet if there could be a json record from annex which would have
"action" but without byte-progress reporting, so decided to just condition on
it being not a progress report for logging an error (work @mih is doing might
eventually remove all those ERROR logs anyways). With this patch we would just
get the cleaner
where this non-consequential/non-informative and possibly confusing to the user
error is reported, but how could we address that (e.g. not report any errors if
command eventually succedeed?) is a separate question.
PS. relevant issues in dandisets, which whenever addressed would eliminate this
problem, and thus would cause this issue to not reproduce using aforementioned
commands: