[Serve][LLM] Add regression test for nested dict in engine_kwargs#60430
[Serve][LLM] Add regression test for nested dict in engine_kwargs#60430kouroshHakha merged 9 commits intoray-project:masterfrom
Conversation
Add a GPU test that verifies vLLM engine can start with nested dicts in engine_kwargs (e.g., structured_outputs_config). This test will fail without the fix from PR ray-project#60380 with: AttributeError: 'dict' object has no attribute 'backend' The test ensures that nested dicts are properly converted to argparse.Namespace objects so vLLM's init_app_state() can access nested attributes via dot notation. See: ray-project#60380 Signed-off-by: Seiji Eicher <seiji@anyscale.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a valuable regression test to verify that vLLM engines can be initialized with nested dictionaries in engine_kwargs. The test case is well-defined and directly addresses the issue described. I have a couple of suggestions to improve the test's robustness regarding resource cleanup and to address a potential issue with a method call that might not exist.
python/ray/llm/tests/serve/gpu/deployments/llm/vllm/test_nested_engine_kwargs.py
Outdated
Show resolved
Hide resolved
python/ray/llm/tests/serve/gpu/deployments/llm/vllm/test_nested_engine_kwargs.py
Outdated
Show resolved
Hide resolved
Move regression test for nested dict in engine_kwargs from GPU tests to release/llm_tests/serve/test_llm_serve_integration.py where it properly exercises the full vLLM integration path. Signed-off-by: Seiji Eicher <seiji@anyscale.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
python/ray/llm/tests/serve/gpu/deployments/llm/vllm/test_nested_engine_kwargs.py
Outdated
Show resolved
Hide resolved
python/ray/llm/tests/serve/gpu/deployments/llm/vllm/test_nested_engine_kwargs.py
Outdated
Show resolved
Hide resolved
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
When users pass dicts for vLLM config fields like structured_outputs_config,
compilation_config, etc. in engine_kwargs, they need to be converted to
their proper dataclass types (not argparse.Namespace) so that default
field values are populated correctly.
Without this fix, passing `structured_outputs_config={"backend": "xgrammar"}`
causes AttributeError when vLLM tries to access default fields like
`reasoning_parser` that don't exist on the Namespace object.
The fix uses typing.get_type_hints() to inspect AsyncEngineArgs and
automatically convert dict values to their expected dataclass types.
This handles all current and future config dataclass fields generically.
Signed-off-by: Seiji Eicher <seiji@anyscale.com>
…seiji/ray into test-namespace-conversion Signed-off-by: Seiji Eicher <seiji@anyscale.com> # Conflicts: # python/ray/llm/_internal/serve/engines/vllm/vllm_engine.py
|
Release test: https://buildkite.com/ray-project/release/builds/77110 |
ray-gardener
bot
added
serve
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the regression issue by introducing a robust mechanism to convert nested dictionaries within engine_kwargs into their appropriate vLLM dataclass types. The _convert_config_dicts function correctly handles optional types and gracefully logs warnings for conversion failures, ensuring the system remains resilient. The addition of a dedicated regression test (test_nested_engine_kwargs_structured_outputs) provides excellent coverage and verifies the fix's effectiveness. Overall, the changes are well-implemented, maintainable, and directly resolve the identified problem.
|
The release test failure |
|
Fixing here #60507 |
kouroshHakha
changed the title
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com> Signed-off-by: jinbum-kim <jinbum9958@gmail.com>
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com>
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com> Signed-off-by: 400Ping <jiekaichang@apache.org>
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com> Signed-off-by: Adel Nour <ans9868@nyu.edu>
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
…y-project#60430) Signed-off-by: Seiji Eicher <seiji@anyscale.com> Signed-off-by: peterxcli <peterxcli@gmail.com>
2 participants
Summary
Add a release integration test that verifies vLLM engine can start with nested dicts in
engine_kwargs(e.g.,structured_outputs_config).This test serves as a regression test for #60380. Without that fix, this test fails with:
The test ensures that nested dicts are properly converted to
argparse.Namespaceobjects so vLLM'sinit_app_state()can access nested attributes via dot notation.Test plan
pytest release/llm_tests/serve/test_llm_serve_integration.py::test_nested_engine_kwargs_structured_outputs -vRelated: #60380