From d1b9ebc7b2a3aa582643d06548f286cd1886f049 Mon Sep 17 00:00:00 2001 From: Joakim Plate Date: Thu, 8 Aug 2019 00:35:50 +0200 Subject: [PATCH] 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 | 7 +-- homeassistant/auth/providers/__init__.py | 7 +-- homeassistant/config.py | 49 +++++------------ homeassistant/helpers/check_config.py | 62 +++++++--------------- homeassistant/requirements.py | 47 ++++++++++++---- homeassistant/setup.py | 8 +-- tests/helpers/test_check_config.py | 6 +-- tests/scripts/test_check_config.py | 7 +-- tests/test_requirements.py | 43 ++++++++++++--- 9 files changed, 115 insertions(+), 121 deletions(-) diff --git a/homeassistant/auth/mfa_modules/__init__.py b/homeassistant/auth/mfa_modules/__init__.py index fa9b1f50224..5481b8fe08b 100644 --- a/homeassistant/auth/mfa_modules/__init__.py +++ b/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 diff --git a/homeassistant/auth/providers/__init__.py b/homeassistant/auth/providers/__init__.py index c720cf0df64..c35af2e0b96 100644 --- a/homeassistant/auth/providers/__init__.py +++ b/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 diff --git a/homeassistant/config.py b/homeassistant/config.py index 3ba59fd80ec..4d3d4dd841f 100644 --- a/homeassistant/config.py +++ b/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.requirements import async_process_requirements +from homeassistant.loader import Integration, IntegrationNotFound +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) - except IntegrationNotFound: - _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 + integration = await async_get_integration_with_requirements( + hass, domain ) - ): - _log_pkg_error( - pack_name, comp_name, config, "unable to install all requirements" - ) - continue - - try: component = integration.get_component() - except ImportError: - _log_pkg_error(pack_name, comp_name, config, "unable to import") + except (IntegrationNotFound, RequirementsNotFound, ImportError) as ex: + _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) - except IntegrationNotFound: - continue - - 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 - ) + p_integration = await async_get_integration_with_requirements(hass, p_name) + except (RequirementsNotFound, IntegrationNotFound) as ex: + _LOGGER.error("Platform error: %s - %s", domain, ex) 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 diff --git a/homeassistant/helpers/check_config.py b/homeassistant/helpers/check_config.py index a2b4a8580f9..331599e9b0f 100644 --- a/homeassistant/helpers/check_config.py +++ b/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) - except loader.IntegrationNotFound: - result.add_error("Integration not found: {}".format(domain)) - 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) - ) - ) + integration = await async_get_integration_with_requirements(hass, domain) + except (RequirementsNotFound, loader.IntegrationNotFound) as ex: + result.add_error("Component error: {} - {}".format(domain, ex)) continue try: component = integration.get_component() - except ImportError: - result.add_error("Component not found: {}".format(domain)) + except ImportError as ex: + 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) - except loader.IntegrationNotFound: - result.add_error( - "Integration {} not found when trying to verify its {} " - "platform.".format(p_name, domain) + p_integration = await async_get_integration_with_requirements( + hass, p_name ) - 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: - result.add_error("Platform not found: {}.{}".format(domain, p_name)) + except ( + loader.IntegrationNotFound, + RequirementsNotFound, + ImportError, + ) as ex: + result.add_error("Platform error {}.{} - {}".format(domain, p_name, ex)) continue # Validate platform specific schema diff --git a/homeassistant/requirements.py b/homeassistant/requirements.py index 09cdee2c35a..bdc7798e4f8 100644 --- a/homeassistant/requirements.py +++ b/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( - "Not initializing %s because could not install " "requirement %s", - name, - req, - ) - return False - - return True + raise RequirementsNotFound(name, [req]) def _install(hass: HomeAssistant, req: str, kwargs: Dict) -> bool: diff --git a/homeassistant/setup.py b/homeassistant/setup.py index dd29bf3ab09..78bcb2e6505 100644 --- a/homeassistant/setup.py +++ b/homeassistant/setup.py @@ -283,14 +283,10 @@ async def async_process_deps_reqs( ): raise HomeAssistantError("Could not set up all dependencies.") - if ( - not hass.config.skip_pip - and integration.requirements - and not await requirements.async_process_requirements( + if not hass.config.skip_pip and integration.requirements: + await requirements.async_process_requirements( hass, integration.domain, integration.requirements ) - ): - raise HomeAssistantError("Could not install all requirements.") processed.add(integration.domain) diff --git a/tests/helpers/test_check_config.py b/tests/helpers/test_check_config.py index 2a60b9ee2a4..9e5ea15293a 100644 --- a/tests/helpers/test_check_config.py +++ b/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.", - None, - None, + "Platform error light.beer - Integration beer not found.", None, None ) # Only 1 error expected diff --git a/tests/scripts/test_check_config.py b/tests/scripts/test_check_config.py index a07b812bc96..bd4f37bd135 100644 --- a/tests/scripts/test_check_config.py +++ b/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 " - "light platform." + "Platform error light.beer - Integration beer not found." ] } assert res["secret_cache"] == {} diff --git a/tests/test_requirements.py b/tests/test_requirements.py index 486374e3909..b5574fe96fd 100644 --- a/tests/test_requirements.py +++ b/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( - hass, "test_component", ["hello==1.0.0"] - ) + await async_process_requirements(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( - hass, "test_component", ["hello==1.0.0"] - ) + await async_process_requirements(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