ENH: minor tune up of addurls to be more tollerant and "informative"#7388
Merged
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## maint #7388 +/- ##
===========================================
+ Coverage 20.56% 90.70% +70.13%
===========================================
Files 327 327
Lines 44163 44691 +528
Branches 0 5922 +5922
===========================================
+ Hits 9084 40537 +31453
+ Misses 35079 4138 -30941
- Partials 0 16 +16
☔ View full report in Codecov by Sentry. |
yarikoptic
added
semver-patch
github-actions
Bot
removed
the
CHANGELOG-missing
Member
Author
|
@mih please chime in. |
…ta fields Use case is the https://gpt4all.io/models/models.json where two keys are used only in a few records. We can exclude them for all but I thought it would then would effectively loose metadata. An alternative is to provide an alternative value e.g. "n/a" so may be worth an option?
ATM it would just silently proceed and then end up downloading possibly huge files without providing user an alert of some kind that some fields he/she entered are not present. IMHO it is better to make it more explicit than to require digging through DEBUG
Collaborator
|
PR released in |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
I was trying addurls on a file from https://gpt4all.io/models/models.json .
First
was just crashing since not all records have some fields. I could have excluded them using
-xregex, but I thought it would be suboptimal since that would result in not recording some metadata. So I decided just to issue a warning and proceed adding all other fields which have metadata present.Then, surprisingly the
--keysimply had no effect and no warning of any kind was issued. It was since I usedsizeinstead offilesize. DEBUG level message was likely there but IMHO it is too much of a user mistake to just silently ignore since IMHO it is most likely a misspecification of the--key. So I decided to issue a warning there too. And I did not really see the point for CapturedException there, so removed that part.