Skip to content

fix: map disable_log_requests to --no-enable-log-requests for newer vLLM#779

Open
lilyz-ai wants to merge 1 commit intomainfrom
lilyzhu/fix-vllm-disable-log-requests
Open

fix: map disable_log_requests to --no-enable-log-requests for newer vLLM#779
lilyz-ai wants to merge 1 commit intomainfrom
lilyzhu/fix-vllm-disable-log-requests

Conversation

@lilyz-ai
Copy link
Collaborator

@lilyz-ai lilyz-ai commented Mar 11, 2026

Summary

  • Newer vLLM versions removed --disable-log-requests in favor of --no-enable-log-requests
  • Add a VLLM_FLAG_RENAMES dict in the vLLM command builder to translate DTO field names to their current CLI flag equivalents
  • This fixes pods crashing on startup when sensitive_log_mode: true (prod) triggers disable_log_requests

Test plan

  • Verify endpoint pods start successfully with the new flag
  • Confirm --no-enable-log-requests appears in the generated vLLM command

🤖 Generated with Claude Code

Greptile Summary

This PR fixes a pod startup crash in production by mapping the deprecated --disable-log-requests vLLM CLI flag to its modern equivalent --no-enable-log-requests, which is triggered whenever sensitive_log_mode is enabled. The fix introduces a VLLM_FLAG_RENAMES dictionary in the vLLM command builder that translates DTO field names to their current CLI flag equivalents, providing an extensible mechanism for handling future vLLM flag renames.

Key changes:

  • Adds a VLLM_FLAG_RENAMES dict (local to the command-building block) mapping disable_log_requestsno-enable-log-requests
  • Updates the flag generation logic to use this map before falling back to the default field.replace("_", "-") transformation
  • No tests were added to cover the new rename logic or the generated command string
  • disable_sliding_window (another bool field in VLLMModelConfig) may be subject to an analogous rename in newer vLLM and could warrant a proactive entry in the rename map

Confidence Score: 4/5

  • This PR is safe to merge — it is a targeted, minimal fix for a production crash with correct logic and no regressions for unaffected fields.
  • The rename mapping is semantically correct: disable_log_requests=True now emits --no-enable-log-requests (a boolean presence flag) rather than the removed --disable-log-requests, and the fallback path for all other fields is unchanged. The only concerns are a minor naming convention issue with the local VLLM_FLAG_RENAMES variable and the absence of tests, neither of which affects correctness.
  • No files require special attention beyond the single changed file.

Important Files Changed

Filename Overview
model-engine/model_engine_server/domain/use_cases/llm_model_endpoint_use_cases.py Adds a VLLM_FLAG_RENAMES dict to translate DTO field names to renamed vLLM CLI flags, mapping disable_log_requestsno-enable-log-requests to fix pod startup crashes in newer vLLM versions when sensitive_log_mode is enabled. The logic is correct but the constant is defined as a local variable using ALL_CAPS naming (typically reserved for module-level constants in Python), and disable_sliding_window may need similar treatment.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Iterate VLLMEndpointAdditionalArgs fields] --> B{config_value is not None?}
    B -- No --> A
    B -- Yes --> C{field == chat_template?}
    C -- Yes --> D[Encode template as base64\nexport CHAT_TEMPLATE env var]
    D --> E
    C -- No --> E[cli_flag = VLLM_FLAG_RENAMES.get field\nor field.replace _ -]
    E --> F{isinstance config_value bool?}
    F -- Yes --> G{config_value is True?}
    G -- Yes --> H[append --cli_flag]
    G -- No --> A
    F -- No --> I[append --cli_flag config_value]
    H --> A
    I --> A

    style E fill:#d4f1d4,stroke:#2e7d32
    style H fill:#d4f1d4,stroke:#2e7d32

    subgraph VLLM_FLAG_RENAMES
        direction LR
        R1["disable_log_requests → no-enable-log-requests"]
    end
Loading

Last reviewed commit: 5d246da

Newer vLLM versions removed --disable-log-requests in favor of
--no-enable-log-requests. Add a VLLM_FLAG_RENAMES mapping so the
command builder translates DTO field names to their current CLI flags.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lilyz-ai lilyz-ai requested a review from dmchoiboi March 11, 2026 18:12
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