Skip to content

Unit Test: Add error handling for rate limit exceeded in model list - #5715

Merged
loadams merged 5 commits into
masterfrom
HeyangQin/fix_unit_test_http
Jul 11, 2024
Merged

loadams merged 5 commits into
masterfrom
HeyangQin/fix_unit_test_http

Conversation

@HeyangQin

Copy link
Copy Markdown
Contributor

This PR fixes the random failure in our unit test due to HTTP 429

@HeyangQin
HeyangQin requested a review from tohtana July 1, 2024 23:05
@HeyangQin
HeyangQin enabled auto-merge July 1, 2024 23:07
@loadams
loadams disabled auto-merge July 2, 2024 17:02
@loadams
loadams enabled auto-merge July 2, 2024 17:02
@adk9

adk9 commented Jul 2, 2024

Copy link
Copy Markdown
Contributor

Thanks for the fix. I think this will definitely make test_inference less flaky.

I've two high-level concerns though about what's happening here:

  1. What's the reason for the default model cache time to be 1 day? The models/tasks we're testing barely update so we could perhaps set a much longer cache time?
  2. We get rate-limited because we're making ~750k requests (getting model information for all models on HF) to test only 24 models. I think a better approach is to pass a filter to api.list_models() to get information of only the models we're interested in testing?

@loadams

loadams commented Jul 2, 2024

Copy link
Copy Markdown
Collaborator

Thanks for the fix. I think this will definitely make test_inference less flaky.

I've two high-level concerns though about what's happening here:

  1. What's the reason for the default model cache time to be 1 day? The models/tasks we're testing barely update so we could perhaps set a much longer cache time?
  2. We get rate-limited because we're making ~750k requests (getting model information for all models on HF) to test only 24 models. I think a better approach is to pass a filter to api.list_models() to get information of only the models we're interested in testing?

It was re-set here, though we had it set to 1 day before this: https://github.com/microsoft/DeepSpeed/pull/5688/files

@loadams
loadams disabled auto-merge July 2, 2024 17:13
@adk9
adk9 self-requested a review July 9, 2024 17:22
@loadams
loadams merged commit 83aa184 into master Jul 11, 2024
@adk9
adk9 deleted the HeyangQin/fix_unit_test_http branch July 11, 2024 16:49
mauryaavinash95 pushed a commit to DataStates/DeepSpeed that referenced this pull request Mar 20, 2025
…eepspeedai#5715)

This PR fixes the random failure in our unit test due to HTTP 429

---------

Co-authored-by: Logan Adams <114770087+loadams@users.noreply.github.com>
delock pushed a commit that referenced this pull request Jun 8, 2026
The `cpu-torch-latest / unit tests` lane can fail with Hugging Face Hub
HTTP 429 errors during test collection/setup, before it reaches
PR-specific assertions
([example](https://github.com/deepspeedai/DeepSpeed/actions/runs/27073376531/job/79997387234?pr=8038)).
This is existing test flakiness rather. DeepSpeed has already had
similar 429 failures in the inference tests (#5715), and the nested ZeRO
init test also relies on a remote Hugging Face tiny model path added in
#3886.

This PR removes live Hub access from those unit-test setup paths. It
encodes the valid inference model/task combinations locally instead of
calling `HfApi().list_models(...)`, and it creates a tiny local
`VisionEncoderDecoderModel` artifact so the test can still exercise
`from_pretrained(...)` without downloading from the Hub.

Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
nathon-lee pushed a commit to nathon-lee/DeepSpeed_woo that referenced this pull request Jul 1, 2026
The `cpu-torch-latest / unit tests` lane can fail with Hugging Face Hub
HTTP 429 errors during test collection/setup, before it reaches
PR-specific assertions
([example](https://github.com/deepspeedai/DeepSpeed/actions/runs/27073376531/job/79997387234?pr=8038)).
This is existing test flakiness rather. DeepSpeed has already had
similar 429 failures in the inference tests (deepspeedai#5715), and the nested ZeRO
init test also relies on a remote Hugging Face tiny model path added in
deepspeedai#3886.

This PR removes live Hub access from those unit-test setup paths. It
encodes the valid inference model/task combinations locally instead of
calling `HfApi().list_models(...)`, and it creates a tiny local
`VisionEncoderDecoderModel` artifact so the test can still exercise
`from_pretrained(...)` without downloading from the Hub.

Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
Signed-off-by: nathon-lee <leejianwoo@gmail.com>
nathon-lee pushed a commit to nathon-lee/DeepSpeed_woo that referenced this pull request Jul 1, 2026
The `cpu-torch-latest / unit tests` lane can fail with Hugging Face Hub
HTTP 429 errors during test collection/setup, before it reaches
PR-specific assertions
([example](https://github.com/deepspeedai/DeepSpeed/actions/runs/27073376531/job/79997387234?pr=8038)).
This is existing test flakiness rather. DeepSpeed has already had
similar 429 failures in the inference tests (deepspeedai#5715), and the nested ZeRO
init test also relies on a remote Hugging Face tiny model path added in
deepspeedai#3886.

This PR removes live Hub access from those unit-test setup paths. It
encodes the valid inference model/task combinations locally instead of
calling `HfApi().list_models(...)`, and it creates a tiny local
`VisionEncoderDecoderModel` artifact so the test can still exercise
`from_pretrained(...)` without downloading from the Hub.

Signed-off-by: Masahiro Tanaka <mtanaka@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants