From c8fd00e8c45a8784bfa1afd2396ae3879d8b3958 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 7 Oct 2026 13:27:39 -1000 Subject: [PATCH] Simplify the include walk, secret collector, package sources and raw client force_load_include_files creates its result once and the recursive walker returns nothing, the never nested secret collector is a single set, package sources are an ordered dict returned as a tuple, and the raw api client shares one receive step. --- esphome/components/packages/__init__.py | 16 ++--- esphome/yaml_util.py | 65 ++++++++++--------- .../component_tests/packages/test_packages.py | 4 +- tests/integration/raw_api_client.py | 18 +++-- 4 files changed, 52 insertions(+), 51 deletions(-) diff --git a/esphome/components/packages/__init__.py b/esphome/components/packages/__init__.py index d568ce3512b..8d3ce463627 100644 --- a/esphome/components/packages/__init__.py +++ b/esphome/components/packages/__init__.py @@ -54,7 +54,8 @@ class RemotePackageSource: class PackagesData: """Per-run package state, keyed under DOMAIN in CORE.data.""" - remote_sources: list[RemotePackageSource] = field(default_factory=list) + # Insertion-ordered and de-duplicated + remote_sources: dict[RemotePackageSource, None] = field(default_factory=dict) def _get_data() -> PackagesData: @@ -63,15 +64,15 @@ def _get_data() -> PackagesData: return CORE.data[DOMAIN] -def get_remote_package_sources() -> list[RemotePackageSource]: +def get_remote_package_sources() -> tuple[RemotePackageSource, ...]: """Remote sources fetched while processing this config, in fetch order. Consumers (e.g. store_yaml) use this to tell which parts of the config came from remote repositories rather than local files. """ if (data := CORE.data.get(DOMAIN)) is None: - return [] - return list(data.remote_sources) + return () + return tuple(data.remote_sources) # Guard against infinite include chains (e.g. A includes B includes A). @@ -227,10 +228,9 @@ def _process_remote_package(config: dict[str, Any]) -> dict[str, Any]: username=config.get(CONF_USERNAME), password=config.get(CONF_PASSWORD), ) - source = RemotePackageSource(config[CONF_URL], config.get(CONF_REF)) - remote_sources = _get_data().remote_sources - if source not in remote_sources: - remote_sources.append(source) + _get_data().remote_sources[ + RemotePackageSource(config[CONF_URL], config.get(CONF_REF)) + ] = None files: list[dict[str, Any]] = [] # ``repo_root`` is the directory containing ``.git`` and must be passed diff --git a/esphome/yaml_util.py b/esphome/yaml_util.py index 42523924e76..719c6cd9cce 100644 --- a/esphome/yaml_util.py +++ b/esphome/yaml_util.py @@ -47,10 +47,9 @@ _LOGGER = logging.getLogger(__name__) SECRET_YAML = "secrets.yaml" _SECRET_CACHE = {} _SECRET_VALUES = {} -# Stack of collectors (one per active secret_values_registered context); the -# dumper records every emitted `!secret` name into the innermost one. YAML -# processing is single-threaded. -_EMITTED_SECRET_NAMES: list[set[str]] = [] +# Set while a secret_values_registered context is active; the dumper records +# every emitted `!secret` name into it. +_EMITTED_SECRET_NAMES: set[str] | None = None # Not thread-safe — config processing is single-threaded today. _load_listeners: list[Callable[[Path], None]] = [] @@ -439,7 +438,7 @@ def _load_include_candidates( # The throwaway IncludeFile is this tree's only owner; keep the tree # alive so ids recorded in ``seen`` stay unique for the traversal. keepalive.append(loaded) - force_load_include_files( + _force_load_include_files( loaded, warn_on_unresolved=warn_on_unresolved, _seen=seen, @@ -476,13 +475,7 @@ class ForceLoadResult(NamedTuple): def force_load_include_files( - obj: Any, - *, - warn_on_unresolved: bool = True, - _seen: set[int] | None = None, - _expanded_paths: set[Path] | None = None, - _keepalive: list[Any] | None = None, - _result: ForceLoadResult | None = None, + obj: Any, *, warn_on_unresolved: bool = True ) -> ForceLoadResult: """Recursively resolve any deferred ``IncludeFile`` instances in a YAML tree. @@ -500,6 +493,23 @@ def force_load_include_files( run on a fresh re-parse where substitutions haven't been applied yet) to demote it to a debug log. """ + result = ForceLoadResult([], []) + _force_load_include_files( + obj, warn_on_unresolved=warn_on_unresolved, _result=result + ) + return result + + +def _force_load_include_files( + obj: Any, + *, + warn_on_unresolved: bool = True, + _seen: set[int] | None = None, + _expanded_paths: set[Path] | None = None, + _keepalive: list[Any] | None = None, + _result: ForceLoadResult, +) -> None: + """Walk for :func:`force_load_include_files`, adding to ``result``.""" from voluptuous import Invalid if _seen is None: @@ -513,12 +523,10 @@ def force_load_include_files( # fresh tree look already seen. Discovery is a one-shot operation, # so holding the parsed trees costs nothing. _keepalive = [] - if _result is None: - _result = ForceLoadResult([], []) if isinstance(obj, IncludeFile): if id(obj) in _seen: - return _result + return _seen.add(id(obj)) if obj.has_unresolved_file(): _load_include_candidates( @@ -529,7 +537,7 @@ def force_load_include_files( keepalive=_keepalive, result=_result, ) - return _result + return try: loaded = obj.load() except (EsphomeError, Invalid) as err: @@ -540,8 +548,8 @@ def force_load_include_files( err, ) _result.errors.append(f"{obj.file}: {err}") - return _result - force_load_include_files( + return + _force_load_include_files( loaded, warn_on_unresolved=warn_on_unresolved, _seen=_seen, @@ -551,10 +559,10 @@ def force_load_include_files( ) elif isinstance(obj, dict): if id(obj) in _seen: - return _result + return _seen.add(id(obj)) for value in obj.values(): - force_load_include_files( + _force_load_include_files( value, warn_on_unresolved=warn_on_unresolved, _seen=_seen, @@ -564,10 +572,10 @@ def force_load_include_files( ) elif isinstance(obj, (list, tuple)): if id(obj) in _seen: - return _result + return _seen.add(id(obj)) for item in obj: - force_load_include_files( + _force_load_include_files( item, warn_on_unresolved=warn_on_unresolved, _seen=_seen, @@ -575,7 +583,6 @@ def force_load_include_files( _keepalive=_keepalive, _result=_result, ) - return _result @dataclass(slots=True) @@ -1193,17 +1200,15 @@ def secret_values_registered(values: dict[str, str]) -> Generator[set[str]]: registered dumps as a bare ``!secret``, dropping its tag rather than the value leaking. """ + global _EMITTED_SECRET_NAMES # noqa: PLW0603 added = {v: n for v, n in values.items() if v not in _SECRET_VALUES} _SECRET_VALUES.update(added) emitted: set[str] = set() - _EMITTED_SECRET_NAMES.append(emitted) + _EMITTED_SECRET_NAMES = emitted try: yield emitted finally: - # Contexts unwind LIFO, so the innermost collector is always last; - # pop() removes by position where remove() would match the first - # *equal* set and could strip an outer context's collector. - _EMITTED_SECRET_NAMES.pop() + _EMITTED_SECRET_NAMES = None # Only drop mappings this context still owns; a real !secret loaded # meanwhile may have registered the same value under its own name. for value, name in added.items(): @@ -1426,8 +1431,8 @@ class ESPHomeDumper(yaml.SafeDumper): def represent_secret(self, value): name = _SECRET_VALUES[str(value)] - if _EMITTED_SECRET_NAMES: - _EMITTED_SECRET_NAMES[-1].add(name) + if _EMITTED_SECRET_NAMES is not None: + _EMITTED_SECRET_NAMES.add(name) return self.represent_scalar(tag="!secret", value=name) def represent_stringify(self, value): diff --git a/tests/component_tests/packages/test_packages.py b/tests/component_tests/packages/test_packages.py index ca54f9d7c18..0107314947a 100644 --- a/tests/component_tests/packages/test_packages.py +++ b/tests/component_tests/packages/test_packages.py @@ -618,7 +618,7 @@ def test_package_remove_by_missing_id() -> None: def test_get_remote_package_sources_empty_without_remote_packages() -> None: """A config without remote packages records no sources.""" - assert get_remote_package_sources() == [] + assert get_remote_package_sources() == () @patch("esphome.yaml_util.load_yaml") @@ -652,8 +652,6 @@ def test_get_remote_package_sources_records_each_fetch_once( } packages_pass(config) - sources = get_remote_package_sources() - sources.clear() sources = get_remote_package_sources() assert sorted(sources, key=lambda source: source.url) == [ RemotePackageSource("https://github.com/esphome/repo-a", "main"), diff --git a/tests/integration/raw_api_client.py b/tests/integration/raw_api_client.py index e0162268f00..46be7cd12d8 100644 --- a/tests/integration/raw_api_client.py +++ b/tests/integration/raw_api_client.py @@ -166,22 +166,23 @@ class RawApiClient: loop = asyncio.get_running_loop() await loop.sock_sendall(self._sock, encode_frame(msg_type, payload)) + async def _recv_once(self) -> None: + data = await asyncio.get_running_loop().sock_recv(self._sock, _READ_CHUNK) + assert data, "server closed the connection unexpectedly" + self.bytes_received += len(data) + self.frame_counts.update(self._parser.feed(data)) + async def read_until_frame(self, msg_type: int, timeout: float = 10.0) -> None: """Read until at least one frame of msg_type has been received.""" - loop = asyncio.get_running_loop() async def _read_loop() -> None: while not self.frame_counts[msg_type]: - data = await loop.sock_recv(self._sock, _READ_CHUNK) - assert data, "server closed the connection unexpectedly" - self.bytes_received += len(data) - self.frame_counts.update(self._parser.feed(data)) + await self._recv_once() await asyncio.wait_for(_read_loop(), timeout) async def read_frame(self, msg_type: int, timeout: float = 10.0) -> bytes: """Return the payload of the next frame of msg_type, dropping other frames before it.""" - loop = asyncio.get_running_loop() frames = self._parser.frames async def _read_loop() -> bytes: @@ -190,10 +191,7 @@ class RawApiClient: frame_type, payload = frames.popleft() if frame_type == msg_type: return payload - data = await loop.sock_recv(self._sock, _READ_CHUNK) - assert data, "server closed the connection unexpectedly" - self.bytes_received += len(data) - self.frame_counts.update(self._parser.feed(data)) + await self._recv_once() return await asyncio.wait_for(_read_loop(), timeout)