[Tests] Disable retries and use context manager for openai client #7565
Conversation
|
👋 Hi! Thank you for contributing to the vLLM project. Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge). To run full CI, you can do one of these:
🚀 |
|
The guided decoding tests have been shown to be fragile (see #5526 (comment)). Seems like changing the usage of OpenAI client in this way breaks them as well. |
|
Thanks @DarkLight1337. I was digging into the cause of the failures here but got diverted, going to resume that now :) |
The openai python client by default retries failed requests up to two times. In our tests I think we should disable this to avoid hiding issues. Doing this actually caused some other failures, which seem to be related to how a single client fixture is shared between multiple async tests. The openai docs suggest it should be used via a context manager so I have updated the various usages to do so and have reduced the scope of associated client fixtures.
njhill
force-pushed
the
oai-client-retries
branch
from
e562aba
to
8d21009
Compare
The openai python client by default retries failed requests up to two times. In our tests I think we should disable this to avoid hiding issues.
Doing this actually caused some failures (at least with another PR I'm working on), which seem to be related to how a single client fixture is shared between multiple async tests.
The openai docs suggest it should be used via a context manager so I have updated the various usages to do so and have reduced the scope of associated client fixtures.