Skip to content

Commit 2495247

Browse files
dhellmannclaude
andcommitted
refactor(bootstrapper): apply review follow-ups to _cache module
Add docstring to `Bootstrapper` class describing it as an iterative DFS engine to be used as a context manager. Rename private `_cache` functions that cross module boundaries to drop the leading underscore, reflecting their role as internal-but-public APIs within the bootstrapper package: - `_find_cached_wheel` → `find_cached_wheel` - `_bg_prepare_prebuilt` → `bg_prepare_prebuilt` Update all call sites in `_bootstrapper.py`, `_prepare_source.py`, `_resolve.py`, and tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Doug Hellmann <dhellmann@redhat.com>
1 parent fdf55b3 commit 2495247

6 files changed

Lines changed: 23 additions & 17 deletions

File tree

src/fromager/bootstrapper/_bootstrapper.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@
4949

5050

5151
class Bootstrapper:
52+
"""Iterative DFS bootstrap engine for resolving and building packages.
53+
54+
Use as a context manager. Call ``bootstrap(requirements)`` to process
55+
the dependency tree, then ``finalize()`` for cleanup and exit code.
56+
"""
57+
5258
def __init__(
5359
self,
5460
ctx: context.WorkContext,
@@ -555,7 +561,7 @@ def _download_prebuilt(
555561
resolved_version: Version,
556562
wheel_url: str,
557563
) -> tuple[pathlib.Path, pathlib.Path]:
558-
result = _cache._bg_prepare_prebuilt(
564+
result = _cache.bg_prepare_prebuilt(
559565
self.ctx, req, req_type, resolved_version, wheel_url
560566
)
561567
assert result.wheel_filename is not None

src/fromager/bootstrapper/_cache.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ def _download_wheel_from_cache(
171171
return None, None
172172

173173

174-
def _find_cached_wheel(
174+
def find_cached_wheel(
175175
ctx: context.WorkContext,
176176
cache_wheel_server_url: str | None,
177177
req: Requirement,
@@ -209,7 +209,7 @@ def _find_cached_wheel(
209209
return None, None
210210

211211

212-
def _bg_prepare_prebuilt(
212+
def bg_prepare_prebuilt(
213213
ctx: context.WorkContext,
214214
req: Requirement,
215215
req_type: RequirementType,

src/fromager/bootstrapper/_prepare_source.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def _bg_prepare_source(
3535
# Paths from download_source() and prepare_source() include {name}-{version},
3636
# making them unique across concurrent threads processing different packages.
3737
logger.info("preparing source")
38-
cached_wheel, unpacked = _cache._find_cached_wheel(
38+
cached_wheel, unpacked = _cache.find_cached_wheel(
3939
ctx, cache_wheel_server_url, req, resolved_version
4040
)
4141
if unpacked is not None:
@@ -87,7 +87,7 @@ def background_work(
8787

8888
def do_prepare_prebuilt() -> PreparedSourceData:
8989
with req_ctxvar_context(req, resolved_version):
90-
return _cache._bg_prepare_prebuilt(
90+
return _cache.bg_prepare_prebuilt(
9191
ctx, req, req_type, resolved_version, source_url
9292
)
9393

src/fromager/bootstrapper/_resolve.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ def run(self, bt: Bootstrapper) -> list[Phase]:
112112
)
113113
filtered: list[tuple[str, Version]] = []
114114
for source_url, version in resolved_versions:
115-
cached_wheel, _ = _cache._find_cached_wheel(
115+
cached_wheel, _ = _cache.find_cached_wheel(
116116
bt.ctx, bt.cache_wheel_server_url, self.work_item.req, version
117117
)
118118
if cached_wheel:

tests/test_bootstrapper.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
1414
from fromager import bootstrapper, log
1515
from fromager.bootstrapper._build import Build
1616
from fromager.bootstrapper._cache import (
17-
_bg_prepare_prebuilt,
1817
_download_wheel_from_cache,
19-
_find_cached_wheel,
18+
bg_prepare_prebuilt,
19+
find_cached_wheel,
2020
)
2121
from fromager.bootstrapper._phase import Phase
2222
from fromager.bootstrapper._prepare_source import (
@@ -292,11 +292,11 @@ def test_is_build_requirement(tmp_context: WorkContext) -> None:
292292

293293

294294
def test_find_cached_wheel_returns_tuple(tmp_context: WorkContext) -> None:
295-
"""Verify _find_cached_wheel returns tuple of (Path|None, Path|None)."""
295+
"""Verify `find_cached_wheel` returns tuple of (Path|None, Path|None)."""
296296
bt = bootstrapper.Bootstrapper(tmp_context)
297297

298298
# Call method (will return None, None since no wheels exist)
299-
result = _find_cached_wheel(
299+
result = find_cached_wheel(
300300
bt.ctx,
301301
bt.cache_wheel_server_url,
302302
req=Requirement("test-package"),
@@ -897,7 +897,7 @@ def test_bg_prepare_source_log_prefix_includes_version(
897897
with (
898898
caplog.at_level(logging.INFO, logger="fromager.bootstrapper"),
899899
patch(
900-
"fromager.bootstrapper._cache._find_cached_wheel",
900+
"fromager.bootstrapper._cache.find_cached_wheel",
901901
return_value=(None, None),
902902
),
903903
patch(
@@ -937,7 +937,7 @@ def test_bg_prepare_prebuilt_log_prefix_includes_version(
937937
tmp_context: WorkContext,
938938
caplog: pytest.LogCaptureFixture,
939939
) -> None:
940-
"""_bg_prepare_prebuilt log message includes name-version prefix when called with context."""
940+
"""`bg_prepare_prebuilt` log message includes name-version prefix when called with context."""
941941
old_factory = logging.getLogRecordFactory()
942942
logging.setLogRecordFactory(log.FromagerLogRecord)
943943
req = Requirement("mypkg==1.2.3")
@@ -954,7 +954,7 @@ def test_bg_prepare_prebuilt_log_prefix_includes_version(
954954
patch("fromager.server.update_wheel_mirror"),
955955
log.req_ctxvar_context(req, version),
956956
):
957-
_bg_prepare_prebuilt(
957+
bg_prepare_prebuilt(
958958
ctx=tmp_context,
959959
req=req,
960960
req_type=RequirementType.INSTALL,

tests/test_bootstrapper_iterative.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ def mock_cache(
425425
return (None, None)
426426

427427
with patch(
428-
"fromager.bootstrapper._cache._find_cached_wheel", side_effect=mock_cache
428+
"fromager.bootstrapper._cache.find_cached_wheel", side_effect=mock_cache
429429
):
430430
result = item.run(bt)
431431

@@ -446,7 +446,7 @@ def test_all_cached_keeps_highest_version(self, tmp_context: WorkContext) -> Non
446446
)
447447

448448
with patch(
449-
"fromager.bootstrapper._cache._find_cached_wheel",
449+
"fromager.bootstrapper._cache.find_cached_wheel",
450450
return_value=(tmp_context.work_dir / "cached.whl", None),
451451
):
452452
result = item.run(bt)
@@ -462,7 +462,7 @@ def test_no_filtering_in_single_version_mode(
462462
item = _make_resolve_item()
463463
item.bg_future = _make_resolved_future([("url-1.0", Version("1.0"))])
464464

465-
with patch("fromager.bootstrapper._cache._find_cached_wheel") as mock_cache:
465+
with patch("fromager.bootstrapper._cache.find_cached_wheel") as mock_cache:
466466
result = item.run(bt)
467467

468468
assert len(result) == 1
@@ -499,7 +499,7 @@ def test_filters_failed_versions_in_multiple_versions_mode(
499499
)
500500

501501
with patch(
502-
"fromager.bootstrapper._cache._find_cached_wheel", return_value=(None, None)
502+
"fromager.bootstrapper._cache.find_cached_wheel", return_value=(None, None)
503503
):
504504
result = item.run(bt)
505505

0 commit comments

Comments
 (0)