Refacto error parsing (HfHubHttpError) #2474
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
This comment was marked as duplicate.
This comment was marked as duplicate.
| @@ -11,117 +11,117 @@ | |||
| RepositoryNotFoundError, | |||
| RevisionNotFoundError, | |||
| ) | |||
| from huggingface_hub.utils._errors import REPO_API_REGEX, hf_raise_for_status | |||
| from huggingface_hub.utils._http import REPO_API_REGEX, X_REQUEST_ID, _format, hf_raise_for_status | |||
There was a problem hiding this comment.
Should the tests be moved to https://github.com/huggingface/huggingface_hub/blob/main/tests/test_utils_http.py for consistency with the rest of the codebase?
| assert context.exception.response.status_code == 404 | ||
| assert "Request ID: 123" in str(context.exception) |
There was a problem hiding this comment.
Out of curiosity, why the change from self.assertEqual/self.assertIn to blank asserts with boolean statements? For consistency with new tests?
There was a problem hiding this comment.
Mostly for consistency yes. I'm only doing it when I'm adding tests next to existing ones (see related convo on slack -private-)
…gface_hub into refacto-error-parsing
|
Thanks for the review @LysandreJik! I addressed @osanseviero 's comment and resolved the merge conflicts. Failing CI is unrelated so I'm merging now. |
(sorry for long PR, most of it is moving logic around)
Done as part of #2069.
This PR updates how
HfHubHttpErroris formatted. With #2069, all errors are now defined insrc/huggingface_hub/errors.pywith no logic it the module. Before this PR, the logic to format the error message from a server response was split betweenHfHubHttpError.__init__andhf_raise_for_status. This should not have being split in the first place (harder to read/investigate)What does this PR do?
HfHubHttpError.__init__tohf_raise_for_status. This is the only way to instantiate aHfHubHttpErroranywayhf_raise_for_statusdefinition inside./utils/_http.pywhere it belongs instead of./utils/_errors.py./utils/_errors.pymodule to avoid confusion with./errors.pywhere all exceptions are definedrequest_id/server_messageas attributes. I'm pretty sure it's not used by anyone but at least it keeps backward compatibility.self.assertEquals...)In the end, sorry for the long diff because 95% of it is simply moving some logic around.