maximize use of the RHOAI index#173
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe changes update Python dependencies across configuration, lock, and script files, replacing a package set (removing aiohttp, cryptography, scipy; adding fastapi, opentelemetry-*, sqlalchemy, etc.). The dependency resolution process shifts from RHOAI 3.2 to 3.3, employs unsafe-first-match index strategy, and generates updated lock files and Tekton pipeline configurations accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![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 (2)
.tekton/rag-tool-pull-request.yaml (1)
62-66:⚠️ Potential issue | 🟡 MinorDuplicate package name
tree-sitter-typescriptin binary packages list.The package
tree-sitter-typescriptappears twice in the list - once in the main wheel packages and again at the end from PYPI_WHEELS. This occurs because the package is present in both the RHOAI wheel list and the PYPI_WHEELS constant.While this duplication won't break the build, it adds unnecessary noise. Consider deduplicating in
konflux_requirements.shbefore writing to the YAML files.Suggested fix in scripts/konflux_requirements.sh
wheel_packages=$(grep -v "^[`#-`]" "$WHEEL_FILE" | sed 's/==.*//' | tr '\n' ',' | sed 's/,$//') # append extra wheels to the list -wheel_packages="$wheel_packages,$EXTRA_WHEELS,$PYPI_WHEELS" +wheel_packages=$(echo "$wheel_packages,$EXTRA_WHEELS,$PYPI_WHEELS" | tr ',' '\n' | sort -u | tr '\n' ',' | sed 's/,$//')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.tekton/rag-tool-pull-request.yaml around lines 62 - 66, The binary packages list contains duplicate "tree-sitter-typescript" because the script merges the RHOAI wheel list with the PYPI_WHEELS constant without deduplication; update konflux_requirements.sh to dedupe the combined wheel list (e.g., after concatenating the RHOAI list and PYPI_WHEELS) before writing to .tekton/rag-tool-pull-request.yaml by using a unique filter (assoc array, sort -u, or similar) on the variable that holds the merged packages so the final "binary" -> "packages" value contains only unique package names.scripts/konflux_requirements.sh (1)
85-89:⚠️ Potential issue | 🟡 MinorPackage list may contain duplicates when PYPI_WHEELS overlaps with wheel packages.
As noted in the Tekton pipeline review,
tree-sitter-typescriptappears in both the RHOAI wheel list andPYPI_WHEELS, causing duplication.Suggested fix to deduplicate
wheel_packages=$(grep -v "^[`#-`]" "$WHEEL_FILE" | sed 's/==.*//' | tr '\n' ',' | sed 's/,$//') # append extra wheels to the list -wheel_packages="$wheel_packages,$EXTRA_WHEELS,$PYPI_WHEELS" +# Combine and deduplicate +wheel_packages=$(echo "$wheel_packages,$EXTRA_WHEELS,$PYPI_WHEELS" | tr ',' '\n' | sort -u | tr '\n' ',' | sed 's/,$//')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/konflux_requirements.sh` around lines 85 - 89, The wheel_packages assembly can produce duplicates when entries from WHEEL_FILE overlap with EXTRA_WHEELS or PYPI_WHEELS; change the construction of wheel_packages (the variable derived from WHEEL_FILE and then appended with EXTRA_WHEELS and PYPI_WHEELS) to perform deduplication before it's injected into the YAMLs: split/join the combined list (from wheel_packages, EXTRA_WHEELS and PYPI_WHEELS), normalize delimiters, then remove duplicates (e.g., via a temporary associative set or piping through sort -u / awk to keep unique entries) and reassign wheel_packages to that unique, comma-separated string before running the sed replacements on the files referenced (.tekton/rag-tool-pull-request.yaml and .tekton/rag-tool-push.yaml).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/konflux_requirements.sh`:
- Around line 29-31: The invoked helper script
generate_requirements_overrides.py referenced by the uv run command (with flags
--rhoai-index and -o) is missing; either add a new script named
generate_requirements_overrides.py that parses --rhoai-index and -o and performs
the requirements override generation, or update the uv run invocation to call
the correct existing script name that already implements those flags (or adjust
the flags to match the existing script). Ensure the chosen script accepts
--rhoai-index and -o and exits nonzero on failure so the calling command fails
loudly.
---
Outside diff comments:
In @.tekton/rag-tool-pull-request.yaml:
- Around line 62-66: The binary packages list contains duplicate
"tree-sitter-typescript" because the script merges the RHOAI wheel list with the
PYPI_WHEELS constant without deduplication; update konflux_requirements.sh to
dedupe the combined wheel list (e.g., after concatenating the RHOAI list and
PYPI_WHEELS) before writing to .tekton/rag-tool-pull-request.yaml by using a
unique filter (assoc array, sort -u, or similar) on the variable that holds the
merged packages so the final "binary" -> "packages" value contains only unique
package names.
In `@scripts/konflux_requirements.sh`:
- Around line 85-89: The wheel_packages assembly can produce duplicates when
entries from WHEEL_FILE overlap with EXTRA_WHEELS or PYPI_WHEELS; change the
construction of wheel_packages (the variable derived from WHEEL_FILE and then
appended with EXTRA_WHEELS and PYPI_WHEELS) to perform deduplication before it's
injected into the YAMLs: split/join the combined list (from wheel_packages,
EXTRA_WHEELS and PYPI_WHEELS), normalize delimiters, then remove duplicates
(e.g., via a temporary associative set or piping through sort -u / awk to keep
unique entries) and reassign wheel_packages to that unique, comma-separated
string before running the sed replacements on the files referenced
(.tekton/rag-tool-pull-request.yaml and .tekton/rag-tool-push.yaml).
🪄 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: 970ee6f8-fd52-4a09-8f61-ef60781a07f1
📒 Files selected for processing (8)
.tekton/rag-tool-pull-request.yaml.tekton/rag-tool-push.yamlrequirements-build.txtrequirements.hashes.source.txtrequirements.hashes.wheel.pypi.txtrequirements.hashes.wheel.txtrequirements.overrides.txtscripts/konflux_requirements.sh
| uv run --script scripts/generate_requirements_overrides.py \ | ||
| --rhoai-index "$RHOAI_INDEX_URL" \ | ||
| -o requirements.overrides.txt |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if the generate_requirements_overrides.py script exists and examine its argument parser
if [ -f "scripts/generate_requirements_overrides.py" ]; then
echo "Script exists"
head -100 scripts/generate_requirements_overrides.py
else
echo "Script NOT FOUND"
fd -t f "generate_requirements" scripts/
fiRepository: lightspeed-core/rag-content
Length of output: 87
🏁 Script executed:
#!/bin/bash
# Search for any file with "generate_requirements" in its name across the entire repository
echo "=== Searching for files with 'generate_requirements' pattern ==="
fd -t f "generate_requirements" .
echo ""
echo "=== Listing all files in scripts/ directory ==="
ls -la scripts/
echo ""
echo "=== Searching for references to generate_requirements_overrides in the codebase ==="
rg "generate_requirements_overrides" . --type sh --type pyRepository: lightspeed-core/rag-content
Length of output: 1194
scripts/generate_requirements_overrides.py does not exist in the repository.
The script at lines 29-31 attempts to invoke scripts/generate_requirements_overrides.py, but this file is not present. This will cause runtime failure. Either create the script with support for --rhoai-index and -o arguments, or correct the script reference if it should point to an existing file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/konflux_requirements.sh` around lines 29 - 31, The invoked helper
script generate_requirements_overrides.py referenced by the uv run command (with
flags --rhoai-index and -o) is missing; either add a new script named
generate_requirements_overrides.py that parses --rhoai-index and -o and performs
the requirements override generation, or update the uv run invocation to call
the correct existing script name that already implements those flags (or adjust
the flags to match the existing script). Ensure the chosen script accepts
--rhoai-index and -o and exits nonzero on failure so the calling command fails
loudly.
syedriko
force-pushed
the
syedriko-python-deps-max-rhoai
branch
2 times, most recently
from
54c0fd7 to
8094e4e
Compare
syedriko
force-pushed
the
syedriko-python-deps-max-rhoai
branch
from
8094e4e to
f6f1ff7
Compare
1 participant
Description
maximize use of the RHOAI index
Type of change
Tools used to create PR
Identify any AI code assistants used in this PR (for transparency and review context)
Related Tickets & Documents
Checklist before requesting a review
Testing
Summary by CodeRabbit
Release Notes