Repository navigation
chore(deps): bump TensorRT-LLM to 1.3.0rc22 - #1972
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
github-actions
Bot
added
ci
📝 WalkthroughWalkthroughTensorRT-LLM references are updated from RC18-era versions to RC22-era versions across the Docker release workflow and CI installation script. The CI setup also pins Typer below version 0.26, and a Dockerfile comment is adjusted. ChangesTensorRT-LLM version alignment
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
👋 The PR description doesn't fully follow
Please update the PR description so reviewers have the context they need. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Straightforward dependency version bump (TensorRT-LLM 1.3.0rc18 → 1.3.0rc22). Version references are consistent across all three changed files. No code changes — per REVIEW.md, skipping detailed review for dep bumps.
0 issues found (0 🔴 Important · 0 🟡 Nit · 0 🟣 Pre-existing)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
CI wheel and release-docker base images move from 1.3.0rc18 to rc22 (matrix rc22/rc21/rc20). The NGC-base PyYAML shadow in engine.Dockerfile stays; its comment no longer names a single rc. rc22's dependency tree (via transformers) resolves typer 0.27.0, whose CLI exit path leaks click.exceptions.Exit — every hf model download exits 1 after succeeding. Pin typer<0.26 after the trtllm install (transformers leaves typer unbounded). Signed-off-by: Simo Lin <25425177+slin1237@users.noreply.github.com>
5dc9189 to
0c1b06a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/release-trtllm-docker.yml:
- Line 5: Preserve the documented empty base_image_ref source-build behavior in
the workflow matrix expression and related input handling. Remove the RC22
fallback for empty values or add an explicit build_from_source input and branch,
ensuring manual source builds reach the source-build path consistently across
the matrix definition and referenced workflow conditions.
In `@scripts/ci_install_trtllm.sh`:
- Around line 71-75: Extend the Typer workaround in scripts/ci_install_trtllm.sh
after the pip install to run pip check, verify the installed Typer version is
below 0.26, and invoke the same successful hf model-download CLI path used by CI
while requiring exit status 0. Keep the existing installation constraint and use
the CI download command rather than introducing a separate test path.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e52d3a4b-fb7c-43de-91d2-7fdc8eaa54c4
📒 Files selected for processing (3)
.github/workflows/release-trtllm-docker.ymldocker/engine.Dockerfilescripts/ci_install_trtllm.sh
| run-name: >- | ||
| SMG+TRTLLM | | ||
| ${{ github.event_name == 'push' && '3-version matrix' || format('base={0}', inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc18') }} | | ||
| ${{ github.event_name == 'push' && '3-version matrix' || format('base={0}', inputs.base_image_ref || 'nvcr.io/nvidia/tensorrt-llm/release:1.3.0rc22') }} | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the documented source-build mode.
The input says an empty base_image_ref builds from source, but the || ...rc22 fallback in Line 63 converts an empty value back to the RC22 image. Manual source builds therefore cannot reach the source-build path. Add an explicit build_from_source input/branch, or remove the “Empty = build from source” contract and update Line 5 accordingly.
Also applies to: 26-27, 63-63
🤖 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 @.github/workflows/release-trtllm-docker.yml at line 5, Preserve the
documented empty base_image_ref source-build behavior in the workflow matrix
expression and related input handling. Remove the RC22 fallback for empty values
or add an explicit build_from_source input and branch, ensuring manual source
builds reach the source-build path consistently across the matrix definition and
referenced workflow conditions.
| # typer >= 0.26 leaks click.exceptions.Exit through its main on CLI exit, so | ||
| # every `hf` invocation (model downloads) exits 1 even on success. The | ||
| # transformers pulled by tensorrt-llm has an unbounded typer dependency. | ||
| pip install --no-cache-dir "typer<0.26" | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Add a regression check for the Typer workaround.
Installing typer<0.26 does not verify that the selected version is correct or that the successful Hugging Face/model-download path now exits with status 0. Add pip check, assert the installed Typer version, and exercise the same successful CLI path used by CI.
🤖 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 `@scripts/ci_install_trtllm.sh` around lines 71 - 75, Extend the Typer
workaround in scripts/ci_install_trtllm.sh after the pip install to run pip
check, verify the installed Typer version is below 0.26, and invoke the same
successful hf model-download CLI path used by CI while requiring exit status 0.
Keep the existing installation constraint and use the CI download command rather
than introducing a separate test path.
1 participant
Description
Problem
CI installs TensorRT-LLM 1.3.0rc18 and the release-docker matrix builds rc18/rc17/rc16; NVIDIA's index is at 1.3.0rc22 (the GA line on pypi.org is still 1.2.x, so the rc channel remains the right track for the gRPC serve command).
Solution
scripts/ci_install_trtllm.sh:TRTLLM_VERSION="1.3.0rc22"(wheel present on pypi.nvidia.com); header note about the serve-command/Harmony fixes updated — they shipped in rc14 and remain in rc22.release-trtllm-docker.yml: matrix →release:1.3.0rc22 / rc21 / rc20, defaults and examples → rc22. All three NGC image tags verified via manifest inspect.docker/engine.Dockerfile: the PyYAML shadow for NGC bases is kept (harmless if a base ever ships a pip-managed PyYAML); its comment now says "1.3.0rc bases" instead of naming rc18.rc22's dependency tree (via its
transformerspin, which leavestyperunbounded) resolves typer 0.27.0, whose CLI exit path leaksclick.exceptions.Exit— everyhfmodel download exits 1 after succeeding, failing the e2e model-fetch step on repeat attempts. The installer now pinstyper<0.26after the TensorRT-LLM install (0.25.1 verified working against click 8.3.3, reproduced and bisected locally).Test Plan
bash -nclean; rc22 wheel present on pypi.nvidia.com;release:1.3.0rc22/rc21/rc20image manifests verified.Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit