feat(grpc): add smg-grpc-proto Python package for proto definitions - #385
Conversation
This package provides Python gRPC stubs for SGLang, vLLM, and TRT-LLM. Stubs are generated at build/install time using grpcio-tools, not committed. The proto source symlinks to grpc_client/proto/ (the source of truth).
Include grpc_client/python/pyproject.toml and __init__.py in bump-version and show-version Makefile targets to keep the package version in sync with SMG releases.
- Use protoc.main() Python API instead of subprocess - Generate .pyi type stubs with --pyi_out - Add mypy ignore-errors header to generated files - Support editable installs via DevelopWithProto class
📝 WalkthroughWalkthroughAdds a Python gRPC proto package (smg-grpc-proto) with packaging/README/.gitignore, build-time protobuf stub generation integrated into setup.py, and extends Makefile version display/bump targets to include the new gRPC Python package files. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Setup as setup.py (BuildPyWithProto)
participant Protoc as grpc_tools.protoc
participant Gen as smg_grpc_proto/generated
participant Build as build/install
Dev->>Setup: run `python setup.py build` or `pip install -e .`
Setup->>Protoc: discover proto files and invoke protoc
Protoc-->>Gen: emit *_pb2.py, *_pb2_grpc.py and .pyi files
Setup->>Gen: post-process (make imports relative, add mypy-ignore headers)
Setup->>Build: proceed with standard build/develop (include generated files)
Build-->>Dev: build/install completes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
Summary of ChangesHello @CatherineSue, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses the issue of duplicated gRPC proto definitions across various repositories by introducing a new, pip-installable Python package named Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new Python package smg-grpc-proto to centralize the gRPC proto definitions, which is a great step towards reducing code duplication and improving maintainability. The package is well-structured, using a modern pyproject.toml for configuration and a custom setup.py to handle protobuf code generation at build time. The changes to the Makefile for version management are consistent with the existing project structure. My main feedback is a point of clarification in the README.md regarding the local development setup, which seems to contradict the symlink-based approach used in the build script.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@grpc_client/python/pyproject.toml`:
- Around line 39-40: The package-data for the smg_grpc_proto package only lists
generated/*.py which omits the generated type stubs; update the
[tool.setuptools.package-data] entry for smg_grpc_proto to also include
generated/*.pyi so the built wheel ships the .pyi stubs (i.e., add
generated/*.pyi alongside generated/*.py in the smg_grpc_proto package-data
list).
In `@grpc_client/python/setup.py`:
- Around line 23-26: Replace the silent return when no .proto files are found
with a hard failure: in setup.py where proto_files =
list(proto_dir.glob("*.proto")) is checked, remove the print + return and raise
a RuntimeError (or SystemExit) with a clear message that no .proto files were
found for proto_dir so the build fails loudly; alternatively gate the check
behind an explicit opt-in dev flag and raise the same error unless that flag is
set. Ensure the error references proto_dir and proto_files in the message so
it’s easy to debug.
In `@grpc_client/python/smg_grpc_proto/proto`:
- Line 1: The repo currently points smg_grpc_proto/proto at an external symlink
("../../proto"), which breaks sdist/wheel and Windows installs; replace the
symlink by vendoring or copying the actual .proto files into the package
directory and ensure packaging includes them. Update your build/packaging step
(e.g., pyproject.toml build-backend hook or setup.py/build_ext hook) to copy all
.proto files into smg_grpc_proto/proto at build time, add those files to
package_data/include-package-data so they ship in sdist/wheel, and remove
reliance on the "../../proto" symlink so stub generation (and tools that read
smg_grpc_proto/proto) work on Windows and in built distributions.
🧹 Nitpick comments (1)
grpc_client/python/setup.py (1)
18-22: Clean stale generated stubs before regeneration.If a proto is removed, old
*_pb2*.py/pyifiles remain and can be packaged. Consider clearing the generated folder (or at least stale_pb2*files) before runningprotoc.♻️ Proposed refactor
output_dir.mkdir(parents=True, exist_ok=True) + for stale in output_dir.glob("*_pb2*.*"): + stale.unlink(missing_ok=True) (output_dir / "__init__.py").write_text( '"""Auto-generated protobuf stubs. Do not edit."""\n' )
…385) Signed-off-by: ppraneth <pranethparuchuri@gmail.com>
Description
Problem
Proto definitions for gRPC services (SGLang, vLLM, TRT-LLM) are currently duplicated across SMG and upstream repos. Keeping them in sync is error-prone and creates maintenance burden. SMG should be the single source of truth for these protos.
Solution
Add a pip-installable package
smg-grpc-protothat:grpcio-toolspip install smg-grpc-protoinstead of maintaining their own copiesChanges
grpc_client/python/package structure:pyproject.toml- package metadatasetup.py- build-time proto generation (matches vLLM patterns)smg_grpc_proto/__init__.py- re-exports generated modulesgrpc_client/proto/for proto source filesMakefileto include new files inbump-versiontargetTest Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
New Features
Documentation
Chores