feat(sglang): add managed Python sidecar entrypoint - #11796
connorcarpenter15 wants to merge 2 commits into
Conversation
Signed-off-by: Connor Carpenter <connorc@nvidia.com>
WalkthroughChangesThe SGLang sidecar launcher now runs through a backend-scoped Rust binding and a new Python module. The former top-level entrypoint is removed, type stubs and tests are updated, and ownership and CI path mappings exclude the old sidecar directory. SGLang sidecar launcher
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/bindings/python/rust/llm/entrypoint.rs (1)
951-956: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse the crate-level
to_pyerrhelper
lib/bindings/python/rust/llm/entrypoint.rs:951duplicates the sharedcrate::to_pyerralready defined inlib/bindings/python/rust/lib.rs. Use the shared helper here instead of keeping a second conversion path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/bindings/python/rust/llm/entrypoint.rs` around lines 951 - 956, Remove the local to_pyerr function in entrypoint.rs and reuse the existing crate-level crate::to_pyerr helper for all corresponding error conversions, preserving the current PyErr behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/bindings/python/rust/llm/entrypoint.rs`:
- Around line 951-956: Remove the local to_pyerr function in entrypoint.rs and
reuse the existing crate-level crate::to_pyerr helper for all corresponding
error conversions, preserving the current PyErr behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ae44fcb0-2e5f-46df-985e-3852793b4c31
📒 Files selected for processing (13)
.github/codeowners/areas.yaml.github/filters.yamlCODEOWNERScomponents/src/dynamo/common/backend/tests/test_backend_bindings.pycomponents/src/dynamo/sglang/sidecar.pycomponents/src/dynamo/sglang/tests/test_sidecar.pycomponents/src/dynamo/sglang_sidecar/__init__.pycomponents/src/dynamo/sglang_sidecar/__main__.pycomponents/src/dynamo/sglang_sidecar/main.pylib/bindings/python/rust/backend.rslib/bindings/python/rust/lib.rslib/bindings/python/rust/llm/entrypoint.rslib/bindings/python/src/dynamo/_core.pyi
💤 Files with no reviewable changes (7)
- components/src/dynamo/sglang_sidecar/main.py
- components/src/dynamo/sglang_sidecar/init.py
- .github/filters.yaml
- CODEOWNERS
- components/src/dynamo/sglang_sidecar/main.py
- .github/codeowners/areas.yaml
- lib/bindings/python/rust/lib.rs
Signed-off-by: Connor Carpenter <connorc@nvidia.com>
Summary
dynamo.sglang_sidecarmodule with the SGLang-scopeddynamo.sglang.sidecarentry point.dynamo._core.backend._run_sglang_sidecar(argv)binding.main()runs, not when the module is imported.Motivation
SGLang's module-based sidecar interface needs an importable entry point that composes the native SGLang engine adapter with Dynamo's unified backend lifecycle. Keeping this under
dynamo.sglangmakes the ownership and invocation clearer while retaining the released wheel as the only required installation.python3 -m sglang.launch_server \ <args> \ --grpc-port 30001 \ --sidecar dynamo.sglang.sidecarScope
This extracts the Python entry-point portion of #11636. It intentionally leaves the standalone executable changes and the
lib/directory organization to separate PRs. The Rust crate remains atlib/sglang-sidecar.Validation
cargo fmt --all -- --checkcargo metadata --manifest-path lib/bindings/python/Cargo.toml --no-deps --format-version 1ruff checkon the changed Python tests and launchergit diff --checkA full local Cargo test build could not complete because the workspace filesystem ran out of space while compiling Dynamo's transitive dependencies; draft PR CI will provide the full build and test result.
Summary by CodeRabbit
New Features
Removed
Tests