Skip to content

Commit 59e698b

Browse files
refactor(resolver): make Cooldown always present, never None
Replace None-as-disabled with a zero min_age Cooldown so callers never need None checks. Adds Cooldown.disabled() factory and simplifies resolve_package_cooldown to always return a value. Co-Authored-By: Claude <claude@anthropic.com> Signed-off-by: Lalatendu Mohanty <lmohanty@redhat.com>
1 parent abcfd23 commit 59e698b

7 files changed

Lines changed: 67 additions & 75 deletions

File tree

src/fromager/__main__.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,11 +283,7 @@ def main(
283283
network_isolation=network_isolation,
284284
max_jobs=jobs,
285285
settings_dir=settings_dir,
286-
cooldown=(
287-
candidate.Cooldown(min_age=datetime.timedelta(days=min_release_age))
288-
if min_release_age > 0
289-
else None
290-
),
286+
cooldown=candidate.Cooldown(min_age=datetime.timedelta(days=min_release_age)),
291287
)
292288
wkctx.setup()
293289
ctx.obj = wkctx

src/fromager/candidate.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ class Cooldown:
2222
Frozen so that cooldown policy cannot be accidentally weakened after
2323
construction — all parameters are set once and shared read-only.
2424
25+
A cooldown with ``min_age`` of zero (or negative) is effectively disabled —
26+
every package age exceeds the threshold. Use :meth:`disabled` as a
27+
convenient factory instead of ``None``.
28+
2529
bootstrap_time is fixed at construction so all resolutions in a single run
2630
share the same cutoff.
2731
@@ -35,6 +39,11 @@ class Cooldown:
3539
)
3640
exempt_versions: frozenset[Version] = dataclasses.field(default_factory=frozenset)
3741

42+
@classmethod
43+
def disabled(cls) -> "Cooldown":
44+
"""Return a cooldown that never filters any candidate."""
45+
return cls(min_age=datetime.timedelta(0))
46+
3847

3948
@dataclasses.dataclass(frozen=True, order=True, slots=True, repr=False, kw_only=True)
4049
class Candidate:

src/fromager/context.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
external_commands,
2121
packagesettings,
2222
)
23+
from .candidate import Cooldown
2324

2425
if typing.TYPE_CHECKING:
25-
from . import build_environment, candidate
26+
from . import build_environment
2627

2728
logger = logging.getLogger(__name__)
2829

@@ -47,7 +48,7 @@ def __init__(
4748
max_jobs: int | None = None,
4849
settings_dir: pathlib.Path | None = None,
4950
wheel_server_url: str = "",
50-
cooldown: candidate.Cooldown | None = None,
51+
cooldown: Cooldown | None = None,
5152
max_release_age: datetime.timedelta | None = None,
5253
):
5354
if active_settings is None:
@@ -95,7 +96,9 @@ def __init__(
9596

9697
self._parallel_builds = False
9798

98-
self.cooldown: candidate.Cooldown | None = cooldown
99+
self.cooldown: Cooldown = (
100+
cooldown if cooldown is not None else Cooldown.disabled()
101+
)
99102
self._max_release_age: datetime.timedelta | None = max_release_age
100103

101104
@property

src/fromager/finders.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ def __init__(
6161
ignore_platform=False,
6262
use_resolver_cache=use_resolver_cache,
6363
override_download_url=None,
64-
cooldown=None,
6564
supports_upload_time=False,
6665
)
6766

src/fromager/resolver.py

Lines changed: 24 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -164,68 +164,57 @@ def _get_toplevel_pinned_versions(
164164
def _resolve_cooldown_params(
165165
ctx: context.WorkContext,
166166
req: Requirement,
167-
) -> tuple[datetime.timedelta, datetime.datetime] | None:
167+
) -> tuple[datetime.timedelta, datetime.datetime]:
168168
"""Resolve min_age and bootstrap_time for a package's cooldown.
169169
170-
Returns ``None`` when cooldown is disabled (no global/per-package
171-
configuration, or a per-package override of 0). Otherwise returns
172-
the effective ``(min_age, bootstrap_time)`` pair.
170+
Always returns a ``(min_age, bootstrap_time)`` pair. When cooldown is
171+
disabled (per-package override of 0, or no global/per-package config),
172+
``min_age`` will be zero — every package age exceeds that threshold.
173173
"""
174174
min_age_override = ctx.package_build_info(req).resolver_min_release_age
175175

176-
if ctx.cooldown is None and min_age_override is None:
177-
return None
178176
if min_age_override == 0:
179-
return None
180-
181-
if min_age_override is not None:
177+
min_age = datetime.timedelta(0)
178+
elif min_age_override is not None:
182179
min_age = datetime.timedelta(days=min_age_override)
183-
elif ctx.cooldown is not None:
184-
min_age = ctx.cooldown.min_age
185180
else:
186-
return None
181+
min_age = ctx.cooldown.min_age
187182

188-
bootstrap_time = (
189-
ctx.cooldown.bootstrap_time
190-
if ctx.cooldown is not None
191-
else datetime.datetime.now(datetime.UTC)
192-
)
193-
return min_age, bootstrap_time
183+
return min_age, ctx.cooldown.bootstrap_time
194184

195185

196186
def resolve_package_cooldown(
197187
ctx: context.WorkContext,
198188
req: Requirement,
199189
req_type: RequirementType | None = None,
200-
) -> Cooldown | None:
190+
) -> Cooldown:
201191
"""Compute the effective cooldown for a single package.
202192
203-
Returns ``None`` (cooldown disabled) when:
193+
Always returns a ``Cooldown`` instance. A ``min_age`` of zero means
194+
cooldown is effectively disabled — every package age exceeds zero.
195+
196+
Returns a *disabled* cooldown (min_age=0) when:
204197
205198
* The requirement is a top-level exact ``==`` pin — the user explicitly
206199
approved that version.
207200
* A per-package ``min_release_age=0`` override disables cooldown.
208201
* No global cooldown is configured and no per-package override enables one.
209202
210-
Otherwise a ``Cooldown`` is returned with:
203+
Otherwise returns an *active* cooldown with:
211204
212205
* *min_age* from the per-package override (if set) or the global cooldown.
213206
* *bootstrap_time* inherited from the global cooldown (for a consistent
214-
cutoff across the entire run) or the current time.
207+
cutoff across the entire run).
215208
* *exempt_versions* populated from top-level exact-pinned entries in the
216209
dependency graph, so transitive resolutions of the same package honour
217210
the user's explicit pin.
218211
"""
219212
if req_type == RequirementType.TOP_LEVEL and _has_equality_pin(req):
220-
if ctx.cooldown is not None:
213+
if ctx.cooldown.min_age:
221214
logger.info("cooldown bypassed as the top-level requirement uses == pin")
222-
return None
223-
224-
params = _resolve_cooldown_params(ctx, req)
225-
if params is None:
226-
return None
215+
return Cooldown.disabled()
227216

228-
min_age, bootstrap_time = params
217+
min_age, bootstrap_time = _resolve_cooldown_params(ctx, req)
229218
return Cooldown(
230219
min_age=min_age,
231220
bootstrap_time=bootstrap_time,
@@ -243,12 +232,7 @@ def _compute_max_age_cutoff(
243232
"""
244233
if ctx.max_release_age is None:
245234
return None
246-
bootstrap_time = (
247-
ctx.cooldown.bootstrap_time
248-
if ctx.cooldown is not None
249-
else datetime.datetime.now(datetime.UTC)
250-
)
251-
return bootstrap_time - ctx.max_release_age
235+
return ctx.cooldown.bootstrap_time - ctx.max_release_age
252236

253237

254238
def extract_filename_from_url(url: str) -> str:
@@ -609,8 +593,9 @@ def __init__(
609593
self.req_type = req_type
610594
self.use_cache_candidates = use_resolver_cache
611595

612-
# cooldown specific settings
613-
self.cooldown = cooldown
596+
self.cooldown: Cooldown = (
597+
cooldown if cooldown is not None else Cooldown.disabled()
598+
)
614599
# Does this provider supply upload timestamps for candidates?
615600
# Defaults to False (safe/unknown). Subclasses that reliably populate
616601
# upload_time on every candidate should set this to True in their __init__.
@@ -732,7 +717,7 @@ def is_satisfied_by(self, requirement: Requirement, candidate: Candidate) -> boo
732717
def is_blocked_by_cooldown(self, candidate: Candidate) -> bool:
733718
"""Return True if the candidate is rejected by the release-age cooldown."""
734719

735-
if self.cooldown is None:
720+
if not self.cooldown.min_age:
736721
return False
737722

738723
if candidate.version in self.cooldown.exempt_versions:
@@ -981,7 +966,7 @@ def _get_no_match_error_message(
981966

982967
# If a cooldown is active, check whether it's responsible for the
983968
# failure so we can give a more actionable error message.
984-
if self.cooldown is not None:
969+
if self.cooldown.min_age:
985970
cutoff = self.cooldown.bootstrap_time - self.cooldown.min_age
986971
all_candidates = list(self._find_cached_candidates(identifier))
987972
missing_time = [c for c in all_candidates if c.upload_time is None]

0 commit comments

Comments
 (0)