From f774538e663dbe5b4f1a58b41ff1a5ed36ee7191 Mon Sep 17 00:00:00 2001 From: andrey-git Date: Tue, 7 Feb 2017 11:19:08 +0200 Subject: [PATCH] Check config before restarting (#5609) * Check config before restarting. * Make check_config on restart async * don't check if notification service exists * Use .communicate() * Reduce the number of notifications. Add tests. --- homeassistant/core.py | 33 +++++++++++++++++++++-- homeassistant/scripts/check_config.py | 2 +- tests/common.py | 5 ++++ tests/test_core.py | 38 +++++++++++++++++++++++++-- 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/homeassistant/core.py b/homeassistant/core.py index e2831a63d75..2cf2e3c85e4 100644 --- a/homeassistant/core.py +++ b/homeassistant/core.py @@ -330,11 +330,40 @@ class HomeAssistant(object): self.exit_code = 0 self.loop.create_task(self.async_stop()) + @asyncio.coroutine + def _async_check_config_and_restart(self): + """Restart Home Assistant if config is valid. + + This method is a coroutine. + """ + proc = yield from asyncio.create_subprocess_exec( + sys.argv[0], + '--script', + 'check_config', + stdout=asyncio.subprocess.PIPE) + # Wait for the subprocess exit + (stdout_data, dummy) = yield from proc.communicate() + result = yield from proc.wait() + if result: + _LOGGER.error("check_config failed. Not restarting.") + content = re.sub(r'\033\[[^m]*m', '', str(stdout_data, 'utf-8')) + # Put error cleaned from color codes in the error log so it + # will be visible at the UI. + _LOGGER.error(content) + yield from self.services.async_call( + 'persistent_notification', 'create', { + 'message': 'Config error. See dev-info panel for details.', + 'title': 'Restarting', + 'notification_id': '{}.restart'.format(DOMAIN)}) + return + + self.exit_code = RESTART_EXIT_CODE + yield from self.async_stop() + @callback def _async_restart_handler(self, *args): """Restart Home Assistant.""" - self.exit_code = RESTART_EXIT_CODE - self.loop.create_task(self.async_stop()) + self.loop.create_task(self._async_check_config_and_restart()) class EventOrigin(enum.Enum): diff --git a/homeassistant/scripts/check_config.py b/homeassistant/scripts/check_config.py index cb825ad44c8..b1ecaaa57ba 100644 --- a/homeassistant/scripts/check_config.py +++ b/homeassistant/scripts/check_config.py @@ -96,7 +96,6 @@ def run(script_args: List) -> int: domain_info = args.info.split(',') res = check(config_path) - if args.files: print(color(C_HEAD, 'yaml files'), '(used /', color('red', 'not used') + ')') @@ -247,6 +246,7 @@ def check(config_path): res['secret_cache'] = dict(yaml.__SECRET_CACHE) except Exception as err: # pylint: disable=broad-except print(color('red', 'Fatal error while loading config:'), str(err)) + res['except'].setdefault(ERROR_STR, []).append(err) finally: # Stop all patches for pat in PATCHES.values(): diff --git a/tests/common.py b/tests/common.py index b602edbd717..98a3102edf7 100644 --- a/tests/common.py +++ b/tests/common.py @@ -388,6 +388,11 @@ def mock_coro(return_value=None): return coro +def mock_generator(return_value=None): + """Helper method to return a coro generator that returns a value.""" + return mock_coro(return_value)() + + @contextmanager def assert_setup_component(count, domain=None): """Collect valid configuration from setup_component. diff --git a/tests/test_core.py b/tests/test_core.py index 14276584ae2..72c459a4eac 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -14,9 +14,10 @@ from homeassistant.util.async import run_coroutine_threadsafe import homeassistant.util.dt as dt_util from homeassistant.util.unit_system import (METRIC_SYSTEM) from homeassistant.const import ( - __version__, EVENT_STATE_CHANGED, ATTR_FRIENDLY_NAME, CONF_UNIT_SYSTEM) + __version__, EVENT_STATE_CHANGED, ATTR_FRIENDLY_NAME, CONF_UNIT_SYSTEM, + SERVICE_HOMEASSISTANT_RESTART, RESTART_EXIT_CODE) -from tests.common import get_test_home_assistant +from tests.common import get_test_home_assistant, mock_generator PST = pytz.timezone('America/Los_Angeles') @@ -220,6 +221,39 @@ class TestHomeAssistant(unittest.TestCase): with pytest.raises(ValueError): self.hass.add_job(None, 'test_arg') + @patch('asyncio.create_subprocess_exec') + def test_restart(self, mock_create): + """Check that restart propagates to stop.""" + process_mock = MagicMock() + attrs = { + 'communicate.return_value': mock_generator(('output', 'error')), + 'wait.return_value': mock_generator(0)} + process_mock.configure_mock(**attrs) + mock_create.return_value = mock_generator(process_mock) + + self.hass.start() + with patch.object(self.hass, 'async_stop') as mock_stop: + self.hass.services.call(ha.DOMAIN, SERVICE_HOMEASSISTANT_RESTART) + mock_stop.assert_called_once_with() + self.assertEqual(RESTART_EXIT_CODE, self.hass.exit_code) + + @patch('asyncio.create_subprocess_exec') + def test_restart_bad_config(self, mock_create): + """Check that restart with a bad config doesn't propagate to stop.""" + process_mock = MagicMock() + attrs = { + 'communicate.return_value': + mock_generator((r'\033[hellom'.encode('utf-8'), 'error')), + 'wait.return_value': mock_generator(1)} + process_mock.configure_mock(**attrs) + mock_create.return_value = mock_generator(process_mock) + + self.hass.start() + with patch.object(self.hass, 'async_stop') as mock_stop: + self.hass.services.call(ha.DOMAIN, SERVICE_HOMEASSISTANT_RESTART) + mock_stop.assert_not_called() + self.assertEqual(None, self.hass.exit_code) + class TestEvent(unittest.TestCase): """A Test Event class."""