Skip to content
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

http_backoff retry with SliceFileObj #2542

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

hlky
Copy link
Contributor

@hlky hlky commented Sep 14, 2024

Retry mechanism of http_backoff doesn't work from lfs_upload->_upload_multi_part->_upload_parts_iteratively path, data is consumed after the first attempt when using SliceFileObj because io_obj_initial_pos should be set and reset on retry, io_obj_initial_pos is not set due to isinstance(kwargs["data"], io.IOBase).

This PR changes the check to isinstance(kwargs["data"], (io.IOBase, SliceFileObj)) and moves SliceFileObj to utils._lfs to avoid circular import:

huggingface_hub> python utils/check_inference_input_params.py
Error importing huggingface_hub.inference._client: cannot import name 'fix_hf_endpoint_in_url' from partially initialized module 'huggingface_hub.utils' (most likely due to a circular import) (huggingface_hub\src\huggingface_hub\utils\__init__.py)
Traceback (most recent call last):
  File "huggingface_hub\utils\check_inference_input_params.py", line 28, in <module>
    from huggingface_hub import InferenceClient
  File "huggingface_hub\src\huggingface_hub\__init__.py", line 530, in __getattr__
    submod = importlib.import_module(submod_path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "Python311\Lib\importlib\__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "huggingface_hub\src\huggingface_hub\inference\_client.py", line 57, in <module>
    from huggingface_hub.inference._common import (
  File "huggingface_hub\src\huggingface_hub\inference\_common.py", line 53, in <module>
    from ..utils import (
  File "huggingface_hub\src\huggingface_hub\utils\__init__.py", line 54, in <module>
    from ._http import (
  File "huggingface_hub\src\huggingface_hub\utils\_http.py", line 33, in <module>
    from huggingface_hub.lfs import SliceFileObj
  File "huggingface_hub\src\huggingface_hub\lfs.py", line 32, in <module>
    from .utils import (
ImportError: cannot import name 'fix_hf_endpoint_in_url' from partially initialized module 'huggingface_hub.utils' (most likely due to a circular import) (huggingface_hub\src\huggingface_hub\utils\__init__.py)

Usage of SliceFileObj in other areas is updated to import from utils._lfs.

Closes #2539

Copy link
Contributor

@Wauplin Wauplin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very sensible fix! Thanks for tracking that down @hlky and suggesting a PR. It's ready to merge "as is" as soon as CI passes :)

EDIT: next time if we again have an issue with this, we could use a Protocol + runtime_checkable to check if .seek() and .tell() exist. (Never used this solution before and still think an explicit ref to SliceFileObj is better, just mentioning it for curiosity)

@Wauplin
Copy link
Contributor

Wauplin commented Sep 16, 2024

Failing tests are unrelated so let's merge this PR :)

@Wauplin Wauplin merged commit c9458ad into huggingface:main Sep 16, 2024
13 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants