8000 Increase request timeout for tests that time out by albertvillanova · Pull Request #972 · huggingface/smolagents · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Increase request timeout for tests that time out #972

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Apr 1, 2025
Merged

Increase request timeout for tests that time out #972

merged 4 commits into from
Apr 1, 2025

Conversation

albertvillanova
Copy link
Member
@albertvillanova albertvillanova commented Mar 13, 2025 •
edited
Loading

Increase request timeout for tests that often time out.

Require RUN_ALL for test that often times out (443 error code): https://github.com/huggingface/smolagents/actions/runs/13842480645/job/38733074899

FAILED tests/test_models.py::ModelTests::test_transformers_message_no_tool - ValueError: Failed to load tokenizer and model for model_id='HuggingFaceTB/SmolLM2-135M-Instruct': (ReadTimeoutError("HTTPSConnectionPool(host='huggingface.co', port=443): Read timed out. (read timeout=10)"), '(Request ID: c31c8db6-d17c-419c-aebf-53d5ab7fabd0)')

Also: https://github.com/huggingface/smolagents/actions/runs/13842766023/job/38734041267?pr=972

FAILED tests/test_models.py::ModelTests::test_transformers_message_vl_no_tool - ValueError: Failed to load tokenizer and model for model_id='llava-hf/llava-interleave-qwen-0.5b-hf': We couldn't connect to 'https://huggingface.co/' to load this file, couldn't find it in the cached files and it looks like llava-hf/llava-interleave-qwen-0.5b-hf is not the path to a directory containing a file named config.json.
Checkout your internet connection or see how to run the library in offline mode at 'https://huggingface.co/docs/transformers/installation#offline-mode'.

@aymeric-roucher
Copy link
Collaborator

Good point to try and solve these tests!

Couldn't we mostly solve the timeouts by increasing the timeout counter, say from 10 to 60 seconds?

@albertvillanova
Copy link
Member Author

We could do that as well.
I just wanted to reduce the number of tests that make API calls (network traffic) that run at every PR.
Maybe we could add some variants but patching the network request?

@aymeric-roucher
Copy link
Collaborator

Yes I think we could have a hybrid approach with

  • Some variants require RUN_ALL to reduce the count of API calls$
  • One of them still runs each time, with a patched request for longer timeout

@albertvillanova
Copy link
Member Author

I have checked that currently huggingface-hub already has a retry mechanism in place when ReadTimeout happens: https://github.com/huggingface/huggingface_hub/blob/6378820ebb03f071988a96c7f3268f5bdf8f9449/src/huggingface_hub/file_download.py#L328-L330

If ConnectionError (SSLError) or ReadTimeout happen while streaming data from the server, it is most likely a
transient error (network outage?). We log a warning message and try to resume the download a few times before
giving up. The method gives up after 5 attempts if no new data has being received from the server.

I think in this case, instead of retry, we should just skip the test if the ReadTimeout happens.

@albertvillanova
Copy link
Member Author

After investigation, I think the error was raised before entering the huggingface-hub retry mechanism.

Therefore, I increased the timeout instead: from 10 to 30.

@albertvillanova albertvillanova changed the title Require RUN_ALL for test that times out Increase request timeout for tests that time out Apr 1, 2025
@albertvillanova albertvillanova merged commit ef39c6e into huggingface:main Apr 1, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aymeric-roucher aymeric-roucher aymeric-roucher approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@albertvillanova @aymeric-roucher
0