[cli] fix: detect model-path from --config in sglang serve - #36107
Open
clayrosenthal wants to merge 1 commit into
Open
[cli] fix: detect model-path from --config in sglang serve#36107clayrosenthal wants to merge 1 commit into
clayrosenthal wants to merge 1 commit into
Conversation
Author
|
@slin1237 Could you take a look at this? Thank you |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes #36105
Running
sglang serve --config my_config.yamlfails withError: --model-path is requiredeven when the YAML config contains amodel-pathkey, whilepython -m sglang.launch_server --config my_config.yamlworks fine. This is becausetry_get_model_path()inpython/sglang/cli/utils.pyonly scanned the raw CLI argv for--model-path/--modeland ignored values supplied through--config, so backend auto-detection was skipped andget_model_path()raised.Modifications
_try_get_model_path_from_config()inpython/sglang/cli/utils.py: a best-effort helper that reads the--config <file>YAML and returns the value of amodel-path/model_path/modelkey. It mirrorsConfigArgumentMergersemantics (space-form--configonly, 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 returnNone, so the authoritative parser inprepare_server_argsremains the source of truth.try_get_model_path()now falls back to this helper only when no explicit--model-path/--modelCLI flag is present, preserving CLI > config > defaults precedence.import yaml(PyYAML is already a hard dependency).Accuracy Tests
test/registered/unit/cli/test_cli_utils.py(17 tests, registered inbase-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.test/registered/unit/cli/test_serve_backends.py(12 tests) still passes, includingtest_explicit_backend_can_support_config_only_requests(missingpipeline.yaml→model_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
test/registered/unit/cli/test_cli_utils.py)Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): ❌ Run #32662327838
Latest PR Test (Extra): ❌ Run #32662327718
Latest PR Test (AMD ROCm 7.2): ❌ Run #32662327869