-
Notifications
You must be signed in to change notification settings - Fork 315
Exempt Dynamo vLLM/SGLang from sign-off engine-first ordering / 签核引擎优先顺序规则豁免 Dynamo vLLM/SGLang #3667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Exempt Dynamo vLLM/SGLang from sign-off engine-first ordering / 签核引擎优先顺序规则豁免 Dynamo vLLM/SGLang #3667
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 (optional) A future llm-d vLLM master-config entry (
framework: llmd-vllm, documented in inferencex-e2e/docs/configuration-procedures.md:220 as the master-entry name for llm-d recipes) will FAIL Check 6(b) as a vendor framework requiring a prior open-source engine entry, even though it is a plain vLLM deployment like dynamo-vllm. The new open-source engine list at line 242 only namesvllm,dynamo-vllm,sglang,sglang-disagg,dynamo-sglang, sollmd-vllmfalls into "every other engine" (line 244) by omission, wrongly blocking it the same way dynamo-sglang was wrongly blocked before this PR. Fix: enumerate every vLLM/SGLang deployment wrapper (including llmd-vllm) as open-source, or match by engine prefix (vllm*/sglang*) instead of a fixed list.Why this was flagged
Trigger: a PR adds/updates a
framework: llmd-vllmentry in inferencex-e2e/configs/*-master.yaml (the documented path per inferencex-e2e/docs/configuration-procedures.md:220, 'Add/update thellmd-vllmmaster entry'), with no prior vllm/sglang entry for the same model-prefix/runner. The verifier's Check 6(b) (lines 239-253 of .github/codeowner-signoff-verify-prompt.md) classifies anything not in the open-source list at line 242 as vendor-specific ('every other engine', line 244), so it FAILs this legitimate open-source vLLM deployment and demands a sign-off exception that doesn't apply. No other part of the prompt lists llmd-vllm as exempt. This is the same bug class the PR itself was written to fix for dynamo-sglang (#3629), left open for the llm-d deployment path.Verification: Nit, latent gap, near pre-existing. The new Check 6(b) open-source engine list at .github/codeowner-signoff-verify-prompt.md:242 lists only vllm, dynamo-vllm, sglang, sglang-disagg, dynamo-sglang, and line 244 classifies "every other engine" as vendor-specific.