From d17f27b65cf4f09f589a82461900b1cd8b4a95e2 Mon Sep 17 00:00:00 2001 From: Pascal Vizeli Date: Tue, 4 Jun 2019 20:04:20 +0200 Subject: [PATCH] Create progress file for pip installs (#24297) * Create progress file for pip installs * fix dedlock * unflacky test * Address comments * Lint * Types --- homeassistant/requirements.py | 20 ++++++++++++++++---- tests/test_requirements.py | 22 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/homeassistant/requirements.py b/homeassistant/requirements.py index 1164eff4eb8..2ab4fe28bdc 100644 --- a/homeassistant/requirements.py +++ b/homeassistant/requirements.py @@ -1,6 +1,6 @@ """Module to handle installing requirements.""" import asyncio -from functools import partial +from pathlib import Path import logging import os from typing import Any, Dict, List, Optional @@ -11,6 +11,7 @@ from homeassistant.core import HomeAssistant DATA_PIP_LOCK = 'pip_lock' DATA_PKG_CACHE = 'pkg_cache' CONSTRAINT_FILE = 'package_constraints.txt' +PROGRESS_FILE = '.pip_progress' _LOGGER = logging.getLogger(__name__) @@ -24,15 +25,16 @@ async def async_process_requirements(hass: HomeAssistant, name: str, if pip_lock is None: pip_lock = hass.data[DATA_PIP_LOCK] = asyncio.Lock() - pip_install = partial(pkg_util.install_package, - **pip_kwargs(hass.config.config_dir)) + kwargs = pip_kwargs(hass.config.config_dir) async with pip_lock: for req in requirements: if pkg_util.is_installed(req): continue - ret = await hass.async_add_executor_job(pip_install, req) + ret = await hass.async_add_executor_job( + _install, hass, req, kwargs + ) if not ret: _LOGGER.error("Not initializing %s because could not install " @@ -42,6 +44,16 @@ async def async_process_requirements(hass: HomeAssistant, name: str, return True +def _install(hass: HomeAssistant, req: str, kwargs: Dict) -> bool: + """Install requirement.""" + progress_path = Path(hass.config.path(PROGRESS_FILE)) + progress_path.touch() + try: + return pkg_util.install_package(req, **kwargs) + finally: + progress_path.unlink() + + def pip_kwargs(config_dir: Optional[str]) -> Dict[str, Any]: """Return keyword arguments for PIP install.""" is_docker = pkg_util.is_docker_env() diff --git a/tests/test_requirements.py b/tests/test_requirements.py index bbf86278bd2..fc9dee20ed2 100644 --- a/tests/test_requirements.py +++ b/tests/test_requirements.py @@ -1,10 +1,11 @@ """Test requirements module.""" import os +from pathlib import Path from unittest.mock import patch, call from homeassistant import setup from homeassistant.requirements import ( - CONSTRAINT_FILE, async_process_requirements) + CONSTRAINT_FILE, async_process_requirements, PROGRESS_FILE, _install) from tests.common import ( get_test_home_assistant, MockModule, mock_coro, mock_integration) @@ -143,3 +144,22 @@ async def test_install_on_docker(hass): constraints=os.path.join('ha_package_path', CONSTRAINT_FILE), no_cache_dir=True, ) + + +async def test_progress_lock(hass): + """Test an install attempt on an existing package.""" + progress_path = Path(hass.config.path(PROGRESS_FILE)) + kwargs = {'hello': 'world'} + + def assert_env(req, **passed_kwargs): + """Assert the env.""" + assert progress_path.exists() + assert req == 'hello' + assert passed_kwargs == kwargs + return True + + with patch('homeassistant.util.package.install_package', + side_effect=assert_env): + _install(hass, 'hello', kwargs) + + assert not progress_path.exists()