Skip to content

[cli] fix: detect model-path from --config in sglang serve - #36107

Open
clayrosenthal wants to merge 1 commit into
sgl-project:mainfrom
clayrosenthal:serve-config-yaml-fix
Open

[cli] fix: detect model-path from --config in sglang serve#36107
clayrosenthal wants to merge 1 commit into
sgl-project:mainfrom
clayrosenthal:serve-config-yaml-fix

Conversation

@clayrosenthal

@clayrosenthal clayrosenthal commented Aug 23, 2026

Copy link
Copy Markdown

Motivation

Fixes #36105

Running sglang serve --config my_config.yaml fails with Error: --model-path is required even when the YAML config contains a model-path key, while python -m sglang.launch_server --config my_config.yaml works fine. This is because try_get_model_path() in python/sglang/cli/utils.py only scanned the raw CLI argv for --model-path/--model and ignored values supplied through --config, so backend auto-detection was skipped and get_model_path() raised.

Modifications

  • Added _try_get_model_path_from_config() in python/sglang/cli/utils.py: a best-effort helper that reads the --config <file> YAML and returns the value of a model-path / model_path / model key. It mirrors ConfigArgumentMerger semantics (space-form --config only, key normalization, last-in-file wins) and never raises — missing/unreadable/malformed files, non-dict roots, and non-string keys or empty/non-string values all return None, so the authoritative parser in prepare_server_args remains the source of truth.
  • try_get_model_path() now falls back to this helper only when no explicit --model-path/--model CLI flag is present, preserving CLI > config > defaults precedence.
  • Added import yaml (PyYAML is already a hard dependency).

Accuracy Tests

  • New test/registered/unit/cli/test_cli_utils.py (17 tests, registered in base-a-test-cpu): config-key detection (model-path/model_path/model), CLI-flag precedence, nonexistent/empty/malformed/non-dict configs, non-string keys, falsy/non-string values, last-key-wins, and CLI flag sanity (--model-path X, --model X, --model=X). All pass.
  • Existing test/registered/unit/cli/test_serve_backends.py (12 tests) still passes, including test_explicit_backend_can_support_config_only_requests (missing pipeline.yamlmodel_path is None, never raises).

Speed Tests and Profiling

N/A — pure CLI argument-parsing helper, best-effort, not on any serving hot path.

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ❌ Run #32662327838
Latest PR Test (Extra): ❌ Run #32662327718
Latest PR Test (AMD ROCm 7.2): ❌ Run #32662327869

@clayrosenthal

Copy link
Copy Markdown
Author

@slin1237 Could you take a look at this? Thank you

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.

[Bug] --model-path required when --config is passed

1 participant