Integration requirement check refactor (#25626)

* Factor out code getting requirements for integration

* Have process requirements raise an exception

* One more lint fix

* Blackify

* Catch new exception

* Let RequirementsNotFound be a HomeAssistantError

* Correct another test

* Split catching of exceptions and avoid complete log

homeassistant/auth/mfa_modules/__init__.py

@@ -164,14 +164,9 @@ async def _load_mfa_module(hass: HomeAssistant, module_name: str) -> types.Modul
     processed = hass.data[DATA_REQS] = set()
 
     # https://github.com/python/mypy/issues/1424
-    req_success = await requirements.async_process_requirements(
+    await requirements.async_process_requirements(
         hass, module_path, module.REQUIREMENTS  # type: ignore
     )
 
-    if not req_success:
-        raise HomeAssistantError(
-            "Unable to process requirements of mfa module {}".format(module_name)
-        )
-
     processed.add(module_name)
     return module

homeassistant/auth/providers/__init__.py

@@ -165,15 +165,10 @@ async def load_auth_provider_module(
 
     # https://github.com/python/mypy/issues/1424
     reqs = module.REQUIREMENTS  # type: ignore
-    req_success = await requirements.async_process_requirements(
+    await requirements.async_process_requirements(
         hass, "auth provider {}".format(provider), reqs
     )
 
-    if not req_success:
-        raise HomeAssistantError(
-            "Unable to process requirements of auth provider {}".format(provider)
-        )
-
     processed.add(provider)
     return module
 

homeassistant/config.py

@@ -53,8 +53,11 @@ from homeassistant.const import (
 )
 from homeassistant.core import DOMAIN as CONF_CORE, SOURCE_YAML, HomeAssistant, callback
 from homeassistant.exceptions import HomeAssistantError
-from homeassistant.loader import Integration, async_get_integration, IntegrationNotFound
+from homeassistant.loader import Integration, IntegrationNotFound
-from homeassistant.requirements import async_process_requirements
+from homeassistant.requirements import (
+    async_get_integration_with_requirements,
+    RequirementsNotFound,
+)
 from homeassistant.util.yaml import load_yaml, SECRET_YAML
 from homeassistant.util.package import is_docker_env
 import homeassistant.helpers.config_validation as cv
@@ -658,27 +661,12 @@ async def merge_packages_config(
             domain = comp_name.split(" ")[0]
 
             try:
-                integration = await async_get_integration(hass, domain)
+                integration = await async_get_integration_with_requirements(
-            except IntegrationNotFound:
+                    hass, domain
-                _log_pkg_error(pack_name, comp_name, config, "does not exist")
-                continue
-
-            if (
-                not hass.config.skip_pip
-                and integration.requirements
-                and not await async_process_requirements(
-                    hass, integration.domain, integration.requirements
                 )
-            ):
-                _log_pkg_error(
-                    pack_name, comp_name, config, "unable to install all requirements"
-                )
-                continue
-
-            try:
                 component = integration.get_component()
-            except ImportError:
+            except (IntegrationNotFound, RequirementsNotFound, ImportError) as ex:
-                _log_pkg_error(pack_name, comp_name, config, "unable to import")
+                _log_pkg_error(pack_name, comp_name, config, str(ex))
                 continue
 
             if hasattr(component, "PLATFORM_SCHEMA"):
@@ -775,26 +763,15 @@ async def async_process_component_config(
             continue
 
         try:
-            p_integration = await async_get_integration(hass, p_name)
+            p_integration = await async_get_integration_with_requirements(hass, p_name)
-        except IntegrationNotFound:
+        except (RequirementsNotFound, IntegrationNotFound) as ex:
-            continue
+            _LOGGER.error("Platform error: %s - %s", domain, ex)
-
-        if (
-            not hass.config.skip_pip
-            and p_integration.requirements
-            and not await async_process_requirements(
-                hass, p_integration.domain, p_integration.requirements
-            )
-        ):
-            _LOGGER.error(
-                "Unable to install all requirements for %s.%s", domain, p_name
-            )
             continue
 
         try:
             platform = p_integration.get_platform(domain)
         except ImportError:
-            _LOGGER.exception("Failed to get platform %s.%s", domain, p_name)
+            _LOGGER.exception("Platform error: %s", domain)
             continue
 
         # Validate platform specific schema

homeassistant/helpers/check_config.py

@@ -5,7 +5,7 @@ from typing import List
 import attr
 import voluptuous as vol
 
-from homeassistant import loader, requirements
+from homeassistant import loader
 from homeassistant.core import HomeAssistant
 from homeassistant.config import (
     CONF_CORE,
@@ -18,6 +18,10 @@ from homeassistant.config import (
     extract_domain_configs,
     config_per_platform,
 )
+from homeassistant.requirements import (
+    async_get_integration_with_requirements,
+    RequirementsNotFound,
+)
 
 import homeassistant.util.yaml.loader as yaml_loader
 from homeassistant.exceptions import HomeAssistantError
@@ -101,29 +105,15 @@ async def async_check_ha_config_file(hass: HomeAssistant) -> HomeAssistantConfig
     # Process and validate config
     for domain in components:
         try:
-            integration = await loader.async_get_integration(hass, domain)
+            integration = await async_get_integration_with_requirements(hass, domain)
-        except loader.IntegrationNotFound:
+        except (RequirementsNotFound, loader.IntegrationNotFound) as ex:
-            result.add_error("Integration not found: {}".format(domain))
+            result.add_error("Component error: {} - {}".format(domain, ex))
-            continue
-
-        if (
-            not hass.config.skip_pip
-            and integration.requirements
-            and not await requirements.async_process_requirements(
-                hass, integration.domain, integration.requirements
-            )
-        ):
-            result.add_error(
-                "Unable to install all requirements: {}".format(
-                    ", ".join(integration.requirements)
-                )
-            )
             continue
 
         try:
             component = integration.get_component()
-        except ImportError:
+        except ImportError as ex:
-            result.add_error("Component not found: {}".format(domain))
+            result.add_error("Component error: {} - {}".format(domain, ex))
             continue
 
         config_schema = getattr(component, "CONFIG_SCHEMA", None)
@@ -161,32 +151,16 @@ async def async_check_ha_config_file(hass: HomeAssistant) -> HomeAssistantConfig
                 continue
 
             try:
-                p_integration = await loader.async_get_integration(hass, p_name)
+                p_integration = await async_get_integration_with_requirements(
-            except loader.IntegrationNotFound:
+                    hass, p_name
-                result.add_error(
-                    "Integration {} not found when trying to verify its {} "
-                    "platform.".format(p_name, domain)
                 )
-                continue
-
-            if (
-                not hass.config.skip_pip
-                and p_integration.requirements
-                and not await requirements.async_process_requirements(
-                    hass, p_integration.domain, p_integration.requirements
-                )
-            ):
-                result.add_error(
-                    "Unable to install all requirements: {}".format(
-                        ", ".join(integration.requirements)
-                    )
-                )
-                continue
-
-            try:
                 platform = p_integration.get_platform(domain)
-            except ImportError:
+            except (
-                result.add_error("Platform not found: {}.{}".format(domain, p_name))
+                loader.IntegrationNotFound,
+                RequirementsNotFound,
+                ImportError,
+            ) as ex:
+                result.add_error("Platform error {}.{} - {}".format(domain, p_name, ex))
                 continue
 
             # Validate platform specific schema

homeassistant/requirements.py

@@ -5,8 +5,10 @@ import logging
 import os
 from typing import Any, Dict, List, Optional
 
+from homeassistant.exceptions import HomeAssistantError
 import homeassistant.util.package as pkg_util
 from homeassistant.core import HomeAssistant
+from homeassistant.loader import async_get_integration, Integration
 
 DATA_PIP_LOCK = "pip_lock"
 DATA_PKG_CACHE = "pkg_cache"
@@ -15,12 +17,44 @@ PROGRESS_FILE = ".pip_progress"
 _LOGGER = logging.getLogger(__name__)
 
 
+class RequirementsNotFound(HomeAssistantError):
+    """Raised when a component is not found."""
+
+    def __init__(self, domain: str, requirements: List) -> None:
+        """Initialize a component not found error."""
+        super().__init__(
+            "Requirements for {} not found: {}.".format(domain, requirements)
+        )
+        self.domain = domain
+        self.requirements = requirements
+
+
+async def async_get_integration_with_requirements(
+    hass: HomeAssistant, domain: str
+) -> Integration:
+    """Get an integration with installed requirements.
+
+    This can raise IntegrationNotFound if manifest or integration
+    is invalid, RequirementNotFound if there was some type of
+    failure to install requirements.
+    """
+    integration = await async_get_integration(hass, domain)
+
+    if hass.config.skip_pip or not integration.requirements:
+        return integration
+
+    await async_process_requirements(hass, integration.domain, integration.requirements)
+
+    return integration
+
+
 async def async_process_requirements(
     hass: HomeAssistant, name: str, requirements: List[str]
-) -> bool:
+) -> None:
     """Install the requirements for a component or platform.
 
-    This method is a coroutine.
+    This method is a coroutine. It will raise RequirementsNotFound
+    if an requirement can't be satisfied.
     """
     pip_lock = hass.data.get(DATA_PIP_LOCK)
     if pip_lock is None:
@@ -36,14 +70,7 @@ async def async_process_requirements(
             ret = await hass.async_add_executor_job(_install, hass, req, kwargs)
 
             if not ret:
-                _LOGGER.error(
+                raise RequirementsNotFound(name, [req])
-                    "Not initializing %s because could not install " "requirement %s",
-                    name,
-                    req,
-                )
-                return False
-
-    return True
 
 
 def _install(hass: HomeAssistant, req: str, kwargs: Dict) -> bool:

homeassistant/setup.py

@@ -283,14 +283,10 @@ async def async_process_deps_reqs(
     ):
         raise HomeAssistantError("Could not set up all dependencies.")
 
-    if (
+    if not hass.config.skip_pip and integration.requirements:
-        not hass.config.skip_pip
+        await requirements.async_process_requirements(
-        and integration.requirements
-        and not await requirements.async_process_requirements(
             hass, integration.domain, integration.requirements
         )
-    ):
-        raise HomeAssistantError("Could not install all requirements.")
 
     processed.add(integration.domain)
 

tests/helpers/test_check_config.py

@@ -75,7 +75,7 @@ async def test_component_platform_not_found(hass, loop):
 
         assert res.keys() == {"homeassistant"}
         assert res.errors[0] == CheckConfigError(
-            "Integration not found: beer", None, None
+            "Component error: beer - Integration beer not found.", None, None
         )
 
         # Only 1 error expected
@@ -95,9 +95,7 @@ async def test_component_platform_not_found_2(hass, loop):
         assert res["light"] == []
 
         assert res.errors[0] == CheckConfigError(
-            "Integration beer not found when trying to verify its " "light platform.",
+            "Platform error light.beer - Integration beer not found.", None, None
-            None,
-            None,
         )
 
         # Only 1 error expected

tests/scripts/test_check_config.py

@@ -62,7 +62,9 @@ def test_component_platform_not_found(isfile_patch, loop):
         res = check_config.check(get_test_config_dir())
         assert res["components"].keys() == {"homeassistant"}
         assert res["except"] == {
-            check_config.ERROR_STR: ["Integration not found: beer"]
+            check_config.ERROR_STR: [
+                "Component error: beer - Integration beer not found."
+            ]
         }
         assert res["secret_cache"] == {}
         assert res["secrets"] == {}
@@ -75,8 +77,7 @@ def test_component_platform_not_found(isfile_patch, loop):
         assert res["components"]["light"] == []
         assert res["except"] == {
             check_config.ERROR_STR: [
-                "Integration beer not found when trying to verify its "
+                "Platform error light.beer - Integration beer not found."
-                "light platform."
             ]
         }
         assert res["secret_cache"] == {}

tests/test_requirements.py

@@ -2,13 +2,16 @@
 import os
 from pathlib import Path
 from unittest.mock import patch, call
+from pytest import raises
 
 from homeassistant import setup
 from homeassistant.requirements import (
     CONSTRAINT_FILE,
+    async_get_integration_with_requirements,
     async_process_requirements,
     PROGRESS_FILE,
     _install,
+    RequirementsNotFound,
 )
 
 from tests.common import get_test_home_assistant, MockModule, mock_integration
@@ -74,22 +77,50 @@ async def test_install_existing_package(hass):
     with patch(
         "homeassistant.util.package.install_package", return_value=True
     ) as mock_inst:
-        assert await async_process_requirements(
+        await async_process_requirements(hass, "test_component", ["hello==1.0.0"])
-            hass, "test_component", ["hello==1.0.0"]
-        )
 
     assert len(mock_inst.mock_calls) == 1
 
     with patch("homeassistant.util.package.is_installed", return_value=True), patch(
         "homeassistant.util.package.install_package"
     ) as mock_inst:
-        assert await async_process_requirements(
+        await async_process_requirements(hass, "test_component", ["hello==1.0.0"])
-            hass, "test_component", ["hello==1.0.0"]
-        )
 
     assert len(mock_inst.mock_calls) == 0
 
 
+async def test_install_missing_package(hass):
+    """Test an install attempt on an existing package."""
+    with patch(
+        "homeassistant.util.package.install_package", return_value=False
+    ) as mock_inst:
+        with raises(RequirementsNotFound):
+            await async_process_requirements(hass, "test_component", ["hello==1.0.0"])
+
+    assert len(mock_inst.mock_calls) == 1
+
+
+async def test_get_integration_with_requirements(hass):
+    """Check getting an integration with loaded requirements."""
+    hass.config.skip_pip = False
+    mock_integration(hass, MockModule("test_component", requirements=["hello==1.0.0"]))
+
+    with patch(
+        "homeassistant.util.package.is_installed", return_value=False
+    ) as mock_is_installed, patch(
+        "homeassistant.util.package.install_package", return_value=True
+    ) as mock_inst:
+
+        integration = await async_get_integration_with_requirements(
+            hass, "test_component"
+        )
+        assert integration
+        assert integration.domain == "test_component"
+
+    assert len(mock_is_installed.mock_calls) == 1
+    assert len(mock_inst.mock_calls) == 1
+
+
 async def test_install_with_wheels_index(hass):
     """Test an install attempt with wheels index URL."""
     hass.config.skip_pip = False