From a90d840c26f38b6916974cf61c7b407b814be06d Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Sun, 26 Apr 2026 10:11:52 -0500 Subject: [PATCH] Address Copilot follow-up on PR #16021 - has_remote_file_changed now accepts a `timeout` argument and passes it to requests.head; download_content forwards its own timeout in. Without this, callers couldn't actually control the end-to-end timeout -- the GET respected it but the conditional HEAD didn't. - Replace the (path, url) -> (url, path) lambda inside ex.map with a named local helper that unpacks the tuple. Easier to read; the awkward (item[1], item[0]) indexing was a maintenance hazard. --- esphome/external_files.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/esphome/external_files.py b/esphome/external_files.py index 132fe76a9e6..80644e3fec0 100644 --- a/esphome/external_files.py +++ b/esphome/external_files.py @@ -25,7 +25,9 @@ CONTENT_DISPOSITION = "content-disposition" TEMP_DIR = "temp" -def has_remote_file_changed(url: str, local_file_path: Path) -> bool: +def has_remote_file_changed( + url: str, local_file_path: Path, timeout: int = NETWORK_TIMEOUT +) -> bool: if local_file_path.exists(): _LOGGER.debug("has_remote_file_changed: File exists at %s", local_file_path) try: @@ -39,7 +41,7 @@ def has_remote_file_changed(url: str, local_file_path: Path) -> bool: CACHE_CONTROL: CACHE_CONTROL_MAX_AGE + "3600", } response = requests.head( - url, headers=headers, timeout=NETWORK_TIMEOUT, allow_redirects=True + url, headers=headers, timeout=timeout, allow_redirects=True ) _LOGGER.debug( @@ -88,7 +90,7 @@ def download_content(url: str, path: Path, timeout: int = NETWORK_TIMEOUT) -> by if CORE.skip_external_update and path.exists(): _LOGGER.debug("Skipping update for %s (refresh disabled)", url) return path.read_bytes() - if not has_remote_file_changed(url, path): + if not has_remote_file_changed(url, path, timeout): _LOGGER.debug("Remote file has not changed %s", url) return path.read_bytes() @@ -156,15 +158,17 @@ def download_content_many( path, url = next(iter(seen.items())) download_content(url, path, timeout) return + + def _download_one(path_url: tuple[Path, str]) -> None: + # `seen` stores entries as (path, url) so the dict can dedupe by + # path; flip them back to download_content's (url, path) order. + path, url = path_url + download_content(url, path, timeout) + workers = max(1, min(max_workers, len(seen))) with ThreadPoolExecutor(max_workers=workers) as ex: # list() forces iteration so exceptions surface here, not silently. - list( - ex.map( - lambda item: download_content(item[1], item[0], timeout), - seen.items(), - ) - ) + list(ex.map(_download_one, seen.items())) # Each component that uses external_files defines its own local