Skip to content

Commit 0d5711d

Browse files
authored
Merge pull request #1029 from LalatenduMohanty/fix/differentiate-network-errors-in-wheel-cache-lookup
fix(bootstrapper): log specific error types in wheel cache lookup
2 parents 1109dac + 7568402 commit 0d5711d

2 files changed

Lines changed: 158 additions & 1 deletion

File tree

src/fromager/bootstrapper.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@
1414
import zipfile
1515
from urllib.parse import urlparse
1616

17+
import requests.exceptions
1718
from packaging.requirements import Requirement
1819
from packaging.utils import NormalizedName, canonicalize_name
1920
from packaging.version import Version
21+
from resolvelib.resolvers import ResolverException
2022

2123
from . import (
2224
bootstrap_requirement_resolver,
@@ -1144,11 +1146,23 @@ def _download_wheel_from_cache(
11441146
req, resolved_version, cached_wheel
11451147
)
11461148
return cached_wheel, unpack_dir
1147-
except Exception:
1149+
except ResolverException:
11481150
logger.info(
11491151
f"did not find wheel for {resolved_version} in {self.cache_wheel_server_url}"
11501152
)
11511153
return None, None
1154+
except requests.exceptions.RequestException as err:
1155+
logger.warning(
1156+
f"network error checking wheel cache for {resolved_version} "
1157+
f"at {self.cache_wheel_server_url}: {err}"
1158+
)
1159+
return None, None
1160+
except Exception as err:
1161+
logger.warning(
1162+
f"unexpected error checking wheel cache for {resolved_version} "
1163+
f"at {self.cache_wheel_server_url}: {err}"
1164+
)
1165+
return None, None
11521166

11531167
def _unpack_metadata_from_wheel(
11541168
self, req: Requirement, resolved_version: Version, wheel_filename: pathlib.Path

tests/test_bootstrapper.py

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,12 @@
22
import pathlib
33
from unittest.mock import Mock, patch
44

5+
import pytest
6+
import requests.exceptions
57
from packaging.requirements import Requirement
68
from packaging.utils import canonicalize_name
79
from packaging.version import Version
10+
from resolvelib.resolvers import ResolverException
811

912
from fromager import bootstrapper, requirements_file
1013
from fromager.context import WorkContext
@@ -408,3 +411,143 @@ def test_download_wheel_from_cache_bypasses_hooks(
408411
constraints=tmp_context.constraints,
409412
)
410413
mock_find_all.assert_called_once_with(mock_provider, Requirement("test-pkg==1.0.0"))
414+
415+
416+
def _make_cache_bootstrapper(
417+
tmp_context: WorkContext,
418+
) -> bootstrapper.Bootstrapper:
419+
bt = bootstrapper.Bootstrapper(tmp_context)
420+
bt.cache_wheel_server_url = "https://cache.test/simple"
421+
return bt
422+
423+
424+
def test_cache_lookup_resolver_exception_logs_info(
425+
tmp_context: WorkContext,
426+
caplog: pytest.LogCaptureFixture,
427+
) -> None:
428+
"""ResolverException (wheel not found) returns (None, None) and logs info."""
429+
bt = _make_cache_bootstrapper(tmp_context)
430+
431+
with patch(
432+
"fromager.resolver.find_all_matching_from_provider",
433+
side_effect=ResolverException("no matching version"),
434+
):
435+
result = bt._download_wheel_from_cache(
436+
req=Requirement("test-package"),
437+
resolved_version=Version("1.0.0"),
438+
)
439+
440+
assert result == (None, None)
441+
assert "did not find wheel for" in caplog.text
442+
443+
444+
@pytest.mark.parametrize(
445+
"exc_class,exc_msg",
446+
[
447+
(requests.exceptions.ConnectionError, "DNS failure"),
448+
(requests.exceptions.Timeout, "timed out"),
449+
(requests.exceptions.HTTPError, "401 Unauthorized"),
450+
],
451+
)
452+
def test_cache_lookup_request_exception_logs_warning(
453+
tmp_context: WorkContext,
454+
caplog: pytest.LogCaptureFixture,
455+
exc_class: type[Exception],
456+
exc_msg: str,
457+
) -> None:
458+
"""RequestException subtypes return (None, None) and log warning."""
459+
bt = _make_cache_bootstrapper(tmp_context)
460+
461+
with patch(
462+
"fromager.resolver.find_all_matching_from_provider",
463+
side_effect=exc_class(exc_msg),
464+
):
465+
result = bt._download_wheel_from_cache(
466+
req=Requirement("test-package"),
467+
resolved_version=Version("1.0.0"),
468+
)
469+
470+
assert result == (None, None)
471+
assert "network error checking wheel cache" in caplog.text
472+
473+
474+
def test_cache_lookup_unexpected_exception_logs_warning(
475+
tmp_context: WorkContext,
476+
caplog: pytest.LogCaptureFixture,
477+
) -> None:
478+
"""Unexpected exceptions return (None, None) and log warning."""
479+
bt = _make_cache_bootstrapper(tmp_context)
480+
481+
with patch(
482+
"fromager.resolver.find_all_matching_from_provider",
483+
side_effect=ValueError("unexpected parsing error"),
484+
):
485+
result = bt._download_wheel_from_cache(
486+
req=Requirement("test-package"),
487+
resolved_version=Version("1.0.0"),
488+
)
489+
490+
assert result == (None, None)
491+
assert "unexpected error checking wheel cache" in caplog.text
492+
493+
494+
@pytest.mark.parametrize(
495+
"exc_class,exc_msg,expected_log",
496+
[
497+
(
498+
requests.exceptions.ConnectionError,
499+
"connection reset",
500+
"network error checking wheel cache",
501+
),
502+
(OSError, "disk full", "unexpected error checking wheel cache"),
503+
],
504+
)
505+
def test_cache_lookup_download_wheel_error_logs_warning(
506+
tmp_context: WorkContext,
507+
caplog: pytest.LogCaptureFixture,
508+
exc_class: type[Exception],
509+
exc_msg: str,
510+
expected_log: str,
511+
) -> None:
512+
"""Errors from download_wheel (after resolve succeeds) are caught."""
513+
bt = _make_cache_bootstrapper(tmp_context)
514+
515+
with (
516+
patch(
517+
"fromager.resolver.find_all_matching_from_provider",
518+
return_value=[
519+
(
520+
"https://cache.test/simple/test-package/test_package-1.0.0-py3-none-any.whl",
521+
Version("1.0.0"),
522+
),
523+
],
524+
),
525+
patch(
526+
"fromager.bootstrapper.wheels.extract_info_from_wheel_file",
527+
return_value=("test_package", "1.0.0", None, None),
528+
),
529+
patch(
530+
"fromager.bootstrapper.wheels.download_wheel",
531+
side_effect=exc_class(exc_msg),
532+
),
533+
):
534+
result = bt._download_wheel_from_cache(
535+
req=Requirement("test-package"),
536+
resolved_version=Version("1.0.0"),
537+
)
538+
539+
assert result == (None, None)
540+
assert expected_log in caplog.text
541+
542+
543+
def test_cache_lookup_no_cache_url_returns_none(tmp_context: WorkContext) -> None:
544+
"""When no cache URL is configured, returns (None, None) immediately."""
545+
bt = bootstrapper.Bootstrapper(tmp_context)
546+
bt.cache_wheel_server_url = ""
547+
548+
result = bt._download_wheel_from_cache(
549+
req=Requirement("test-package"),
550+
resolved_version=Version("1.0.0"),
551+
)
552+
553+
assert result == (None, None)

0 commit comments

Comments
 (0)