Tighten docstrings and comments

esphome/external_files.py

@@ -158,14 +158,11 @@ def has_remote_file_changed(
             }
             if etag := _read_etag(local_file_path):
                 headers[IF_NONE_MATCH] = etag
-            # Retried even though a failure degrades gracefully to the
-            # cached copy below: allow_stale=False consumers reject an
-            # unverified copy, so for them a healed flake avoids a hard
-            # failure in download_content. Only connection-level failures
-            # are retryable here; HEAD never raises on an HTTP status
-            # (deliberately: servers that reject HEAD with 405/501 fall
-            # through to "changed" and the GET below handles them), so a
-            # 5xx lands in the != 304 branch and the GET's retry covers it.
+            # Retried so allow_stale=False consumers don't hard-fail on a
+            # healed flake. Only connection-level failures retry: HEAD
+            # never raises on HTTP status (servers rejecting HEAD with
+            # 405/501 must fall through to the GET), so 5xx is handled by
+            # the GET's own retry.
             response = fetch_with_retry(
                 url,
                 lambda: requests.head(

esphome/net_retry.py

@@ -12,34 +12,22 @@ import time
 
 _LOGGER = logging.getLogger(__name__)
 
-# Transient network failures are retried with 2s/4s backoff, matching
-# git.py's _NETWORK_MAX_ATTEMPTS and framework_helpers' mirror downloads.
-# Worst case per fetch is NETWORK_MAX_ATTEMPTS timeouts plus 6s of sleeps;
-# callers are expected to memoize failures so a flaky host pays that at
-# most once per file per run.
+# 3 tries with 2s/4s backoff, matching git.py's _NETWORK_MAX_ATTEMPTS.
+# Callers memoize failures so a flaky host pays this once per file per run.
 NETWORK_MAX_ATTEMPTS = 3
 
 
 def _is_permanent_dns_failure(e: BaseException) -> bool:
-    """Whether a permanent socket.gaierror hides in ``e``'s exception chain.
+    """Whether a hard socket.gaierror hides in ``e``'s exception chain.
 
-    EAI_AGAIN ("temporary failure in name resolution", the flaky container
-    resolver case) stays retryable; only hard resolution failures like
-    NXDOMAIN count. Everything except EAI_AGAIN is treated as permanent:
-    deliberately narrower than git.py, which retries NXDOMAIN too. Codes
-    like EAI_NONAME can occur during a brief network outage, but a 6s
-    backoff rarely outlives one, and permanent means callers with a cached
-    copy fall back to it immediately instead of sleeping first (the
-    offline-build case). The trade-off is that a hard resolver failure on
-    a first download fails without retrying, same as before retries
-    existed.
+    EAI_AGAIN (flaky resolver) stays retryable; anything else is permanent
+    so offline builds fall back to their cache without sleeping first.
+    Narrower than git.py, which retries NXDOMAIN too.
 
-    Only the deliberate chain is walked: ``__cause__``, ``args`` (requests
-    wraps MaxRetryError as ``ConnectionError(e)`` without ``from``) and
-    urllib3 MaxRetryError's ``reason`` attribute (NameResolutionError never
-    lands on the cause chain). Implicit ``__context__`` is skipped so a
-    resolution failure from an unrelated earlier attempt cannot reclassify
-    an error it did not cause.
+    Walks ``__cause__``, ``args`` (requests wraps MaxRetryError without
+    ``from``) and MaxRetryError's ``reason``, but not implicit
+    ``__context__``: an unrelated earlier attempt's resolution failure
+    must not reclassify an error it did not cause.
     """
     import socket
 
@@ -71,10 +59,8 @@ def _is_permanent_dns_failure(e: BaseException) -> bool:
 def is_transient_download_error(e: Exception) -> bool:
     """Return True when a download failure is worth retrying.
 
-    Connection-level failures and HTTP 429/5xx are transient. Hard name
-    resolution failures (NXDOMAIN, offline hosts; not EAI_AGAIN), other
-    HTTP errors, local errors, and exhausted-attempts EsphomeError
-    wrappers (their per-mirror retries are already spent) are permanent.
+    Connection-level failures and HTTP 429/5xx are transient; hard DNS
+    failures, other HTTP errors, and local errors are permanent.
     """
     # Imported lazily: requests is a heavy import (~85ms) and is only
     # needed when actually downloading, never during config validation.
@@ -99,9 +85,9 @@ def is_transient_download_error(e: Exception) -> bool:
 
 
 def fetch_with_retry[T](url: str, fetch: Callable[[], T], what: str = "Download") -> T:
-    """Run ``fetch``, retrying failures that ``is_transient_download_error``
-    classifies as transient with 2s/4s backoff. Other failures and the
-    final attempt's failure propagate to the caller's fallback handling.
+    """Run ``fetch``, retrying transient failures with 2s/4s backoff.
+
+    Permanent failures and the final attempt propagate to the caller;
     ``what`` names the operation in the retry warning.
     """
     import requests

tests/unit_tests/test_external_files.py

@@ -83,10 +83,8 @@ def mock_download_content_many() -> MagicMock:
 
 @pytest.fixture
 def mock_retry_sleep() -> MagicMock:
-    """Patch the retry backoff sleep (module-global time.sleep, so the
-    blast radius is the whole process) for tests that inject transient
-    network errors; without it they would really wait 2s/4s per retry.
-    Retry tests assert on this mock's call args.
+    """Patch the retry backoff sleep (process-wide; net_retry.time is the
+    global module) so transient-error tests don't really wait 2s/4s.
     """
     with patch("esphome.net_retry.time.sleep") as m:
         yield m

tests/unit_tests/test_net_retry.py

@@ -39,10 +39,8 @@ class TestIsTransientDownloadError:
         assert not is_transient_download_error(req.HTTPError("boom"))
 
     def test_hard_dns_failures_are_permanent(self) -> None:
-        """Hard name resolution failures won't heal within a retry window;
-        offline builds must fall back to their cache without sleeping first.
-        requests can surface the gaierror via the cause chain or via
-        urllib3's MaxRetryError.reason attribute."""
+        """Hard resolution failures are permanent via both the cause chain
+        and MaxRetryError.reason."""
         from urllib3.exceptions import MaxRetryError, NameResolutionError
 
         gai = socket.gaierror(socket.EAI_NONAME, "nodename nor servname provided")
@@ -51,9 +49,8 @@ class TestIsTransientDownloadError:
         chained.__cause__ = gai
         assert not is_transient_download_error(chained)
 
-        # The real urllib3 shape, raised the way urllib3 raises it: the
-        # gaierror is NameResolutionError.__cause__ (set at the raise
-        # site), which MaxRetryError carries in its reason attribute.
+        # The real urllib3 shape: gaierror on NameResolutionError.__cause__,
+        # carried by MaxRetryError.reason.
         try:
             raise NameResolutionError("example.invalid", None, gai) from gai
         except NameResolutionError as nre:
@@ -66,8 +63,7 @@ class TestIsTransientDownloadError:
         assert is_transient_download_error(req.ConnectionError("reset by peer"))
 
     def test_temporary_dns_failure_stays_transient(self) -> None:
-        """EAI_AGAIN is a temporary resolver failure (flaky container DNS)
-        and does heal, matching git.py's policy of retrying DNS flakes."""
+        """EAI_AGAIN (flaky resolver) stays retryable."""
         gai = socket.gaierror(socket.EAI_AGAIN, "temporary failure in name resolution")
         chained = req.ConnectionError("resolution failed")
         chained.__cause__ = gai
@@ -75,9 +71,8 @@ class TestIsTransientDownloadError:
         assert is_transient_download_error(chained)
 
     def test_implicit_context_does_not_reclassify(self) -> None:
-        """A resolution failure from an unrelated earlier attempt reaches the
-        final error only via implicit __context__ (raise inside an except
-        block); it must not turn a genuine connection reset permanent."""
+        """A gaierror riding along as implicit __context__ must not turn a
+        genuine connection reset permanent."""
         try:
             try:
                 raise socket.gaierror(socket.EAI_NONAME, "first attempt")
@@ -133,8 +128,7 @@ class TestFetchWithRetry:
     def test_logs_the_upcoming_attempt_number(
         self, caplog: pytest.LogCaptureFixture
     ) -> None:
-        """The warning names the attempt about to run, not the one that just
-        failed, so a user sees attempt 2/3 and 3/3 before the final failure."""
+        """The warning names the attempt about to run, not the failed one."""
         with (
             patch("esphome.net_retry.time.sleep") as mock_sleep,
             pytest.raises(req.ConnectionError),