Skip to content

Commit 35c8ffc

Browse files
rd4398claude
andcommitted
refactor(bootstrap): encapsulate max_release_age and improve filter logging
Encapsulate max_release_age behind set_max_release_age(days) method on WorkContext to prevent callers from setting it to arbitrary types. Restructure filtering logs to show the pipeline: total candidates found, then candidates remaining after each filter step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Rohan Devasthale <rdevasth@redhat.com>
1 parent 58b9c20 commit 35c8ffc

5 files changed

Lines changed: 40 additions & 19 deletions

File tree

e2e/test_bootstrap_max_release_age.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ fi
7676
# Verify that max-release-age filtering was applied (check log)
7777
echo ""
7878
echo "Checking log for max-release-age filtering..."
79-
if grep -q "filtered.*versions by max release age" "$OUTDIR/bootstrap.log"; then
79+
if grep -q "published within.*days" "$OUTDIR/bootstrap.log"; then
8080
echo "✓ Log confirms max-release-age filtering was applied"
8181
else
8282
echo "✗ No max-release-age filtering found in log"

src/fromager/commands/bootstrap.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ def bootstrap(
181181
skip_constraints = True
182182

183183
if max_release_age > 0:
184-
wkctx.max_release_age = timedelta(days=max_release_age)
184+
wkctx.set_max_release_age(max_release_age)
185185
logger.info(
186186
"max release age: rejecting versions older than %d days",
187187
max_release_age,

src/fromager/context.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,15 @@ def __init__(
114114
self._parallel_builds = False
115115

116116
self.cooldown: Cooldown | None = cooldown
117-
self.max_release_age: datetime.timedelta | None = max_release_age
117+
self._max_release_age: datetime.timedelta | None = max_release_age
118+
119+
@property
120+
def max_release_age(self) -> datetime.timedelta | None:
121+
return self._max_release_age
122+
123+
def set_max_release_age(self, days: int) -> None:
124+
"""Set the maximum release age in days."""
125+
self._max_release_age = datetime.timedelta(days=days)
118126

119127
def enable_parallel_builds(self) -> None:
120128
self._parallel_builds = True

src/fromager/resolver.py

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -272,29 +272,39 @@ def find_all_matching_from_provider(
272272

273273
# Materialize candidates so we can iterate more than once if filtering
274274
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+
)
275281

276282
if max_age_cutoff is not None:
283+
max_age_days = (
284+
datetime.datetime.now(datetime.UTC) - max_age_cutoff
285+
).days
277286
filtered = [
278287
c
279288
for c in candidates_list
280289
if c.upload_time is None or c.upload_time >= max_age_cutoff
281290
]
282291
if filtered:
283-
dropped = len(candidates_list) - len(filtered)
284-
if dropped > 0:
285-
logger.info(
286-
"%s: filtered %d/%d versions by max release age",
287-
req.name,
288-
dropped,
289-
len(candidates_list),
290-
)
292+
logger.info(
293+
"%s: have %d candidate(s) of %s published within %d days",
294+
req.name,
295+
len(filtered),
296+
req,
297+
max_age_days,
298+
)
291299
candidates_list = filtered
292300
else:
293301
logger.warning(
294-
"%s: all %d versions are older than max release age cutoff, "
295-
"keeping all versions to avoid empty resolution",
302+
"%s: all %d candidate(s) of %s are older than %d days, "
303+
"keeping all to avoid empty resolution",
296304
req.name,
297305
len(candidates_list),
306+
req,
307+
max_age_days,
298308
)
299309

300310
# Convert candidates to list of (url, version) tuples

tests/test_cooldown.py

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,9 @@ def test_max_release_age_filters_old_versions(
711711
assert "2.0.0" in versions
712712
assert "1.3.2" in versions
713713
assert "1.2.2" not in versions
714-
assert "filtered 1/3 versions by max release age" in caplog.text
714+
assert "found 3 candidate(s)" in caplog.text
715+
assert "have 2 candidate(s)" in caplog.text
716+
assert "published within" in caplog.text
715717

716718

717719
def test_max_release_age_keeps_only_recent(
@@ -733,7 +735,9 @@ def test_max_release_age_keeps_only_recent(
733735

734736
versions = [str(v) for _, v in results]
735737
assert versions == ["2.0.0"]
736-
assert "filtered 2/3 versions by max release age" in caplog.text
738+
assert "found 3 candidate(s)" in caplog.text
739+
assert "have 1 candidate(s)" in caplog.text
740+
assert "published within" in caplog.text
737741

738742

739743
def test_max_release_age_disabled_returns_all() -> None:
@@ -774,7 +778,7 @@ def test_max_release_age_all_too_old_keeps_all(
774778

775779
versions = [str(v) for _, v in results]
776780
assert len(versions) == 3
777-
assert "keeping all versions to avoid empty resolution" in caplog.text
781+
assert "keeping all to avoid empty resolution" in caplog.text
778782

779783

780784
def test_max_release_age_candidates_without_upload_time_pass_through() -> None:
@@ -833,7 +837,7 @@ def test_compute_max_age_cutoff_with_cooldown(
833837
min_age=datetime.timedelta(days=7),
834838
bootstrap_time=_BOOTSTRAP_TIME,
835839
)
836-
tmp_context.max_release_age = datetime.timedelta(days=30)
840+
tmp_context.set_max_release_age(30)
837841
cutoff = resolver._compute_max_age_cutoff(tmp_context)
838842
assert cutoff == _BOOTSTRAP_TIME - datetime.timedelta(days=30)
839843

@@ -843,7 +847,7 @@ def test_compute_max_age_cutoff_without_cooldown(
843847
) -> None:
844848
"""_compute_max_age_cutoff uses current time when no cooldown is set."""
845849
tmp_context.cooldown = None
846-
tmp_context.max_release_age = datetime.timedelta(days=30)
850+
tmp_context.set_max_release_age(30)
847851
cutoff = resolver._compute_max_age_cutoff(tmp_context)
848852
assert cutoff is not None
849853
expected = datetime.datetime.now(datetime.UTC) - datetime.timedelta(days=30)
@@ -854,6 +858,5 @@ def test_compute_max_age_cutoff_disabled(
854858
tmp_context: context.WorkContext,
855859
) -> None:
856860
"""_compute_max_age_cutoff returns None when max_release_age is not set."""
857-
tmp_context.max_release_age = None
858861
cutoff = resolver._compute_max_age_cutoff(tmp_context)
859862
assert cutoff is None

0 commit comments

Comments
 (0)