fix(client): support protobuf 6 runtimes - #564
Conversation
b5e6bcb to
d2af351
Compare
WalkthroughThe Python package now supports protobuf runtime versions 5 and 6, uses protobuf 5 generated bindings, separates code-generation tooling, and tests both runtime versions in CI. ChangesProtobuf compatibility
Estimated code review effort: 3 (Moderate) | ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@modelexpress_client/python/pyproject.toml`:
- Line 31: Raise the protobuf minimum to >=5.27.2 in
modelexpress_client/python/pyproject.toml and
modelexpress_client/python/README.md, and update the protobuf 5 CI constraint in
.github/workflows/ci.yml accordingly. Add a CI job testing protobuf==5.27.2 to
validate the declared minimum version.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: a614398b-6b1e-4441-a3c9-d4ddd22b77da
⛔ Files ignored due to path filters (1)
modelexpress_client/python/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/ci.ymlmodelexpress_client/python/README.mdmodelexpress_client/python/pyproject.tomlmodelexpress_client/python/tests/test_types.py
Signed-off-by: Zheng Luo <zheluo@nvidia.com>
d2af351 to
b1829fb
Compare
Summary
protobuf>=5.27,<7grpcio-toolsout of the test extra into an explicitcodegenextraWhy
Current TensorRT-LLM images require protobuf 6.x, while ModelExpress declares
protobuf<6. A normal ModelExpress install therefore conflicts with the TRT-LLM runtime or requires--no-deps, leaving compatibility unverified. The generated protobuf 5 bindings already run correctly on protobuf 6; the blocker is the package constraint and same-major-only test guard.User impact
ModelExpress can be installed normally into qualified TensorRT-LLM images without downgrading their protobuf stack. Existing protobuf 5 users remain supported. Contributors changing
p2p.protousepip install -e ".[codegen]"to preserve dual-runtime bindings.Validation
protobuf>=5.27,<7uv lock --checkgit diff --checkSummary by CodeRabbit
Compatibility
Documentation
Testing