fix: increase timeouts in LMEval tests and fixtures#306
fix: increase timeouts in LMEval tests and fixtures#306dbasunag merged 1 commit intoopendatahub-io:mainfrom
Conversation
|
""" WalkthroughThe changes increase various timeout durations from 10 minutes to 20 minutes across test utilities, fixtures, and test cases related to model explainability evaluation. A new constant for a 20-minute timeout is introduced in the constants module, and this constant is used to update relevant wait operations throughout the test suite. Changes
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/model_explainability/lm_eval/utils.py (1)
28-28: Timeout increased from 10 to 20 minutes, but docstring still references the old timeout.The timeout value has been correctly increased to improve test stability, but the docstring at line 23 still references the old 10-minute timeout value.
- TimeoutError: If Pod doesn't reach Running state within 10 minutes + TimeoutError: If Pod doesn't reach Running state within 20 minutes
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/model_explainability/lm_eval/conftest.py(2 hunks)tests/model_explainability/lm_eval/test_lm_eval.py(3 hunks)tests/model_explainability/lm_eval/utils.py(1 hunks)utilities/constants.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
tests/model_explainability/lm_eval/utils.py (1)
utilities/constants.py (1)
Timeout(191-199)
tests/model_explainability/lm_eval/test_lm_eval.py (2)
tests/model_explainability/lm_eval/conftest.py (3)
lmevaljob_hf_pod(401-402)lmevaljob_vllm_emulator_pod(406-409)lmevaljob_s3_offline_pod(413-414)utilities/constants.py (1)
Timeout(191-199)
tests/model_explainability/lm_eval/conftest.py (1)
utilities/constants.py (1)
Timeout(191-199)
🔇 Additional comments (6)
utilities/constants.py (1)
199-199: Good addition of the new timeout constant.The addition of the
TIMEOUT_20MINconstant follows the existing pattern and is well-integrated with the other timeout constants.tests/model_explainability/lm_eval/test_lm_eval.py (3)
20-20: Timeout increased to match PR objectives.The increased timeout for waiting on the HuggingFace model job pod to complete is consistent with the PR's goal to reduce test flakiness.
93-94: Timeout increased to match PR objectives.The increased timeout for waiting on the vLLM emulator job pod to complete is consistent with the PR's goal to reduce test flakiness.
114-115: Timeout increased to match PR objectives.The increased timeout for waiting on the S3 storage job pod to complete is consistent with the PR's goal to reduce test flakiness.
tests/model_explainability/lm_eval/conftest.py (2)
207-207: Timeout increased to match PR objectives.The increased timeout for the data downloader pod to reach SUCCEEDED status is consistent with the PR's goal to reduce test flakiness.
321-321: Timeout increased to match PR objectives.The increased timeout for the MinIO deployment to reach the desired replica count is consistent with the PR's goal to reduce test flakiness.
|
The following are automatically added/executed:
Available user actions:
Supported labels{'/hold', '/lgtm', '/verified', '/wip'} |
|
/verified |
adolfo-ab
force-pushed
the
lmeval-timeouts
branch
from
0d8dc2b to
82970e2
Compare
|
/verified |
rhods-ci-bot
added
Verified
|
Status of building tag latest: success. |
4 participants
Increases the timeouts in LMEval tests and fixtures
Description
The 10 minutes timeout used is sometimes not enough, therefore increasing it significantly to avoid flakiness
How Has This Been Tested?
Running the tests in a working cluster
Merge criteria:
Summary by CodeRabbit