Skip to content

Commit 5b048bd

Browse files
rd4398claude
andcommitted
fix(resolver): raise ResolverException on cutoff, validate age, reduce log noise
- Raise ResolverException instead of ValueError when all candidates are older than max-release-age cutoff, consistent with resolver error handling. - Validate that max_release_age is positive in set_max_release_age(). - Move "found N candidate(s)" log inside the max_age_cutoff block so it only fires when filtering is active. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
1 parent 43ac347 commit 5b048bd

3 files changed

Lines changed: 28 additions & 26 deletions

File tree

src/fromager/context.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ def max_release_age(self) -> datetime.timedelta | None:
122122

123123
def set_max_release_age(self, days: int) -> None:
124124
"""Set the maximum release age in days."""
125+
if days < 1:
126+
raise ValueError(f"max_release_age must be positive, got {days}")
125127
self._max_release_age = datetime.timedelta(days=days)
126128

127129
def enable_parallel_builds(self) -> None:

src/fromager/resolver.py

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -237,9 +237,15 @@ def find_all_matching_from_provider(
237237
provider: The provider to query for candidates.
238238
req: The requirement to match.
239239
max_age_cutoff: If set, reject candidates published before this time.
240+
Raises ``ResolverException`` if all candidates are older than
241+
the cutoff.
240242
241243
Returns list of (url, version) tuples sorted by version (highest first).
242244
245+
Raises:
246+
ResolverException: If ``max_age_cutoff`` is set and every candidate
247+
is older than the cutoff.
248+
243249
IMPORTANT: This bypasses resolvelib's full resolver to collect all matching
244250
candidates. This is safe ONLY because BaseProvider.get_dependencies() returns
245251
an empty list (no transitive dependencies to resolve). The empty incompatibilities
@@ -272,38 +278,35 @@ def find_all_matching_from_provider(
272278

273279
# Materialize candidates so we can iterate more than once if filtering
274280
candidates_list = list(candidates)
275-
logger.info(
276-
"%s: found %d candidate(s) matching %s",
277-
req.name,
278-
len(candidates_list),
279-
req,
280-
)
281281

282282
if max_age_cutoff is not None:
283+
logger.info(
284+
"%s: found %d candidate(s) matching %s",
285+
req.name,
286+
len(candidates_list),
287+
req,
288+
)
283289
max_age_days = (datetime.datetime.now(datetime.UTC) - max_age_cutoff).days
284290
filtered = [
285291
c
286292
for c in candidates_list
287293
if c.upload_time is None or c.upload_time >= max_age_cutoff
288294
]
289-
if filtered:
295+
dropped = len(candidates_list) - len(filtered)
296+
if dropped:
290297
logger.info(
291298
"%s: have %d candidate(s) of %s published within %d days",
292299
req.name,
293300
len(filtered),
294301
req,
295302
max_age_days,
296303
)
297-
candidates_list = filtered
298-
else:
299-
logger.warning(
300-
"%s: all %d candidate(s) of %s are older than %d days, "
301-
"keeping all to avoid empty resolution",
302-
req.name,
303-
len(candidates_list),
304-
req,
305-
max_age_days,
304+
if not filtered:
305+
raise resolvelib.resolvers.ResolverException(
306+
f"all {len(candidates_list)} candidate(s) for {req} were "
307+
f"published before the max-release-age cutoff"
306308
)
309+
candidates_list = filtered
307310

308311
# Convert candidates to list of (url, version) tuples
309312
# Candidates are sorted by version (highest first) by BaseProvider.find_matches()

tests/test_cooldown.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -759,10 +759,8 @@ def test_max_release_age_disabled_returns_all() -> None:
759759
assert "1.2.2" in versions
760760

761761

762-
def test_max_release_age_all_too_old_keeps_all(
763-
caplog: pytest.LogCaptureFixture,
764-
) -> None:
765-
"""When all versions are older than cutoff, keep all to avoid empty resolution."""
762+
def test_max_release_age_all_too_old_raises() -> None:
763+
"""When all versions are older than cutoff, raise ResolverException."""
766764
max_age_cutoff = _BOOTSTRAP_TIME + datetime.timedelta(days=1)
767765
with requests_mock.Mocker() as r:
768766
r.get(
@@ -771,15 +769,14 @@ def test_max_release_age_all_too_old_keeps_all(
771769
headers={"Content-Type": _PYPI_SIMPLE_JSON_CONTENT_TYPE},
772770
)
773771
provider = resolver.PyPIProvider(include_sdists=True)
774-
with caplog.at_level(logging.WARNING, logger="fromager.resolver"):
775-
results = resolver.find_all_matching_from_provider(
772+
with pytest.raises(
773+
resolvelib.resolvers.ResolverException,
774+
match=r"all 3 candidate.*max-release-age cutoff",
775+
):
776+
resolver.find_all_matching_from_provider(
776777
provider, Requirement("test-pkg"), max_age_cutoff=max_age_cutoff
777778
)
778779

779-
versions = [str(v) for _, v in results]
780-
assert len(versions) == 3
781-
assert "keeping all to avoid empty resolution" in caplog.text
782-
783780

784781
def test_max_release_age_candidates_without_upload_time_pass_through() -> None:
785782
"""Candidates without upload_time are not filtered out by max-release-age."""

0 commit comments

Comments
 (0)