diff --git a/homeassistant/components/analytics/manifest.json b/homeassistant/components/analytics/manifest.json index 955c4a813f4..6ab6898ec27 100644 --- a/homeassistant/components/analytics/manifest.json +++ b/homeassistant/components/analytics/manifest.json @@ -5,6 +5,7 @@ "codeowners": ["@home-assistant/core", "@ludeeus"], "dependencies": ["api", "websocket_api"], "documentation": "https://www.home-assistant.io/integrations/analytics", + "import_executor": true, "integration_type": "system", "iot_class": "cloud_push", "quality_scale": "internal" diff --git a/homeassistant/components/apple_tv/manifest.json b/homeassistant/components/apple_tv/manifest.json index 1f7ac45372e..0a14e11ecb7 100644 --- a/homeassistant/components/apple_tv/manifest.json +++ b/homeassistant/components/apple_tv/manifest.json @@ -5,6 +5,7 @@ "config_flow": true, "dependencies": ["zeroconf"], "documentation": "https://www.home-assistant.io/integrations/apple_tv", + "import_executor": true, "iot_class": "local_push", "loggers": ["pyatv", "srptools"], "requirements": ["pyatv==0.14.3"], diff --git a/homeassistant/components/cloud/manifest.json b/homeassistant/components/cloud/manifest.json index e816516fd7a..ef2d32fcb0c 100644 --- a/homeassistant/components/cloud/manifest.json +++ b/homeassistant/components/cloud/manifest.json @@ -5,6 +5,7 @@ "codeowners": ["@home-assistant/cloud"], "dependencies": ["http", "webhook"], "documentation": "https://www.home-assistant.io/integrations/cloud", + "import_executor": true, "integration_type": "system", "iot_class": "cloud_push", "loggers": ["hass_nabucasa"], diff --git a/homeassistant/components/denonavr/manifest.json b/homeassistant/components/denonavr/manifest.json index 9188009bde5..d595c7616ba 100644 --- a/homeassistant/components/denonavr/manifest.json +++ b/homeassistant/components/denonavr/manifest.json @@ -4,6 +4,7 @@ "codeowners": ["@ol-iver", "@starkillerOG"], "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/denonavr", + "import_executor": true, "iot_class": "local_push", "loggers": ["denonavr"], "requirements": ["denonavr==0.11.6"], diff --git a/homeassistant/components/esphome/manifest.json b/homeassistant/components/esphome/manifest.json index ae38497e563..78c9f37fcbf 100644 --- a/homeassistant/components/esphome/manifest.json +++ b/homeassistant/components/esphome/manifest.json @@ -11,6 +11,7 @@ } ], "documentation": "https://www.home-assistant.io/integrations/esphome", + "import_executor": true, "integration_type": "device", "iot_class": "local_push", "loggers": ["aioesphomeapi", "noiseprotocol", "bleak_esphome"], diff --git a/homeassistant/components/hardware/manifest.json b/homeassistant/components/hardware/manifest.json index f2772e609db..8056a4cca4f 100644 --- a/homeassistant/components/hardware/manifest.json +++ b/homeassistant/components/hardware/manifest.json @@ -4,6 +4,7 @@ "codeowners": ["@home-assistant/core"], "config_flow": false, "documentation": "https://www.home-assistant.io/integrations/hardware", + "import_executor": true, "integration_type": "system", "quality_scale": "internal", "requirements": ["psutil-home-assistant==0.0.1"] diff --git a/homeassistant/components/homekit_controller/manifest.json b/homeassistant/components/homekit_controller/manifest.json index 1617b907a26..8460a719419 100644 --- a/homeassistant/components/homekit_controller/manifest.json +++ b/homeassistant/components/homekit_controller/manifest.json @@ -12,6 +12,7 @@ "config_flow": true, "dependencies": ["bluetooth_adapters", "zeroconf"], "documentation": "https://www.home-assistant.io/integrations/homekit_controller", + "import_executor": true, "iot_class": "local_push", "loggers": ["aiohomekit", "commentjson"], "requirements": ["aiohomekit==3.1.4"], diff --git a/homeassistant/components/matter/manifest.json b/homeassistant/components/matter/manifest.json index 801704c25c5..f8ce353f894 100644 --- a/homeassistant/components/matter/manifest.json +++ b/homeassistant/components/matter/manifest.json @@ -5,6 +5,7 @@ "config_flow": true, "dependencies": ["websocket_api"], "documentation": "https://www.home-assistant.io/integrations/matter", + "import_executor": true, "iot_class": "local_push", "requirements": ["python-matter-server==5.5.0"] } diff --git a/homeassistant/components/roomba/manifest.json b/homeassistant/components/roomba/manifest.json index ae08d8f6a1f..530ba8e8137 100644 --- a/homeassistant/components/roomba/manifest.json +++ b/homeassistant/components/roomba/manifest.json @@ -22,6 +22,7 @@ } ], "documentation": "https://www.home-assistant.io/integrations/roomba", + "import_executor": true, "iot_class": "local_push", "loggers": ["paho_mqtt", "roombapy"], "requirements": ["roombapy==1.6.13"], diff --git a/homeassistant/components/sonos/manifest.json b/homeassistant/components/sonos/manifest.json index 58a0ec3b7ee..929b6639e9f 100644 --- a/homeassistant/components/sonos/manifest.json +++ b/homeassistant/components/sonos/manifest.json @@ -6,6 +6,7 @@ "config_flow": true, "dependencies": ["ssdp"], "documentation": "https://www.home-assistant.io/integrations/sonos", + "import_executor": true, "iot_class": "local_push", "loggers": ["soco"], "requirements": ["soco==0.30.2", "sonos-websocket==0.1.3"], diff --git a/homeassistant/components/synology_dsm/manifest.json b/homeassistant/components/synology_dsm/manifest.json index 8060bce5c9b..2820c99f889 100644 --- a/homeassistant/components/synology_dsm/manifest.json +++ b/homeassistant/components/synology_dsm/manifest.json @@ -5,6 +5,7 @@ "config_flow": true, "dependencies": ["http"], "documentation": "https://www.home-assistant.io/integrations/synology_dsm", + "import_executor": true, "iot_class": "local_polling", "loggers": ["synology_dsm"], "requirements": ["py-synologydsm-api==2.1.4"], diff --git a/homeassistant/components/unifi/manifest.json b/homeassistant/components/unifi/manifest.json index f3092811227..a7d787dd86a 100644 --- a/homeassistant/components/unifi/manifest.json +++ b/homeassistant/components/unifi/manifest.json @@ -4,6 +4,7 @@ "codeowners": ["@Kane610"], "config_flow": true, "documentation": "https://www.home-assistant.io/integrations/unifi", + "import_executor": true, "integration_type": "hub", "iot_class": "local_push", "loggers": ["aiounifi"], diff --git a/homeassistant/components/unifiprotect/manifest.json b/homeassistant/components/unifiprotect/manifest.json index 726b487884f..9c0c6c5767a 100644 --- a/homeassistant/components/unifiprotect/manifest.json +++ b/homeassistant/components/unifiprotect/manifest.json @@ -37,6 +37,7 @@ } ], "documentation": "https://www.home-assistant.io/integrations/unifiprotect", + "import_executor": true, "integration_type": "hub", "iot_class": "local_push", "loggers": ["pyunifiprotect", "unifi_discovery"], diff --git a/homeassistant/components/zha/manifest.json b/homeassistant/components/zha/manifest.json index bfaf52940a9..216947515a1 100644 --- a/homeassistant/components/zha/manifest.json +++ b/homeassistant/components/zha/manifest.json @@ -6,6 +6,7 @@ "config_flow": true, "dependencies": ["file_upload"], "documentation": "https://www.home-assistant.io/integrations/zha", + "import_executor": true, "iot_class": "local_polling", "loggers": [ "aiosqlite", diff --git a/homeassistant/config_entries.py b/homeassistant/config_entries.py index b9fe9ebbb5d..4aafa9ff043 100644 --- a/homeassistant/config_entries.py +++ b/homeassistant/config_entries.py @@ -476,7 +476,7 @@ class ConfigEntry: if domain_is_integration: try: - integration.get_platform("config_flow") + await integration.async_get_platform("config_flow") except ImportError as err: _LOGGER.error( ( @@ -2427,9 +2427,8 @@ async def _load_integration( # Make sure requirements and dependencies of component are resolved await async_process_deps_reqs(hass, hass_config, integration) - try: - integration.get_platform("config_flow") + await integration.async_get_platform("config_flow") except ImportError as err: _LOGGER.error( "Error occurred loading flow for integration %s: %s", diff --git a/homeassistant/loader.py b/homeassistant/loader.py index 9c3cf9cd484..ab264f52c2e 100644 --- a/homeassistant/loader.py +++ b/homeassistant/loader.py @@ -14,6 +14,7 @@ import importlib import logging import pathlib import sys +import time from types import ModuleType from typing import TYPE_CHECKING, Any, Literal, Protocol, TypedDict, TypeVar, cast @@ -179,6 +180,7 @@ class Manifest(TypedDict, total=False): version: str codeowners: list[str] loggers: list[str] + import_executor: bool single_config_entry: bool @@ -658,6 +660,7 @@ class Integration: self._all_dependencies_resolved = True self._all_dependencies = set() + self._import_futures: dict[str, asyncio.Future[ModuleType]] = {} _LOGGER.info("Loaded %s from %s", self.domain, pkg_path) @cached_property @@ -727,6 +730,11 @@ class Integration: """Return the integration type.""" return self.manifest.get("integration_type", "hub") + @cached_property + def import_executor(self) -> bool: + """Import integration in the executor.""" + return self.manifest.get("import_executor") or False + @property def mqtt(self) -> list[str] | None: """Return Integration MQTT entries.""" @@ -826,8 +834,47 @@ class Integration: return self._all_dependencies_resolved + async def async_get_component(self) -> ComponentProtocol: + """Return the component. + + This method will load the component if it't not already loaded + and will check if import_executor is set and load it in the executor, + otherwise it will load it in the event loop. + """ + if debug := _LOGGER.isEnabledFor(logging.DEBUG): + start = time.perf_counter() + domain = self.domain + load_executor = ( + self.import_executor + and f"hass.components.{domain}" not in sys.modules + and f"custom_components.{domain}" not in sys.modules + ) + # Some integrations fail on import because they call functions incorrectly. + # So we do it before validating config to catch these errors. + if load_executor: + comp = await self.hass.async_add_executor_job(self.get_component) + else: + comp = self.get_component() + + if debug: + _LOGGER.debug( + "Component %s import took %.3f seconds (loaded_executor=%s)", + domain, + time.perf_counter() - start, + load_executor, + ) + return comp + def get_component(self) -> ComponentProtocol: - """Return the component.""" + """Return the component. + + This method must be thread-safe as its called from the executor + and the event loop. + + This is mostly a thin wrapper around importlib.import_module + with a dict cache which is thread-safe since importlib has + appropriate locks. + """ cache: dict[str, ComponentProtocol] = self.hass.data[DATA_COMPONENTS] if self.domain in cache: return cache[self.domain] @@ -846,10 +893,56 @@ class Integration: return cache[self.domain] - def get_platform(self, platform_name: str) -> ModuleType: + async def async_get_platform(self, platform_name: str) -> ModuleType: """Return a platform for an integration.""" - cache: dict[str, ModuleType] = self.hass.data[DATA_COMPONENTS] + domain = self.domain full_name = f"{self.domain}.{platform_name}" + if platform := self._get_platform_cached(full_name): + return platform + if future := self._import_futures.get(full_name): + return await future + if debug := _LOGGER.isEnabledFor(logging.DEBUG): + start = time.perf_counter() + import_future = self.hass.loop.create_future() + self._import_futures[full_name] = import_future + load_executor = ( + self.import_executor + and domain not in self.hass.config.components + and f"hass.components.{domain}" not in sys.modules + and f"custom_components.{domain}" not in sys.modules + ) + try: + if load_executor: + platform = await self.hass.async_add_executor_job( + self._load_platform, platform_name + ) + else: + platform = self._load_platform(platform_name) + import_future.set_result(platform) + except BaseException as ex: + import_future.set_exception(ex) + with suppress(BaseException): + # Clear the exception retrieved flag on the future since + # it will never be retrieved unless there + # are concurrent calls to async_get_platform + import_future.result() + raise + finally: + self._import_futures.pop(full_name) + + if debug: + _LOGGER.debug( + "Loaded flow for %s in %.2fs (loaded_executor=%s)", + domain, + time.perf_counter() - start, + load_executor, + ) + + return platform + + def _get_platform_cached(self, full_name: str) -> ModuleType | None: + """Return a platform for an integration from cache.""" + cache: dict[str, ModuleType] = self.hass.data[DATA_COMPONENTS] if full_name in cache: return cache[full_name] @@ -859,12 +952,35 @@ class Integration: if full_name in missing_platforms_cache: raise missing_platforms_cache[full_name] + return None + + def get_platform(self, platform_name: str) -> ModuleType: + """Return a platform for an integration.""" + if platform := self._get_platform_cached(f"{self.domain}.{platform_name}"): + return platform + return self._load_platform(platform_name) + + def _load_platform(self, platform_name: str) -> ModuleType: + """Load a platform for an integration. + + This method must be thread-safe as its called from the executor + and the event loop. + + This is mostly a thin wrapper around importlib.import_module + with a dict cache which is thread-safe since importlib has + appropriate locks. + """ + full_name = f"{self.domain}.{platform_name}" + cache: dict[str, ModuleType] = self.hass.data[DATA_COMPONENTS] try: cache[full_name] = self._import_platform(platform_name) except ImportError as ex: if self.domain in cache: # If the domain is loaded, cache that the platform # does not exist so we do not try to load it again + missing_platforms_cache: dict[str, ImportError] = self.hass.data[ + DATA_MISSING_PLATFORMS + ] missing_platforms_cache[full_name] = ex raise except Exception as err: @@ -880,7 +996,11 @@ class Integration: return cache[full_name] def _import_platform(self, platform_name: str) -> ModuleType: - """Import the platform.""" + """Import the platform. + + This method must be thread-safe as its called from the executor + and the event loop. + """ return importlib.import_module(f"{self.pkg_path}.{platform_name}") def __repr__(self) -> str: diff --git a/homeassistant/setup.py b/homeassistant/setup.py index ed54a76dd3d..0e3c78285e2 100644 --- a/homeassistant/setup.py +++ b/homeassistant/setup.py @@ -292,7 +292,7 @@ async def _async_setup_component( # noqa: C901 # Some integrations fail on import because they call functions incorrectly. # So we do it before validating config to catch these errors. try: - component = integration.get_component() + component = await integration.async_get_component() except ImportError as err: log_error(f"Unable to import component: {err}", err) return False diff --git a/script/hassfest/manifest.py b/script/hassfest/manifest.py index 7fb878ca28d..6339d8293ec 100644 --- a/script/hassfest/manifest.py +++ b/script/hassfest/manifest.py @@ -265,6 +265,7 @@ INTEGRATION_MANIFEST_SCHEMA = vol.Schema( vol.Optional("loggers"): [str], vol.Optional("disabled"): str, vol.Optional("iot_class"): vol.In(SUPPORTED_IOT_CLASSES), + vol.Optional("import_executor"): bool, vol.Optional("single_config_entry"): bool, } ) diff --git a/tests/test_loader.py b/tests/test_loader.py index 3745b85b54c..27fe3b94cf2 100644 --- a/tests/test_loader.py +++ b/tests/test_loader.py @@ -1,4 +1,5 @@ """Test to verify that we can load components.""" +import asyncio from unittest.mock import patch import pytest @@ -173,6 +174,20 @@ async def test_get_integration(hass: HomeAssistant) -> None: assert hue_light == integration.get_platform("light") +async def test_async_get_component(hass: HomeAssistant) -> None: + """Test resolving integration.""" + with pytest.raises(loader.IntegrationNotLoaded): + loader.async_get_loaded_integration(hass, "hue") + + integration = await loader.async_get_integration(hass, "hue") + assert await integration.async_get_component() == hue + assert integration.get_platform("light") == hue_light + + integration = loader.async_get_loaded_integration(hass, "hue") + assert await integration.async_get_component() == hue + assert integration.get_platform("light") == hue_light + + async def test_get_integration_exceptions(hass: HomeAssistant) -> None: """Test resolving integration.""" integration = await loader.async_get_integration(hass, "hue") @@ -223,6 +238,41 @@ async def test_get_platform_caches_failures_when_component_loaded( assert integration.get_platform("light") == hue_light +async def test_async_get_platform_caches_failures_when_component_loaded( + hass: HomeAssistant, +) -> None: + """Test async_get_platform cache failures only when the component is loaded.""" + integration = await loader.async_get_integration(hass, "hue") + + with pytest.raises(ImportError), patch( + "homeassistant.loader.importlib.import_module", side_effect=ImportError("Boom") + ): + assert integration.get_component() == hue + + with pytest.raises(ImportError), patch( + "homeassistant.loader.importlib.import_module", side_effect=ImportError("Boom") + ): + assert await integration.async_get_platform("light") == hue_light + + # Hue is not loaded so we should still hit the import_module path + with pytest.raises(ImportError), patch( + "homeassistant.loader.importlib.import_module", side_effect=ImportError("Boom") + ): + assert await integration.async_get_platform("light") == hue_light + + assert integration.get_component() == hue + + # Hue is loaded so we should cache the import_module failure now + with pytest.raises(ImportError), patch( + "homeassistant.loader.importlib.import_module", side_effect=ImportError("Boom") + ): + assert await integration.async_get_platform("light") == hue_light + + # Hue is loaded and the last call should have cached the import_module failure + with pytest.raises(ImportError): + assert await integration.async_get_platform("light") == hue_light + + async def test_get_integration_legacy( hass: HomeAssistant, enable_custom_integrations: None ) -> None: @@ -368,7 +418,9 @@ async def test_integrations_only_once(hass: HomeAssistant) -> None: assert await int_1 is await int_2 -def _get_test_integration(hass, name, config_flow): +def _get_test_integration( + hass: HomeAssistant, name: str, config_flow: bool, import_executor: bool = False +) -> loader.Integration: """Return a generated test integration.""" return loader.Integration( hass, @@ -384,6 +436,7 @@ def _get_test_integration(hass, name, config_flow): "homekit": {"models": [name]}, "ssdp": [{"manufacturer": name, "modelName": name}], "mqtt": [f"{name}/discovery"], + "import_executor": import_executor, }, ) @@ -725,6 +778,35 @@ async def test_get_mqtt(hass: HomeAssistant) -> None: assert mqtt["test_2"] == ["test_2/discovery"] +async def test_import_platform_executor( + hass: HomeAssistant, enable_custom_integrations: None +) -> None: + """Test import a platform in the executor.""" + integration = await loader.async_get_integration( + hass, "test_package_loaded_executor" + ) + + config_flow_task_1 = asyncio.create_task( + integration.async_get_platform("config_flow") + ) + config_flow_task_2 = asyncio.create_task( + integration.async_get_platform("config_flow") + ) + config_flow_task_3 = asyncio.create_task( + integration.async_get_platform("config_flow") + ) + + config_flow_task1_result = await config_flow_task_1 + config_flow_task2_result = await config_flow_task_2 + config_flow_task3_result = await config_flow_task_3 + + assert ( + config_flow_task1_result == config_flow_task2_result == config_flow_task3_result + ) + + assert await config_flow_task1_result._async_has_devices(hass) is True + + async def test_get_custom_components_recovery_mode(hass: HomeAssistant) -> None: """Test that we get empty custom components in recovery mode.""" hass.config.recovery_mode = True diff --git a/tests/test_setup.py b/tests/test_setup.py index cb0d4e7084b..0f6a4302200 100644 --- a/tests/test_setup.py +++ b/tests/test_setup.py @@ -813,3 +813,12 @@ async def test_loading_component_loads_translations(hass: HomeAssistant) -> None assert await setup.async_setup_component(hass, "comp", {}) assert mock_setup.called assert translation.async_translations_loaded(hass, {"comp"}) is True + + +async def test_importing_integration_in_executor( + hass: HomeAssistant, enable_custom_integrations: None +) -> None: + """Test we can import an integration in an executor.""" + assert await setup.async_setup_component(hass, "test_package_loaded_executor", {}) + assert await setup.async_setup_component(hass, "test_package_loaded_executor", {}) + await hass.async_block_till_done() diff --git a/tests/testing_config/custom_components/test_package_loaded_executor/__init__.py b/tests/testing_config/custom_components/test_package_loaded_executor/__init__.py new file mode 100644 index 00000000000..44f62380c92 --- /dev/null +++ b/tests/testing_config/custom_components/test_package_loaded_executor/__init__.py @@ -0,0 +1,9 @@ +"""Provide a mock package component.""" +from .const import TEST # noqa: F401 + +DOMAIN = "test_package" + + +async def async_setup(hass, config): + """Mock a successful setup.""" + return True diff --git a/tests/testing_config/custom_components/test_package_loaded_executor/config_flow.py b/tests/testing_config/custom_components/test_package_loaded_executor/config_flow.py new file mode 100644 index 00000000000..9153b666828 --- /dev/null +++ b/tests/testing_config/custom_components/test_package_loaded_executor/config_flow.py @@ -0,0 +1,7 @@ +"""Config flow.""" + +from homeassistant.core import HomeAssistant + + +async def _async_has_devices(hass: HomeAssistant) -> bool: + return True diff --git a/tests/testing_config/custom_components/test_package_loaded_executor/const.py b/tests/testing_config/custom_components/test_package_loaded_executor/const.py new file mode 100644 index 00000000000..7e13e04cb47 --- /dev/null +++ b/tests/testing_config/custom_components/test_package_loaded_executor/const.py @@ -0,0 +1,2 @@ +"""Constants for test_package custom component.""" +TEST = 5 diff --git a/tests/testing_config/custom_components/test_package_loaded_executor/icons.json b/tests/testing_config/custom_components/test_package_loaded_executor/icons.json new file mode 100644 index 00000000000..e82168d7a1a --- /dev/null +++ b/tests/testing_config/custom_components/test_package_loaded_executor/icons.json @@ -0,0 +1,15 @@ +{ + "entity": { + "switch": { + "something": { + "state": { + "away": "mdi:home-outline", + "home": "mdi:home" + } + } + } + }, + "services": { + "enable_god_mode": "mdi:shield" + } +} diff --git a/tests/testing_config/custom_components/test_package_loaded_executor/manifest.json b/tests/testing_config/custom_components/test_package_loaded_executor/manifest.json new file mode 100644 index 00000000000..9d0fde6b549 --- /dev/null +++ b/tests/testing_config/custom_components/test_package_loaded_executor/manifest.json @@ -0,0 +1,11 @@ +{ + "domain": "test_package_loaded_executor", + "name": "Test Package that loads in the executor", + "documentation": "http://test-package.io", + "requirements": [], + "dependencies": [], + "codeowners": [], + "config_flow": true, + "import_executor": true, + "version": "1.2.3" +}