[TRTLLM-10804][infra] add LLM_SBSA_WHEEL_DOCKER_IMAGE#12635
Conversation
|
/bot run --skip-test --disable-fail-fast |
|
PR_Github #40984 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughThis pull request introduces wheel-specific Docker image support across the Jenkins pipeline and test infrastructure. A new Changes
Sequence Diagram(s)sequenceDiagram
participant Jenkins as Jenkins MR<br/>Pipeline
participant L0Test as L0_Test.groovy
participant DockerImage as Docker Image<br/>Tag
participant Build as runLLMBuild
participant TestUtil as test_pip_install.py
Jenkins->>L0Test: Pass wheelDockerImage
L0Test->>DockerImage: Extract PyTorch version<br/>(e.g., pytorch-26.02)
DockerImage-->>L0Test: Version components
L0Test->>L0Test: Format version_local<br/>(e.g., ngcpytorch2602)
L0Test->>Build: Call runLLMBuild<br/>(cpu_arch, version_local)
Build->>Build: Mutate tensorrt_llm/version.py<br/>Add +version_local suffix
Build-->>L0Test: Wheel built
L0Test->>TestUtil: Call test_pip_install.py<br/>(wheel_path, version_local)
TestUtil->>TestUtil: Parse wheel filename<br/>Match local version metadata
TestUtil-->>L0Test: Wheel validation passed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unittest/test_pip_install.py (1)
104-140:⚠️ Potential issue | 🟡 MinorUse
any()to iterate over frozenset of tags and catch specific exceptions.
parse_wheel_filenamereturnstagsas afrozenset[Tag], not a singleTag. The unpacking(tag, ) = tagsfails on wheels with multiple valid tag combinations. Additionally, catchValueErrorspecifically instead of broadExceptionper coding guidelines.♻️ Proposed fix
try: from packaging.utils import parse_wheel_filename - name, ver, build, tags = parse_wheel_filename(filename) - except Exception as e: + name, ver, _build, tags = parse_wheel_filename(filename) + except ValueError as e: print(f"error: {e}") continue - (tag, ) = tags - if tag.abi != cpython_version or tag.interpreter != cpython_version: + if not any(tag.abi == cpython_version and tag.interpreter == cpython_version + for tag in tags): continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unittest/test_pip_install.py` around lines 104 - 140, The get_wheel_url helper incorrectly assumes parse_wheel_filename returns a single Tag and catches all exceptions; change the except to catch ValueError specifically for parse_wheel_filename, and replace the unpacking "(tag, ) = tags" with an any(...) check that iterates over the frozenset tags to see if any tag has tag.abi == cpython_version and tag.interpreter == cpython_version (use any(tag.abi == cpython_version and tag.interpreter == cpython_version for tag in tags)); keep the surrounding name/version filtering logic intact in the get_wheel_url function.
🧹 Nitpick comments (1)
jenkins/L0_Test.groovy (1)
3549-3558: Use an explicit DLFW flag here.
values[4]is still a path fragment (""/"dlfw/"), but this block now relies on its truthiness to decide whether to mint a local-version wheel. An explicit boolean or exact comparison would make this much harder to misread.♻️ Low-friction cleanup
- def isDlfw = values[4] + def wheelVariant = values[4] def versionLocal = "" - if (isDlfw) { + if (wheelVariant == "dlfw/") { // Extract PyTorch version from LLM_DOCKER_IMAGE. e.g. pytorch-25.12 -> 2512 def matcher = LLM_DOCKER_IMAGE =~ /:pytorch-(\d+)\.(\d+)-/ if (!matcher) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@jenkins/L0_Test.groovy` around lines 3549 - 3558, The current check uses def isDlfw = values[4] (a path fragment like "" or "dlfw/") and relies on its truthiness to decide creating versionLocal; change this to an explicit boolean or comparison: compute a boolean flag (e.g., def isDlfw = values[4] == 'dlfw/' or def isDlfw = values[4].trim() == 'dlfw') and use that in the if (isDlfw) block, leaving the extraction logic that uses LLM_DOCKER_IMAGE and assignment to versionLocal unchanged (referencing isDlfw, values[4], LLM_DOCKER_IMAGE, versionLocal).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@jenkins/L0_Test.groovy`:
- Line 41: LLM_WHEEL_DOCKER_IMAGE is being set directly from
env.wheelDockerImage which makes it required; change the assignment to fall back
to the existing docker image variable when wheelDockerImage is empty (e.g., set
LLM_WHEEL_DOCKER_IMAGE = env.wheelDockerImage ?: env.dockerImage) so older
callers that only provide dockerImage still get a valid image; update the
assignment referencing LLM_WHEEL_DOCKER_IMAGE, env.wheelDockerImage, and
env.dockerImage.
---
Outside diff comments:
In `@tests/unittest/test_pip_install.py`:
- Around line 104-140: The get_wheel_url helper incorrectly assumes
parse_wheel_filename returns a single Tag and catches all exceptions; change the
except to catch ValueError specifically for parse_wheel_filename, and replace
the unpacking "(tag, ) = tags" with an any(...) check that iterates over the
frozenset tags to see if any tag has tag.abi == cpython_version and
tag.interpreter == cpython_version (use any(tag.abi == cpython_version and
tag.interpreter == cpython_version for tag in tags)); keep the surrounding
name/version filtering logic intact in the get_wheel_url function.
---
Nitpick comments:
In `@jenkins/L0_Test.groovy`:
- Around line 3549-3558: The current check uses def isDlfw = values[4] (a path
fragment like "" or "dlfw/") and relies on its truthiness to decide creating
versionLocal; change this to an explicit boolean or comparison: compute a
boolean flag (e.g., def isDlfw = values[4] == 'dlfw/' or def isDlfw =
values[4].trim() == 'dlfw') and use that in the if (isDlfw) block, leaving the
extraction logic that uses LLM_DOCKER_IMAGE and assignment to versionLocal
unchanged (referencing isDlfw, values[4], LLM_DOCKER_IMAGE, versionLocal).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 32975d44-6396-47ca-98d2-7a8d0875e035
📒 Files selected for processing (4)
jenkins/L0_MergeRequest.groovyjenkins/L0_Test.groovyjenkins/current_image_tags.propertiestests/unittest/test_pip_install.py
|
PR_Github #40984 [ run ] completed with state
|
|
/bot run --stage-list 'Build-Docker-Images' |
|
PR_Github #46352 [ run ] triggered by Bot. Commit: |
|
PR_Github #46352 [ run ] completed with state
|
Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
|
/bot run --stage-list 'Build-Docker-Images' |
|
PR_Github #47679 [ run ] triggered by Bot. Commit: |
Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
|
/bot run --stage-list 'Build-Docker-Images' |
|
PR_Github #47699 [ run ] triggered by Bot. Commit: |
|
/bot run --skip-test --disable-fail-fast |
|
PR_Github #47725 [ run ] triggered by Bot. Commit: |
|
PR_Github #47725 [ run ] completed with state
|
|
PR_Github #47819 [ run ] triggered by Bot. Commit: |
Updated installation instructions for TensorRT LLM to include the latest version 1.3.0rc16 and clarify usage with the NGC PyTorch container. Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
|
PR_Github #47819 [ run ] completed with state
|
Signed-off-by: Yiteng Niu <6831097+niukuo@users.noreply.github.com>
|
PR_Github #47991 [ run ] triggered by Bot. Commit: |
|
PR_Github #47998 [ run ] triggered by Bot. Commit: |
|
PR_Github #47998 [ run ] completed with state
|
|
PR_Github #48180 [ run ] triggered by Bot. Commit: |
|
PR_Github #48180 [ run ] completed with state |
|
/bot skip --comment "sanity check passed" |
|
PR_Github #48247 [ skip ] triggered by Bot. Commit: |
|
PR_Github #48247 [ skip ] completed with state |
4 participants
Summary by CodeRabbit
New Features
Tests
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.