Added llmcompressor .tekton#1986
Conversation
0c7a124 to
c265fd6
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdded a new Tekton PipelineRun manifest that triggers pull-request builds for the odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312 pipeline, using CEL path filters, Konflux retrigger annotations, parameterized build inputs, prefetch sources, and workspace/secret bindings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Developer
participant GitHost as Git Repository
participant PAC as Pipelines-as-Code
participant Tekton as Tekton PipelineRun
participant Konflux as Konflux Controller
participant Prefetch as Prefetch Sources (gomod/rpm/pip)
participant Build as Multi-arch Build Pipeline
participant Registry as Image Registry
Developer->>GitHost: Open PR / push changes
GitHost->>PAC: Send webhook (PR event)
PAC->>Konflux: Evaluate CEL path filters
Konflux-->>PAC: Condition passed / failed
alt Condition passed
PAC->>Tekton: Create PipelineRun (params, workspaces, annotations)
Tekton->>Prefetch: Fetch dependencies (gomod, rpm, pip CUDA reqs)
Prefetch-->>Build: Provide inputs
Tekton->>Build: Execute multi-arch build pipeline
Build->>Registry: Push images (multi-arch index)
Registry-->>Build: Push confirmation
Build-->>Tekton: TaskRun results
Tekton->>PAC: Update run status
else Condition failed
PAC-->>GitHost: No PipelineRun created
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use your project's `ruff` configuration to improve the quality of Python code reviews.Add a Ruff configuration file to your project to customize how CodeRabbit runs |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml (3)
16-21: Redundant path check in CEL expression.Line 19 explicitly checks
build-args/konflux.cuda.confbut this path is already covered by the glob patternjupyter/pytorch+llmcompressor/ubi9-python-3.12/**on line 18. Consider removing the redundant check.♻️ Suggested simplification
( ".tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml".pathChanged() || "jupyter/pytorch+llmcompressor/ubi9-python-3.12/**".pathChanged() || - "jupyter/pytorch+llmcompressor/ubi9-python-3.12/build-args/konflux.cuda.conf".pathChanged() || "jupyter/utils/**".pathChanged() ) && body.repository.full_name == "red-hat-data-services/notebooks"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml around lines 16 - 21, The CEL trigger contains a redundant path check: remove the explicit string literal check "jupyter/pytorch+llmcompressor/ubi9-python-3.12/build-args/konflux.cuda.conf". Keep the glob check "jupyter/pytorch+llmcompressor/ubi9-python-3.12/**" and the other pathChanged() calls, and ensure the final boolean expression still ANDs with body.repository.full_name == "red-hat-data-services/notebooks"; update the expression to eliminate the duplicate path string.
75-76: Pip prefetch architecture includes aarch64 but pipeline only builds amd64.The
binary.archfield specifies"x86_64,aarch64"for pip prefetch, butbuild-platformsonly includes the amd64 platform. This will prefetch aarch64 dependencies unnecessarily. Consider aligning the architectures or confirm if multi-arch support is planned.♻️ Suggested fix to align architectures
- path: jupyter/pytorch+llmcompressor/ubi9-python-3.12 type: pip binary: - arch: "x86_64,aarch64" + arch: "x86_64" requirements_files: ["requirements.cuda.txt"]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml around lines 75 - 76, The pip prefetch config lists binary.arch as "x86_64,aarch64" but the pipeline only builds amd64; update the YAML so the architectures align by either removing "aarch64" from binary.arch (i.e., set binary.arch to "x86_64") or by adding the matching aarch64 target to the pipeline's build-platforms; look for the binary.arch key and the build-platforms entries in the Tekton YAML and make them consistent (choose single-arch prefetch or enable multi-arch build support).
83-84: Inconsistent template variable formatting.The revision value uses
'{{ target_branch }}'with spaces inside the braces, while other template variables in this file use the no-space format (e.g.,'{{revision}}','{{source_url}}'). While both formats may work, inconsistency can lead to maintenance confusion.♻️ Suggested fix for consistency
- name: revision - value: '{{ target_branch }}' + value: '{{target_branch}}'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml around lines 83 - 84, The template variable format for the revision value is inconsistent: it currently uses '{{ target_branch }}' while other variables use the no-space form (e.g., '{{revision}}', '{{source_url}}'); update the revision value to use the consistent no-space template format (change '{{ target_branch }}' to '{{target_branch}}' or, if the intended variable name is 'revision', replace with '{{revision}}') so all template variables follow the same style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml:
- Around line 67-77: The prefetch-input block for rhds is missing a matching
generic entry; add a second entry under the prefetch-input list with the same
path "jupyter/pytorch+llmcompressor/ubi9-python-3.12/prefetch-input/rhds" but
set type: generic (mirroring the existing rpm entry) so both rpm and generic
rhds variants are present; update the prefetch-input list near the existing rhds
rpm entry to include this new generic entry.
---
Nitpick comments:
In
@.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml:
- Around line 16-21: The CEL trigger contains a redundant path check: remove the
explicit string literal check
"jupyter/pytorch+llmcompressor/ubi9-python-3.12/build-args/konflux.cuda.conf".
Keep the glob check "jupyter/pytorch+llmcompressor/ubi9-python-3.12/**" and the
other pathChanged() calls, and ensure the final boolean expression still ANDs
with body.repository.full_name == "red-hat-data-services/notebooks"; update the
expression to eliminate the duplicate path string.
- Around line 75-76: The pip prefetch config lists binary.arch as
"x86_64,aarch64" but the pipeline only builds amd64; update the YAML so the
architectures align by either removing "aarch64" from binary.arch (i.e., set
binary.arch to "x86_64") or by adding the matching aarch64 target to the
pipeline's build-platforms; look for the binary.arch key and the build-platforms
entries in the Tekton YAML and make them consistent (choose single-arch prefetch
or enable multi-arch build support).
- Around line 83-84: The template variable format for the revision value is
inconsistent: it currently uses '{{ target_branch }}' while other variables use
the no-space form (e.g., '{{revision}}', '{{source_url}}'); update the revision
value to use the consistent no-space template format (change '{{ target_branch
}}' to '{{target_branch}}' or, if the intended variable name is 'revision',
replace with '{{revision}}') so all template variables follow the same style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 78532427-902f-4883-8e39-cc155b8df7e4
📒 Files selected for processing (1)
.tekton/odh-workbench-jupyter-pytorch-llmcompressor-cuda-py312-pull-request.yaml
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
c265fd6 to
4599e19
Compare
|
/retest |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ide-developer The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Added llmcompressor .tekton
Description
Added llmcompressor .tekton
How Has This Been Tested?
Self checklist (all need to be checked):
make test(gmakeon macOS) before asking for reviewDockerfile.konfluxfiles should be done inodh/notebooksand automatically synced torhds/notebooks. For Konflux-specific changes, modifyDockerfile.konfluxfiles directly inrhds/notebooksas these require special attention in the downstream repository and flow to the upcoming RHOAI release.Merge criteria: