Skip to content

Conversation

@zhenga1
Copy link
Contributor

@zhenga1 zhenga1 commented Jan 15, 2026

OpenAIServingModels from VLLM has different signatures depending on version

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request addresses a compatibility issue with newer versions of the VLLM library by dynamically checking the signature of OpenAIServingModels before instantiation. This is a good approach to maintain compatibility across library versions. My review includes a suggestion to make this version check more robust by inspecting parameter names instead of the parameter count, and also includes some minor code cleanup.

Comment on lines +388 to +399
from inspect import signature
sig = signature(OpenAIServingModels)
base_model_paths = [BaseModelPath(name=model_name, model_path=model_path)]
models = OpenAIServingModels(engine, model_config, base_model_paths)
if len(sig.parameters) == 4:
# For OLD vllm (<0.11.2)
models = OpenAIServingModels(engine, model_config, base_model_paths)
else:
# For NEW vllm (>= 0.11.2)
models = OpenAIServingModels(engine, base_model_paths=base_model_paths)#model_config)



Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This is a good way to handle different versions of vllm. However, checking the number of parameters can be brittle if future versions add new optional parameters. A more robust approach would be to check for the existence of the model_config parameter in the signature. I've also cleaned up the commented-out code and removed the extra blank lines for better readability.

Suggested change
from inspect import signature
sig = signature(OpenAIServingModels)
base_model_paths = [BaseModelPath(name=model_name, model_path=model_path)]
models = OpenAIServingModels(engine, model_config, base_model_paths)
if len(sig.parameters) == 4:
# For OLD vllm (<0.11.2)
models = OpenAIServingModels(engine, model_config, base_model_paths)
else:
# For NEW vllm (>= 0.11.2)
models = OpenAIServingModels(engine, base_model_paths=base_model_paths)#model_config)
from inspect import signature
sig = signature(OpenAIServingModels)
base_model_paths = [BaseModelPath(name=model_name, model_path=model_path)]
if 'model_config' in sig.parameters:
# For OLD vllm versions that have the 'model_config' parameter
models = OpenAIServingModels(engine, model_config, base_model_paths)
else:
# For NEW vllm versions
models = OpenAIServingModels(engine, base_model_paths=base_model_paths)

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.

1 participant