Repository navigation
docs: The Rust vLLM servicer switch needs no vLLM change: self-installing plugin and vllm serve --servicer-impl - #115
Merged
Conversation
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
slin1237
marked this pull request as ready for review
October 7, 2026 11:07
slin1237
requested review from
CatherineSue,
key4ng and
slin1237
as code owners
October 7, 2026 11:07
This was referenced Oct 8, 2026
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.
What this PR does
Does selecting the Rust vLLM gRPC servicer still require a vLLM whose gRPC entrypoint carries the smg hook, and can the implementation be chosen on vLLM's own command line (vllm serve --grpc --servicer-impl rust) instead of only through SMG_VLLM_SERVICER_IMPL?
Why we need it
Source change: smg-project/smg@50ba16f
Commit 26 (#2755, 50ba16f7, 2026-10-04) removed the need for a vLLM-side hook, one day after merged docs PR #105 (which documented the hook as a requirement). Verified on current main: grpc_servicer/smg_grpc_servicer/vllm/launcher_switch.py installs the servicer switch over vLLM's serve_grpc from the launcher's own module-level import of this package's servicer classes (install_launcher_switch is called from health_servicer.py at import, plus a meta-path finder for late-loading launchers); grpc_servicer/pyproject.toml registers a vllm.general_plugins entry point (smg-servicer = smg_grpc_servicer.vllm.plugin:register), and plugin.py adds --servicer-impl {python,rust} to every FlexibleArgumentParser that defines --grpc (the vllm serve parser), with resolve_servicer_impl (rust.py:343-355) reading args.servicer_impl ahead of SMG_VLLM_SERVICER_IMPL and writing the decision back to the environment so require_python_impl agrees; plugin.py's module doc notes a VLLM_PLUGINS setting that leaves the plugin out only removes the flag — the env var works regardless. rust.py:upstream_hook_installed (def at line 380) now returns true when the launcher imports this package's servicer modules at module level OR carries the check in its own serve_grpc, and require_upstream_hook (lines 470-479) fails only when neither holds; require_python_impl (lines 364-377) names the one unswitchable form — "a launcher executed directly as main, or one that did not import smg_grpc_servicer.vllm.servicer before defining serve_grpc". The commit's grpc_servicer/README.md hunk states "it needs no vLLM change" and shows vllm serve Qwen/Qwen3-0.6B --grpc --port 50051 --servicer-impl rust. Stale docs, all in src/lib/content/getting-started/grpc-workers.md's Rust vLLM Servicer section (written by merged PR #105 on 2026-10-03, before this commit): line 107 "The switch is the SMG_VLLM_SERVICER_IMPL environment variable, read by upstream vLLM's gRPC entrypoint before it builds an engine" (the entrypoint no longer consults anything; the package installs the switch itself, and a --servicer-impl flag now also exists on vllm serve --grpc); line 133's requirement "a vLLM whose gRPC entrypoint consults the switch (smg_grpc_servicer.vllm.resolve_servicer_impl)" (false on main — any vLLM whose gRPC launcher imports the package's servicer classes, the stock shape, is switched unchanged); line 135 "the installed vLLM lacks the hook" (the fail-fast check now verifies the launcher will be switched, and the Python servicer's refusal covers the launcher-run-as-main case). getting-started/index.md:170 and :298 were read and stay accurate (the smg serve flag and env-var selection are unchanged), so the edit is scoped to grpc-workers.md. Version caveat: the commit landed after the v1.11.0 bump (commit 69, #2670, 2026-09-24); the section already frames the whole feature as newer than v1.11.0, so describe the self-installing switch as current-main behavior without naming a release. This is an engine-compatibility concern (which vLLM installations can serve the Rust servicer and what fails loudly), distinct from merged PR #105's smg serve selection surface, which it corrects from a newer primary commit; one independent concern: one commit changed one delivery mechanism for one switch, corrected in the one section that states the old requirement. No open or closed PR covers #2755, and no open PR touches grpc-workers.md (open PRs #110 and #107 hold concepts/architecture/grpc-pipeline.md, reference/api/messages.md, and getting-started/pd-disaggregation.md). Note: the cli-configuration scan's pending sglang-zmq-direct-backend concern also lists grpc-workers.md among its doc_paths; if that PR opens first, this concern must wait for it to merge.
Scope: grpc-workers / rust-servicer-switch-no-vllm-hook. Other concerns are deferred.
How to test
git diff --checkandpnpm checkand the productionpnpm build.Checklist