[Serve LLM] Fix enable_log_requests=False not being forwarded to vLLM AsyncLLM#60824
[Serve LLM] Fix enable_log_requests=False not being forwarded to vLLM AsyncLLM#60824kouroshHakha wants to merge 4 commits intoray-project:masterfrom
enable_log_requests=False not being forwarded to vLLM AsyncLLM#60824Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces two important logging-related fixes. The primary change correctly forwards the enable_log_requests argument to the vLLM engine, allowing users to suppress per-request logs as intended. This also includes a cleanup to remove a deprecated compatibility shim for disable_log_requests. Additionally, a secondary fix ensures that setting enable_access_log=False in Ray Serve's logging configuration correctly suppresses access logs in stderr, not just in log files. A new regression test has been added to verify this behavior. The changes are well-implemented and improve the logging control and consistency. The code looks good.
| stream_handler.setFormatter(serve_formatter) | ||
| stream_handler.addFilter(log_to_stderr_filter) | ||
| stream_handler.addFilter(ServeContextFilter()) | ||
| if logging_config.enable_access_log is False: |
There was a problem hiding this comment.
Is this meant for this PR?
Summary
Forward
enable_log_requestsfrom engine args to theAsyncLLMconstructor so thatenable_log_requests=Falseactually suppresses per-request log messages.Pre-requisit:
Reproduction
Expected: No
Added request cmpl-...lines in stderr.Actual:
Root Cause
In
_start_async_llm_engine()(vllm_engine.py),log_requestsis never passed toAsyncLLM, so it defaults toTrue:Fix
Also removes the deprecated
disable_log_requestscompat shim invllm_models.pysince vLLM has fully dropped that flag.