Migrate remaining calls in config modules to async_get_component (#112293)

* Migrate remaining calls in config modules to async_get_component

There were a few cases that were still using get_component that
could have done blocking I/O in the event loop, although it
was unlikely.

The caching check in async_get_component has been moved
up to avoid creating the future if the module is already in
the cache

* fix one more
This commit is contained in:
J. Nick Koston 2024-03-05 04:59:52 -10:00 committed by GitHub
parent 390f5822fe
commit 7cb8a8bbc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 30 additions and 20 deletions

View File

@ -1083,7 +1083,7 @@ async def merge_packages_config(
integration = await async_get_integration_with_requirements( integration = await async_get_integration_with_requirements(
hass, domain hass, domain
) )
component = integration.get_component() component = await integration.async_get_component()
except LOAD_EXCEPTIONS as exc: except LOAD_EXCEPTIONS as exc:
_log_pkg_error( _log_pkg_error(
hass, hass,

View File

@ -490,7 +490,7 @@ class ConfigEntry:
hass, self.domain hass, self.domain
) )
try: try:
component = integration.get_component() component = await integration.async_get_component()
except ImportError as err: except ImportError as err:
_LOGGER.error( _LOGGER.error(
"Error importing integration %s to set up %s configuration entry: %s", "Error importing integration %s to set up %s configuration entry: %s",
@ -677,7 +677,7 @@ class ConfigEntry:
self._async_set_state(hass, ConfigEntryState.NOT_LOADED, None) self._async_set_state(hass, ConfigEntryState.NOT_LOADED, None)
return True return True
component = integration.get_component() component = await integration.async_get_component()
if integration.domain == self.domain: if integration.domain == self.domain:
if not self.state.recoverable: if not self.state.recoverable:
@ -734,7 +734,7 @@ class ConfigEntry:
# entry. # entry.
return return
component = integration.get_component() component = await integration.async_get_component()
if not hasattr(component, "async_remove_entry"): if not hasattr(component, "async_remove_entry"):
return return
try: try:
@ -783,7 +783,7 @@ class ConfigEntry:
if not (integration := self._integration_for_domain): if not (integration := self._integration_for_domain):
integration = await loader.async_get_integration(hass, self.domain) integration = await loader.async_get_integration(hass, self.domain)
component = integration.get_component() component = await integration.async_get_component()
supports_migrate = hasattr(component, "async_migrate_entry") supports_migrate = hasattr(component, "async_migrate_entry")
if not supports_migrate: if not supports_migrate:
if same_major_version: if same_major_version:
@ -2497,14 +2497,14 @@ def _handle_entry_updated_filter(event: Event) -> bool:
async def support_entry_unload(hass: HomeAssistant, domain: str) -> bool: async def support_entry_unload(hass: HomeAssistant, domain: str) -> bool:
"""Test if a domain supports entry unloading.""" """Test if a domain supports entry unloading."""
integration = await loader.async_get_integration(hass, domain) integration = await loader.async_get_integration(hass, domain)
component = integration.get_component() component = await integration.async_get_component()
return hasattr(component, "async_unload_entry") return hasattr(component, "async_unload_entry")
async def support_remove_from_device(hass: HomeAssistant, domain: str) -> bool: async def support_remove_from_device(hass: HomeAssistant, domain: str) -> bool:
"""Test if a domain supports being removed from a device.""" """Test if a domain supports being removed from a device."""
integration = await loader.async_get_integration(hass, domain) integration = await loader.async_get_integration(hass, domain)
component = integration.get_component() component = await integration.async_get_component()
return hasattr(component, "async_remove_config_entry_device") return hasattr(component, "async_remove_config_entry_device")

View File

@ -901,6 +901,10 @@ class Integration:
and will check if import_executor is set and load it in the executor, and will check if import_executor is set and load it in the executor,
otherwise it will load it in the event loop. otherwise it will load it in the event loop.
""" """
domain = self.domain
if domain in (cache := self._cache):
return cache[domain]
if self._component_future: if self._component_future:
return await self._component_future return await self._component_future
@ -914,7 +918,7 @@ class Integration:
or (self.config_flow and f"{self.pkg_path}.config_flow" not in sys.modules) or (self.config_flow and f"{self.pkg_path}.config_flow" not in sys.modules)
) )
if not load_executor: if not load_executor:
comp = self.get_component() comp = self._get_component()
if debug: if debug:
_LOGGER.debug( _LOGGER.debug(
"Component %s import took %.3f seconds (loaded_executor=False)", "Component %s import took %.3f seconds (loaded_executor=False)",
@ -927,7 +931,7 @@ class Integration:
try: try:
try: try:
comp = await self.hass.async_add_import_executor_job( comp = await self.hass.async_add_import_executor_job(
self.get_component, True self._get_component, True
) )
except ImportError as ex: except ImportError as ex:
load_executor = False load_executor = False
@ -936,7 +940,7 @@ class Integration:
) )
# If importing in the executor deadlocks because there is a circular # If importing in the executor deadlocks because there is a circular
# dependency, we fall back to the event loop. # dependency, we fall back to the event loop.
comp = self.get_component() comp = self._get_component()
self._component_future.set_result(comp) self._component_future.set_result(comp)
except BaseException as ex: except BaseException as ex:
self._component_future.set_exception(ex) self._component_future.set_exception(ex)
@ -959,22 +963,29 @@ class Integration:
return comp return comp
def get_component(self, preload_platforms: bool = False) -> ComponentProtocol: def get_component(self) -> ComponentProtocol:
"""Return the component. """Return the component.
This method must be thread-safe as it's called from the executor This method must be thread-safe as it's called from the executor
and the event loop. and the event loop.
This method checks the cache and if the component is not loaded
it will load it in the executor if import_executor is set, otherwise
it will load it in the event loop.
This is mostly a thin wrapper around importlib.import_module This is mostly a thin wrapper around importlib.import_module
with a dict cache which is thread-safe since importlib has with a dict cache which is thread-safe since importlib has
appropriate locks. appropriate locks.
""" """
domain = self.domain
if domain in (cache := self._cache):
return cache[domain]
return self._get_component()
def _get_component(self, preload_platforms: bool = False) -> ComponentProtocol:
"""Return the component."""
cache = self._cache cache = self._cache
domain = self.domain domain = self.domain
if domain in cache:
return cache[domain]
try: try:
cache[domain] = cast( cache[domain] = cast(
ComponentProtocol, importlib.import_module(self.pkg_path) ComponentProtocol, importlib.import_module(self.pkg_path)

View File

@ -1188,10 +1188,9 @@ async def test_async_get_component_loads_loop_if_already_in_sys_modules(
), patch("homeassistant.loader.importlib.import_module", import_module): ), patch("homeassistant.loader.importlib.import_module", import_module):
module = await integration.async_get_component() module = await integration.async_get_component()
# Everything is there so we should load in the event loop # Everything is already in the integration cache
# since it will all be cached # so it should not have to call the load
assert "loaded_executor=False" in caplog.text assert "loaded_executor" not in caplog.text
assert "loaded_executor=True" not in caplog.text
assert module is module_mock assert module is module_mock

View File

@ -617,7 +617,7 @@ async def test_async_when_setup_or_start_already_loaded(hass: HomeAssistant) ->
async def test_setup_import_blows_up(hass: HomeAssistant) -> None: async def test_setup_import_blows_up(hass: HomeAssistant) -> None:
"""Test that we handle it correctly when importing integration blows up.""" """Test that we handle it correctly when importing integration blows up."""
with patch( with patch(
"homeassistant.loader.Integration.get_component", side_effect=ImportError "homeassistant.loader.Integration.async_get_component", side_effect=ImportError
): ):
assert not await setup.async_setup_component(hass, "sun", {}) assert not await setup.async_setup_component(hass, "sun", {})