-
Notifications
You must be signed in to change notification settings - Fork 67
konflux: fix pip compile bug on package hash #1049
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRemoves "greenlet" from Tekton prefetch binary package lists, bumps several build dependency versions, refreshes source and wheel hash lockfiles with many updated versions/hashes and added packages, adds overrides for pandas/pyarrow/sqlalchemy, and updates konflux requirements script to route certain packages to source hashes. Changes
Sequence Diagram(s)(omitted — changes are configuration, lockfile, and scripting updates without new multi-component control flow requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
tisnik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
/retest |
ac351fe to
071a523
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@requirements.overrides.txt`:
- Line 15: Update the setuptools entry in requirements.overrides.txt by
replacing the incomplete version string "setuptools==80.9" with the full
semantic version "setuptools==80.9.0"; locate the line containing the exact
token setuptools==80.9 and change it to setuptools==80.9.0 so it matches the
version in requirements.hashes.wheel.txt and PyPI.
In `@scripts/konflux_requirements.sh`:
- Line 45: Replace the sed call that strips the "==..." suffix with bash
parameter expansion to avoid spawning a subshell: update the assignment to
compute package_name from current_package using the longest-match suffix removal
(use the %% pattern) so package_name contains current_package up to but
excluding "==...". Replace the line that sets package_name using sed with an
equivalent using parameter expansion (refer to the variables package_name and
current_package and the removal pattern using %%==*).
🧹 Nitpick comments (1)
scripts/konflux_requirements.sh (1)
53-54: Fragile substring matching may cause false positives.The check
[[ "$NO_WHEEL_PACKAGES" == *"$package_name"* ]]performs substring matching. If a package namedsafeexisted, it would incorrectly matchmarkupsafe. Consider using word-boundary matching or array-based lookup for robustness.Proposed fix using word boundaries
# packages to exclude from the wheel list -NO_WHEEL_PACKAGES="markupsafe" +NO_WHEEL_PACKAGES=",markupsafe," ... - elif [[ "$NO_WHEEL_PACKAGES" == *"$package_name"* ]]; then + elif [[ "$NO_WHEEL_PACKAGES" == *",$package_name,"* ]]; thenAlternatively, use an associative array for O(1) lookup if the exclusion list grows.
Signed-off-by: Haoyu Sun <hasun@redhat.com>
071a523 to
f00b859
Compare
|
|
||
| uv pip compile "$WHEEL_FILE" -o "$WHEEL_HASH_FILE" --refresh --generate-hashes --index-url https://console.redhat.com/api/pypi/public-rhai/rhoai/3.2/cpu-ubi9/simple/ --python-version 3.12 --emit-index-url --no-deps --no-annotate --universal | ||
| uv pip compile "$SOURCE_FILE" -o "$SOURCE_HASH_FILE" --refresh --generate-hashes --python-version 3.12 --emit-index-url --no-deps --no-annotate | ||
| uv pip compile "$WHEEL_FILE" --refresh --generate-hashes --index-url https://console.redhat.com/api/pypi/public-rhai/rhoai/3.2/cpu-ubi9/simple/ --python-version 3.12 --emit-index-url --no-deps --no-annotate --universal > "$WHEEL_HASH_FILE" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the fix to pip compile problem, strange but it works :P
Description
Fix a bug from pip compile that the hash generated in the output file designated by
-oparameter is different from the stdout. For example the packageaiohappyeyeballshas different hash generated in output file and stdout.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
✏️ Tip: You can customize this high-level summary in your review settings.