fix(wheels): disable cooldown for local and cache wheel servers by having separate cache wheel resolution from pre-built path

[smoparth]

Use PyPICacheProvider (introduced in #1143) for trusted local and cache wheel server lookups in the build and download-sequence paths. A package wheel_server_url stays on the pre-built path, where the release-age cooldown still applies.

• Add resolve_cached_wheel() using PyPICacheProvider
• Look up an existing wheel with get_wheel_server_urls(): a package wheel_server_url goes through resolve_prebuilt_wheel; the local server and --cache-wheel-server-url go through resolve_cached_wheel
• Route download_sequence local server through resolve_cached_wheel
• Remove supports_upload_time workaround from resolve_all_prebuilt_wheels
• Add tests for cache cooldown bypass, package wheel_server_url exclusivity, and upstream cooldown retention

Follows up on #1143 and closes #1140.

Pull Request Description

What

Why

• PR follows CONTRIBUTING.md guidelines

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR threads a new cache_wheel_server_url through build and download paths, adds resolve_cached_wheel and cache-first resolution to prefer local/cache servers when locating prebuilt wheels, and updates acquisition to use a local cached file when the resolved URL is the local wheel server. resolve_all_prebuilt_wheels no longer special-cases local servers for upload-time support. Tests were added/updated to assert that local and cache wheel servers skip cooldown warnings while external servers retain cooldown behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	All changes directly address #1140 requirements: resolve_cached_wheel and resolve_existing_wheel implement cache server handling without cooldown, tests verify cooldown bypass for cache/local servers …
Out of Scope Changes check	✅ Passed	All changes are scoped to disabling cooldown for cache/local servers and refactoring wheel resolution to separate cache lookup from pre-built resolution, with no unrelated modifications detected.
Title check	✅ Passed	The title accurately describes the main change: disabling cooldown for local and cache wheel servers by implementing separate cache wheel resolution logic.
Description check	✅ Passed	The pull request description directly matches the changes. It covers PyPICacheProvider usage, cooldown behavior, wheel resolution paths, and the added tests.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[smoparth]

Considered storing cache_wheel_server_url on WorkContext directly, but PR #1065 already proposes that (as cache_wheel_server_urls on ctx). Kept this fix minimal by threading it as a function parameter; once #1065 lands, the parameter can be replaced with ctx.cache_wheel_server_urls.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fromager/wheels.py`:
- Around line 556-565: The equality checks against ctx.wheel_server_url and
cache_wheel_server_url can fail due to URL normalization differences (trailing
slashes, etc.); update the comparisons in the block that assigns
provider.cooldown to None so they compare normalized URLs instead of raw strings
— normalize `url`, `ctx.wheel_server_url`, and `cache_wheel_server_url` (e.g.,
parse and rebuild or at minimum strip trailing slashes and ensure consistent
scheme/netloc/path) before doing the equality checks, and then set
provider.cooldown = None when the normalized values match.

🪄 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: 13501074-a382-4bd6-bbb9-92f828557ff1

📥 Commits

Reviewing files that changed from the base of the PR and between 7b10135 and f1ca260.

📒 Files selected for processing (3)

• src/fromager/commands/build.py
• src/fromager/wheels.py
• tests/test_cooldown.py

[tiran]

This PR feels like a hack on top of a hack. The code is becoming increasingly confusing and hard to understand.

I think the root cause of the issue is the fact that code paths are mixing cache wheel server and PyPI server for pre-built wheels. Cache lookup is using the same code path as pre-built download. At least that's what I think is happening...

[LalatenduMohanty]

We should change this PR to use PyPICacheProvider for both the local and cache wheel server URLs inside resolve_all_prebuilt_wheels, rather than conditionally nulling out cooldown after the fact. refer #1143

[smoparth]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/fromager/commands/build.py (1)

524-532: 💤 Low value

Log wording: "prebuilt" no longer matches the function's scope.

_is_wheel_built now covers cache/local candidates too, not just prebuilt wheels. The "could not locate prebuilt wheel" log messages at lines 526 and 531 will be misleading when the failure is for a cache hit. Consider "existing" to match line 488.

♻️ Suggested change

-        logger.debug(
-            "could not locate prebuilt wheel %s-%s",
-            dist_name,
-            resolved_version,
-            exc_info=True,
-        )
-        logger.info("could not locate prebuilt wheel")
+        logger.debug(
+            "could not locate existing wheel %s-%s",
+            dist_name,
+            resolved_version,
+            exc_info=True,
+        )
+        logger.info("could not locate existing wheel")

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fromager/commands/build.py` around lines 524 - 532, The logs in
_is_wheel_built incorrectly say "prebuilt wheel" while the function also checks
cache/local candidates; update both logger calls that reference dist_name and
resolved_version (the logger.debug with exc_info and the logger.info that
returns None) to use wording like "existing wheel" or "existing candidate"
(consistent with earlier "existing" wording at line 488) so messages accurately
reflect cache/local/prebuilt scope.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/fromager/commands/build.py`:
- Around line 481-489: The call to wheels.resolve_existing_wheel is currently
executed before the try/except in _is_wheel_built, so any exceptions (e.g.,
transient network errors) escape and abort the build; move the
resolve_existing_wheel(...) invocation into the existing try block that
surrounds the logic at lines 491+ so that exceptions are caught and the function
returns None (falling back to source build) as before—adjust references to
ctx=wkctx, req=req, pbi=pbi, cache_wheel_server_url=cache_wheel_server_url and
preserve the existing logger.info("could not locate existing wheel")/return None
flow inside the except path.

---

Nitpick comments:
In `@src/fromager/commands/build.py`:
- Around line 524-532: The logs in _is_wheel_built incorrectly say "prebuilt
wheel" while the function also checks cache/local candidates; update both logger
calls that reference dist_name and resolved_version (the logger.debug with
exc_info and the logger.info that returns None) to use wording like "existing
wheel" or "existing candidate" (consistent with earlier "existing" wording at
line 488) so messages accurately reflect cache/local/prebuilt scope.

🪄 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: d96726c4-5302-4336-905e-1d3f306b488c

📥 Commits

Reviewing files that changed from the base of the PR and between eae3f0d and 19c9cd9.

📒 Files selected for processing (4)

• src/fromager/commands/build.py
• src/fromager/commands/download_sequence.py
• src/fromager/wheels.py
• tests/test_cooldown.py

🚧 Files skipped from review as they are similar to previous changes (3)

• tests/test_cooldown.py
• src/fromager/commands/download_sequence.py
• src/fromager/wheels.py

[rd4398]

This looks good! I will wait for @LalatenduMohanty to approve since he requested changes before

[tiran]

The function resolve_existing_wheel is conflating two different concepts. Let's treat pre-built wheels and cached wheels as separate entities.

• pre-built wheels come from PyPI or 3rd party wheel index. They should use cooldown.
• cached wheels are built by us and stored in a cache index that contains only our wheels + previously cached pre-built wheels.

[mergify]

rebase

✅ Branch has been successfully rebased

[LalatenduMohanty]

@smoparth Any package configured with a per-package wheel_server_url in its settings YAML (e.g.):

settings/some-pkg.yaml

wheel_server_url: https://custom-wheel.test/simple/

will no longer have that custom server checked for existing wheels in _is_wheel_built(). The function will only check the local and cache servers, won't find the wheel there, and proceed to rebuild the package from source unnecessarily.

Because the new _is_wheel_built() at src/fromager/commands/build.py:473-476 builds its server list like this:

  cache_servers: list[str] = list(
      filter(None, [wkctx.wheel_server_url, cache_wheel_server_url])
  )

It only considers the global local server and the cache server. pbi.wheel_server_url is never consulted. The call to get_wheel_server_urls() was removed from _build() entirely

[LalatenduMohanty]

Also we should add a test test for the pbi.wheel_server_url path.

[mergify]

This pull request has merge conflicts that must be resolved before it can be merged.
@smoparth please rebase your branch.

[rd4398]

The architectural direction is right, separating cache resolution from pre-built resolution at the provider level is cleaner than the supports_upload_time = False workaround. The test coverage is also strong. I have left some comments after which this should be good to land

[rd4398]

LGTM!
We should merge this as soon as possible as it is blocking downstream work for us.

[mergify]

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.