Skip to content

Conversation

@mattf
Copy link
Collaborator

@mattf mattf commented Sep 14, 2025

What does this PR do?

the @required_args decorator in openai-python is masking the async nature of the {AsyncCompletions,chat.AsyncCompletions}.create method. see openai/openai-python#996

this means two things -

  1. we cannot use iscoroutine in the recorder to detect async vs non
  2. our mocks are inappropriately introducing identifiable async

for (0), we update the iscoroutine check w/ detection of /v1/models, which is the only non-async function we mock & record.

for (1), we could leave everything as is and assume (0) will catch errors. to be defensive, we update the unit tests to mock below create methods, allowing the true openai-python create() methods to be tested.

the @required_args decorator in openai-python is masking the async nature
of the {AsyncCompletions,chat.AsyncCompletions}.create method.
see openai/openai-python#996

this means two things -
 0. we cannot use iscoroutine in the recorder to detect async vs non
 1. our mocks are inappropriately introducing identifiable async

for (0), we update the iscoroutine check w/ detection of /v1/models,
which is the only non-async function we mock & record.

for (1), we could leave everything as is and assume (0) will catch errors.
to be defensive, we update the unit tests to mock below create methods,
allowing the true openai-python create() methods to be tested.
@meta-cla meta-cla bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 14, 2025
@mattf
Copy link
Collaborator Author

mattf commented Sep 14, 2025

@raghotham i found a bug in #3426. our mocks were masking a bug in openai-python.

@derekhiggins
Copy link
Contributor

With this PR I'm still having problems using models.list with the vllm provider, see logs
https://gist.github.com/derekhiggins/6a4a4a3b6121b7139bb029b893faf00f

The only thing I've managed to get consistently working is Special handling for models.list (AsyncIterableModelsWrapper in #3128) which supports both Direct iteration and await then iterate

@mattf
Copy link
Collaborator Author

mattf commented Sep 15, 2025

With this PR I'm still having problems using models.list with the vllm provider, see logs

https://gist.github.com/derekhiggins/6a4a4a3b6121b7139bb029b893faf00f

The only thing I've managed to get consistently working is Special handling for models.list (AsyncIterableModelsWrapper in #3128) which supports both Direct iteration and await then iterate

ok, once this is in i'll check on that.

@mattf mattf merged commit 01bdcce into llamastack:main Sep 15, 2025
24 of 25 checks passed
iamemilio pushed a commit to iamemilio/llama-stack that referenced this pull request Sep 24, 2025
…lamastack#3442)

# What does this PR do?

the @required_args decorator in openai-python is masking the async
nature of the {AsyncCompletions,chat.AsyncCompletions}.create method.
see openai/openai-python#996

this means two things -

 0. we cannot use iscoroutine in the recorder to detect async vs non
 1. our mocks are inappropriately introducing identifiable async

for (0), we update the iscoroutine check w/ detection of /v1/models,
which is the only non-async function we mock & record.

for (1), we could leave everything as is and assume (0) will catch
errors. to be defensive, we update the unit tests to mock below create
methods, allowing the true openai-python create() methods to be tested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants