From c32f47aea657b887c12560a92d691c6fd7f6a311 Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Sun, 23 Oct 2016 19:17:34 -0700 Subject: [PATCH 1/5] iOS component hotfixes (#4015) * iOS component hot fixes around component/platform loading, logging, and more * Load device_tracker and zeroconf in deps instead of bootstraping * Change conditional check on status code --- homeassistant/components/ios.py | 35 ++++---------------- homeassistant/components/notify/ios.py | 45 ++++++++++++++++---------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/homeassistant/components/ios.py b/homeassistant/components/ios.py index 0793417fab3..e8545210182 100644 --- a/homeassistant/components/ios.py +++ b/homeassistant/components/ios.py @@ -13,8 +13,6 @@ from voluptuous.humanize import humanize_error from homeassistant.helpers import config_validation as cv -import homeassistant.loader as loader - from homeassistant.helpers import discovery from homeassistant.components.http import HomeAssistantView @@ -22,13 +20,11 @@ from homeassistant.components.http import HomeAssistantView from homeassistant.const import (HTTP_INTERNAL_SERVER_ERROR, HTTP_BAD_REQUEST) -from homeassistant.components.notify import DOMAIN as NotifyDomain - _LOGGER = logging.getLogger(__name__) DOMAIN = "ios" -DEPENDENCIES = ["http"] +DEPENDENCIES = ["device_tracker", "http", "zeroconf"] CONF_PUSH = "push" CONF_PUSH_CATEGORIES = "categories" @@ -245,34 +241,17 @@ def setup(hass, config): if CONFIG_FILE == {}: CONFIG_FILE[ATTR_DEVICES] = {} - device_tracker = loader.get_component("device_tracker") - if device_tracker.DOMAIN not in hass.config.components: - device_tracker.setup(hass, {}) - # Need this to enable requirements checking in the app. - hass.config.components.append(device_tracker.DOMAIN) - - if "notify.ios" not in hass.config.components: - notify = loader.get_component("notify.ios") - notify.get_service(hass, {}) - # Need this to enable requirements checking in the app. - if NotifyDomain not in hass.config.components: - hass.config.components.append(NotifyDomain) - - zeroconf = loader.get_component("zeroconf") - if zeroconf.DOMAIN not in hass.config.components: - zeroconf.setup(hass, config) - # Need this to enable requirements checking in the app. - hass.config.components.append(zeroconf.DOMAIN) + # Notify needs to have discovery + # notify_config = {"notify": {CONF_PLATFORM: "ios"}} + # bootstrap.setup_component(hass, "notify", notify_config) discovery.load_platform(hass, "sensor", DOMAIN, {}, config) hass.wsgi.register_view(iOSIdentifyDeviceView(hass)) - if config.get(DOMAIN) is not None: - app_config = config[DOMAIN] - if app_config.get(CONF_PUSH) is not None: - push_config = app_config[CONF_PUSH] - hass.wsgi.register_view(iOSPushConfigView(hass, push_config)) + app_config = config.get(DOMAIN, {}) + hass.wsgi.register_view(iOSPushConfigView(hass, + app_config.get(CONF_PUSH, {}))) return True diff --git a/homeassistant/components/notify/ios.py b/homeassistant/components/notify/ios.py index cb85ab8f753..2517020434e 100644 --- a/homeassistant/components/notify/ios.py +++ b/homeassistant/components/notify/ios.py @@ -23,6 +23,22 @@ PUSH_URL = "https://ios-push.home-assistant.io/push" DEPENDENCIES = ["ios"] +# pylint: disable=invalid-name +def log_rate_limits(target, resp, level=20): + """Output rate limit log line at given level.""" + rate_limits = resp["rateLimits"] + resetsAt = dt_util.parse_datetime(rate_limits["resetsAt"]) + resetsAtTime = resetsAt - datetime.now(timezone.utc) + rate_limit_msg = ("iOS push notification rate limits for %s: " + "%d sent, %d allowed, %d errors, " + "resets in %s") + _LOGGER.log(level, rate_limit_msg, + ios.device_name_for_push_id(target), + rate_limits["successful"], + rate_limits["maximum"], rate_limits["errors"], + str(resetsAtTime).split(".")[0]) + + def get_service(hass, config): """Get the iOS notification service.""" if "notify.ios" not in hass.config.components: @@ -66,22 +82,17 @@ class iOSNotificationService(BaseNotificationService): req = requests.post(PUSH_URL, json=data, timeout=10) - if req.status_code is not 201: - message = req.json()["message"] - if req.status_code is 429: + if req.status_code != 201: + fallback_error = req.json().get("errorMessage", + "Unknown error") + fallback_message = ("Internal server error, " + "please try again later: " + "{}").format(fallback_error) + message = req.json().get("message", fallback_message) + if req.status_code == 429: _LOGGER.warning(message) - elif req.status_code is 400 or 500: + log_rate_limits(target, req.json(), 30) + else: _LOGGER.error(message) - - if req.status_code in (201, 429): - rate_limits = req.json()["rateLimits"] - resetsAt = dt_util.parse_datetime(rate_limits["resetsAt"]) - resetsAtTime = resetsAt - datetime.now(timezone.utc) - rate_limit_msg = ("iOS push notification rate limits for %s: " - "%d sent, %d allowed, %d errors, " - "resets in %s") - _LOGGER.info(rate_limit_msg, - ios.device_name_for_push_id(target), - rate_limits["successful"], - rate_limits["maximum"], rate_limits["errors"], - str(resetsAtTime).split(".")[0]) + else: + log_rate_limits(target, req.json()) From f0a38dded668ef065b0b698eecdf31d3927175c6 Mon Sep 17 00:00:00 2001 From: Johann Kellerman Date: Mon, 24 Oct 2016 03:55:06 +0200 Subject: [PATCH 2/5] Catch UnicodeDecodeError Error (#4007) * Catch UnicodeDecodeError Error Error for #3933 * Forgot (exc) * catch... * Tests by @lwis * Docstring * Create open --- homeassistant/util/yaml.py | 3 +++ tests/util/test_yaml.py | 11 +++++++++++ 2 files changed, 14 insertions(+) diff --git a/homeassistant/util/yaml.py b/homeassistant/util/yaml.py index cf773bb999f..42776241843 100644 --- a/homeassistant/util/yaml.py +++ b/homeassistant/util/yaml.py @@ -43,6 +43,9 @@ def load_yaml(fname: str) -> Union[List, Dict]: except yaml.YAMLError as exc: _LOGGER.error(exc) raise HomeAssistantError(exc) + except UnicodeDecodeError as exc: + _LOGGER.error('Unable to read file %s: %s', fname, exc) + raise HomeAssistantError(exc) def clear_secret_cache() -> None: diff --git a/tests/util/test_yaml.py b/tests/util/test_yaml.py index 7c7bb0b9255..79cae43b845 100644 --- a/tests/util/test_yaml.py +++ b/tests/util/test_yaml.py @@ -92,6 +92,7 @@ class TestYaml(unittest.TestCase): mock_walk.return_value = [ ['/tmp', ['tmp2'], ['zero.yaml']], ['/tmp/tmp2', [], ['one.yaml', 'two.yaml']], + ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ @@ -123,6 +124,7 @@ class TestYaml(unittest.TestCase): mock_walk.return_value = [ ['/tmp', ['tmp2'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], + ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ @@ -155,6 +157,7 @@ class TestYaml(unittest.TestCase): mock_walk.return_value = [ ['/tmp', ['tmp2'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], + ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ @@ -191,6 +194,7 @@ class TestYaml(unittest.TestCase): mock_walk.return_value = [ ['/tmp', ['tmp2'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], + ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ @@ -208,6 +212,13 @@ class TestYaml(unittest.TestCase): "key4": "four" } + @patch('homeassistant.util.yaml.open', create=True) + def test_load_yaml_encoding_error(self, mock_open): + """Test raising a UnicodeDecodeError.""" + mock_open.side_effect = UnicodeDecodeError('', b'', 1, 0, '') + self.assertRaises(HomeAssistantError, yaml.load_yaml, 'test') + + FILES = {} From 3e92318cb2b34bd23430521e6d534932e2944d75 Mon Sep 17 00:00:00 2001 From: Paulus Schoutsen Date: Sun, 23 Oct 2016 19:20:30 -0700 Subject: [PATCH 3/5] Version bump to 0.31.1 --- homeassistant/const.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/const.py b/homeassistant/const.py index efb11cdffbf..d0803fa4351 100644 --- a/homeassistant/const.py +++ b/homeassistant/const.py @@ -2,7 +2,7 @@ """Constants used by Home Assistant components.""" MAJOR_VERSION = 0 MINOR_VERSION = 31 -PATCH_VERSION = '0' +PATCH_VERSION = '1' __short_version__ = '{}.{}'.format(MAJOR_VERSION, MINOR_VERSION) __version__ = '{}.{}'.format(__short_version__, PATCH_VERSION) REQUIRED_PYTHON_VER = (3, 4, 2) From 3230869f74f5db9feacb93e3be225fbc2b0bc051 Mon Sep 17 00:00:00 2001 From: Robbie Trencheny Date: Sun, 23 Oct 2016 20:33:49 -0700 Subject: [PATCH 4/5] Fix a spelling problem on user-facing error --- homeassistant/helpers/condition.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/helpers/condition.py b/homeassistant/helpers/condition.py index bfc637cf946..a6db4f9150d 100644 --- a/homeassistant/helpers/condition.py +++ b/homeassistant/helpers/condition.py @@ -283,7 +283,7 @@ def async_template(hass, value_template, variables=None): try: value = value_template.async_render(variables) except TemplateError as ex: - _LOGGER.error('Error duriong template condition: %s', ex) + _LOGGER.error('Error during template condition: %s', ex) return False return value.lower() == 'true' From 1db18478d2d38a8495f77342112abaf14589f963 Mon Sep 17 00:00:00 2001 From: Lewis Juggins Date: Sun, 23 Oct 2016 15:47:06 +0100 Subject: [PATCH 5/5] Exclude dirs/files prefixed with . (#3986) --- homeassistant/util/yaml.py | 12 ++++++-- tests/util/test_yaml.py | 63 ++++++++++++++++++++++++++------------ 2 files changed, 53 insertions(+), 22 deletions(-) diff --git a/homeassistant/util/yaml.py b/homeassistant/util/yaml.py index 42776241843..3ee47e76cf2 100644 --- a/homeassistant/util/yaml.py +++ b/homeassistant/util/yaml.py @@ -64,11 +64,17 @@ def _include_yaml(loader: SafeLineLoader, return load_yaml(fname) -def _find_files(directory, pattern): +def _is_file_valid(name: str) -> bool: + """Decide if a file is valid.""" + return not name.startswith('.') + + +def _find_files(directory: str, pattern: str): """Recursively load files in a directory.""" - for root, _dirs, files in os.walk(directory): + for root, dirs, files in os.walk(directory, topdown=True): + dirs[:] = [d for d in dirs if _is_file_valid(d)] for basename in files: - if fnmatch.fnmatch(basename, pattern): + if _is_file_valid(basename) and fnmatch.fnmatch(basename, pattern): filename = os.path.join(root, basename) yield filename diff --git a/tests/util/test_yaml.py b/tests/util/test_yaml.py index 79cae43b845..9ead3c858a5 100644 --- a/tests/util/test_yaml.py +++ b/tests/util/test_yaml.py @@ -76,10 +76,13 @@ class TestYaml(unittest.TestCase): @patch('homeassistant.util.yaml.os.walk') def test_include_dir_list(self, mock_walk): """Test include dir list yaml.""" - mock_walk.return_value = [['/tmp', [], ['one.yaml', 'two.yaml']]] + mock_walk.return_value = [ + ['/tmp', [], ['one.yaml', 'two.yaml']], + ] with patch_yaml_files({ - '/tmp/one.yaml': 'one', '/tmp/two.yaml': 'two' + '/tmp/one.yaml': 'one', + '/tmp/two.yaml': 'two', }): conf = "key: !include_dir_list /tmp" with io.StringIO(conf) as file: @@ -90,27 +93,35 @@ class TestYaml(unittest.TestCase): def test_include_dir_list_recursive(self, mock_walk): """Test include dir recursive list yaml.""" mock_walk.return_value = [ - ['/tmp', ['tmp2'], ['zero.yaml']], + ['/tmp', ['tmp2', '.ignore', 'ignore'], ['zero.yaml']], ['/tmp/tmp2', [], ['one.yaml', 'two.yaml']], ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ - '/tmp/zero.yaml': 'zero', '/tmp/tmp2/one.yaml': 'one', - '/tmp/tmp2/two.yaml': 'two' + '/tmp/zero.yaml': 'zero', + '/tmp/tmp2/one.yaml': 'one', + '/tmp/tmp2/two.yaml': 'two' }): conf = "key: !include_dir_list /tmp" with io.StringIO(conf) as file: + assert '.ignore' in mock_walk.return_value[0][1], \ + "Expecting .ignore in here" doc = yaml.yaml.safe_load(file) + assert 'tmp2' in mock_walk.return_value[0][1] + assert '.ignore' not in mock_walk.return_value[0][1] assert sorted(doc["key"]) == sorted(["zero", "one", "two"]) @patch('homeassistant.util.yaml.os.walk') def test_include_dir_named(self, mock_walk): """Test include dir named yaml.""" - mock_walk.return_value = [['/tmp', [], ['first.yaml', 'second.yaml']]] + mock_walk.return_value = [ + ['/tmp', [], ['first.yaml', 'second.yaml']] + ] with patch_yaml_files({ - '/tmp/first.yaml': 'one', '/tmp/second.yaml': 'two' + '/tmp/first.yaml': 'one', + '/tmp/second.yaml': 'two' }): conf = "key: !include_dir_named /tmp" correct = {'first': 'one', 'second': 'two'} @@ -122,19 +133,24 @@ class TestYaml(unittest.TestCase): def test_include_dir_named_recursive(self, mock_walk): """Test include dir named yaml.""" mock_walk.return_value = [ - ['/tmp', ['tmp2'], ['first.yaml']], + ['/tmp', ['tmp2', '.ignore', 'ignore'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ - '/tmp/first.yaml': 'one', '/tmp/tmp2/second.yaml': 'two', - '/tmp/tmp2/third.yaml': 'three' + '/tmp/first.yaml': 'one', + '/tmp/tmp2/second.yaml': 'two', + '/tmp/tmp2/third.yaml': 'three' }): conf = "key: !include_dir_named /tmp" correct = {'first': 'one', 'second': 'two', 'third': 'three'} with io.StringIO(conf) as file: + assert '.ignore' in mock_walk.return_value[0][1], \ + "Expecting .ignore in here" doc = yaml.yaml.safe_load(file) + assert 'tmp2' in mock_walk.return_value[0][1] + assert '.ignore' not in mock_walk.return_value[0][1] assert doc["key"] == correct @patch('homeassistant.util.yaml.os.walk') @@ -143,8 +159,8 @@ class TestYaml(unittest.TestCase): mock_walk.return_value = [['/tmp', [], ['first.yaml', 'second.yaml']]] with patch_yaml_files({ - '/tmp/first.yaml': '- one', - '/tmp/second.yaml': '- two\n- three' + '/tmp/first.yaml': '- one', + '/tmp/second.yaml': '- two\n- three' }): conf = "key: !include_dir_merge_list /tmp" with io.StringIO(conf) as file: @@ -155,18 +171,23 @@ class TestYaml(unittest.TestCase): def test_include_dir_merge_list_recursive(self, mock_walk): """Test include dir merge list yaml.""" mock_walk.return_value = [ - ['/tmp', ['tmp2'], ['first.yaml']], + ['/tmp', ['tmp2', '.ignore', 'ignore'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ - '/tmp/first.yaml': '- one', '/tmp/tmp2/second.yaml': '- two', - '/tmp/tmp2/third.yaml': '- three\n- four' + '/tmp/first.yaml': '- one', + '/tmp/tmp2/second.yaml': '- two', + '/tmp/tmp2/third.yaml': '- three\n- four' }): conf = "key: !include_dir_merge_list /tmp" with io.StringIO(conf) as file: + assert '.ignore' in mock_walk.return_value[0][1], \ + "Expecting .ignore in here" doc = yaml.yaml.safe_load(file) + assert 'tmp2' in mock_walk.return_value[0][1] + assert '.ignore' not in mock_walk.return_value[0][1] assert sorted(doc["key"]) == sorted(["one", "two", "three", "four"]) @@ -192,19 +213,23 @@ class TestYaml(unittest.TestCase): def test_include_dir_merge_named_recursive(self, mock_walk): """Test include dir merge named yaml.""" mock_walk.return_value = [ - ['/tmp', ['tmp2'], ['first.yaml']], + ['/tmp', ['tmp2', '.ignore', 'ignore'], ['first.yaml']], ['/tmp/tmp2', [], ['second.yaml', 'third.yaml']], ['/tmp/ignore', [], ['.ignore.yaml']] ] with patch_yaml_files({ - '/tmp/first.yaml': 'key1: one', - '/tmp/tmp2/second.yaml': 'key2: two', - '/tmp/tmp2/third.yaml': 'key3: three\nkey4: four' + '/tmp/first.yaml': 'key1: one', + '/tmp/tmp2/second.yaml': 'key2: two', + '/tmp/tmp2/third.yaml': 'key3: three\nkey4: four' }): conf = "key: !include_dir_merge_named /tmp" with io.StringIO(conf) as file: + assert '.ignore' in mock_walk.return_value[0][1], \ + "Expecting .ignore in here" doc = yaml.yaml.safe_load(file) + assert 'tmp2' in mock_walk.return_value[0][1] + assert '.ignore' not in mock_walk.return_value[0][1] assert doc["key"] == { "key1": "one", "key2": "two",