fix(bootstrap): don't call get_resolver_provider hook in cache lookups#1051
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/fromager/bootstrapper.py`:
- Around line 1113-1114: The code currently takes the first candidate from
resolver.find_all_matching_from_provider (results[0]) and treats it as a miss if
it fails the build-tag check; instead iterate over the results array for the
pinned_req and pick the first (wheel_url, metadata) whose
build-tag/compatibility check passes (the same logic used later around the
1124-1131 build-tag verification), only falling back to (None, None) if no
candidate matches; update both the immediate use (where results[0] is read) and
the similar block covering lines 1124-1131 to perform this filtered selection so
later valid cache entries aren’t ignored.
🪄 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: 7e540214-986a-4d2e-9422-b35ef46423a9
📒 Files selected for processing (2)
src/fromager/bootstrapper.pytests/test_bootstrapper.py
9abd5dd to
7a8f95a
Compare
|
Looks good to me. I would appreciate a second opinion from @rd4398 or @dhellmann . |
_download_wheel_from_cache() called resolver.resolve(), which triggered custom resolver hooks. Cache servers are always simple PyPI index servers and don't need hook-based resolution. Call default_resolver_provider() directly instead. Closes: python-wheel-build#1049 Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
|
Deprecation notice: This pull request comes from a fork and was rebased using |
7a8f95a to
986fb38
Compare
_download_wheel_from_cache() called resolver.resolve(), which triggered custom resolver hooks. Cache servers are always simple PyPI index servers and don't need hook-based resolution. Call default_resolver_provider() directly instead.
Closes: #1049