Skip to content

Load HA core config from storage #23872

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
balloob merged 7 commits into from
May 16, 2019
Merged

Load HA core config from storage #23872

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
502e41f
Load HA core config from storage
emontnemery May 14, 2019
fb3bbe7
Tweak
emontnemery May 14, 2019
7edaa0c
Lint, review comments
emontnemery May 15, 2019
81b4369
Fix test
emontnemery May 15, 2019
4e6e03a
Add tests
emontnemery May 16, 2019
69afb72
Lint
emontnemery May 16, 2019
7e367d9
Address comments
balloob May 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 68 additions & 14 deletions homeassistant/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@
('ios.conf', '.ios.conf'),
)

CORE_STORAGE_KEY = 'homeassistant.core_config'
CORE_STORAGE_VERSION = 1

SOURCE_DISCOVERED = 'discovered'
SOURCE_STORAGE = 'storage'
SOURCE_YAML = 'yaml'

DEFAULT_CORE_CONFIG = (
# Tuples (attribute, default, auto detect property, description)
(CONF_NAME, 'Home', None, 'Name of the location where Home Assistant is '
Expand Down Expand Up @@ -473,6 +480,40 @@ def _format_config_error(ex: vol.Invalid, domain: str, config: Dict) -> str:
return message


def _set_time_zone(hass: HomeAssistant, time_zone_str: Optional[str]) -> None:
"""Help to set the time zone."""
if time_zone_str is None:
return

time_zone = date_util.get_time_zone(time_zone_str)

if time_zone:
hass.config.time_zone = time_zone
date_util.set_default_time_zone(time_zone)
else:
_LOGGER.error("Received invalid time zone %s", time_zone_str)


async def async_load_ha_core_config(hass: HomeAssistant) -> None:
"""Store [homeassistant] core config."""
store = hass.helpers.storage.Store(CORE_STORAGE_VERSION, CORE_STORAGE_KEY,
private=True)
data = await store.async_load()
if data:
Comment thread
balloob marked this conversation as resolved.
Outdated
hac = hass.config
hac.config_source = SOURCE_STORAGE
hac.latitude = data['latitude']

@OttoWinter OttoWinter May 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will this handle users backrolling? If we decide to change the schema here, some of these keys might be invalid. If a user then tries backporting to a version before the schema change this will raise an exception - and since this is core code it could (I think) stop HA from starting up.

@emontnemery emontnemery May 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, how about validating the data with a schema, and rejecting it if it doesn't pass?

@OttoWinter OttoWinter May 16, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that could probably work

hac.longitude = data['longitude']
hac.elevation = data['elevation']
unit_system = data['unit_system']
if unit_system == CONF_UNIT_SYSTEM_IMPERIAL:
hac.units = IMPERIAL_SYSTEM
else:
hac.units = METRIC_SYSTEM
hac.location_name = data['location_name']
_set_time_zone(hass, data['time_zone'])


async def async_process_ha_core_config(
hass: HomeAssistant, config: Dict,
api_password: Optional[str] = None,
Expand Down Expand Up @@ -511,20 +552,14 @@ async def async_process_ha_core_config(
auth_conf,
mfa_conf))

hac = hass.config
await async_load_ha_core_config(hass)

def set_time_zone(time_zone_str: Optional[str]) -> None:
"""Help to set the time zone."""
if time_zone_str is None:
return

time_zone = date_util.get_time_zone(time_zone_str)
hac = hass.config

if time_zone:
hac.time_zone = time_zone
date_util.set_default_time_zone(time_zone)
else:
_LOGGER.error("Received invalid time zone %s", time_zone_str)
if any([k in config for k in [
CONF_LATITUDE, CONF_LONGITUDE, CONF_NAME, CONF_ELEVATION,
CONF_TIME_ZONE, CONF_UNIT_SYSTEM]]):

@OttoWinter OttoWinter May 15, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a list is unnecessary here (https://twitter.com/raymondh/status/1125487457443532800). Admittedly, this is a micro-optimization though.

@emontnemery emontnemery May 15, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's even a pico-optimization ;)
Do you have a suggestion for a cleaner way to do it though?

@OttoWinter OttoWinter May 16, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? You just have to remove the [] so that the call inside any is a generator, not a list

hac.config_source = SOURCE_YAML

for key, attr in ((CONF_LATITUDE, 'latitude'),
(CONF_LONGITUDE, 'longitude'),
Expand All @@ -533,7 +568,7 @@ def set_time_zone(time_zone_str: Optional[str]) -> None:
if key in config:
setattr(hac, attr, config[key])

set_time_zone(config.get(CONF_TIME_ZONE))
_set_time_zone(hass, config.get(CONF_TIME_ZONE))

# Init whitelist external dir
hac.whitelist_external_dirs = {hass.config.path('www')}
Expand Down Expand Up @@ -591,6 +626,7 @@ def set_time_zone(time_zone_str: Optional[str]) -> None:
# If we miss some of the needed values, auto detect them
if None in (hac.latitude, hac.longitude, hac.units,
Comment thread
emontnemery marked this conversation as resolved.
hac.time_zone):
hac.config_source = SOURCE_DISCOVERED
info = await loc_util.async_detect_location_info(
hass.helpers.aiohttp_client.async_get_clientsession()
)
Expand All @@ -613,7 +649,7 @@ def set_time_zone(time_zone_str: Optional[str]) -> None:
discovered.append(('name', info.city))

if hac.time_zone is None:
set_time_zone(info.time_zone)
_set_time_zone(hass, info.time_zone)
discovered.append(('time_zone', info.time_zone))

if hac.elevation is None and hac.latitude is not None and \
Expand All @@ -630,6 +666,24 @@ def set_time_zone(time_zone_str: Optional[str]) -> None:
", ".join('{}: {}'.format(key, val) for key, val in discovered))


async def async_store_ha_core_config(hass: HomeAssistant) -> None:
Comment thread
emontnemery marked this conversation as resolved.
"""Store [homeassistant] core config."""
config = hass.config.as_dict()

data = {
'latitude': config['latitude'],
'longitude': config['longitude'],
'elevation': config['elevation'],
'unit_system': hass.config.units.name,
'location_name': config['location_name'],
'time_zone': config['time_zone'],
}

store = hass.helpers.storage.Store(CORE_STORAGE_VERSION, CORE_STORAGE_KEY,
private=True)
await store.async_save(data)


def _log_pkg_error(
package: str, component: str, config: Dict, message: str) -> None:
"""Log an error while merging packages."""
Expand Down
5 changes: 4 additions & 1 deletion homeassistant/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1177,6 +1177,8 @@ def __init__(self) -> None:
self.time_zone = None # type: Optional[datetime.tzinfo]
self.units = METRIC_SYSTEM # type: UnitSystem

self.config_source = None # type: Optional[str]

# If True, pip install is skipped for requirements on startup
self.skip_pip = False # type: bool

Expand Down Expand Up @@ -1251,7 +1253,8 @@ def as_dict(self) -> Dict:
'components': self.components,
'config_dir': self.config_dir,
'whitelist_external_dirs': self.whitelist_external_dirs,
'version': __version__
'version': __version__,
'config_source': self.config_source
}


Expand Down
64 changes: 64 additions & 0 deletions tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,66 @@ def _mock_isfile(filename):
assert mock_os.rename.call_count == 0


async def test_loading_configuration_from_storage(hass, hass_storage):
"""Test loading core config onto hass object."""
hass.config = mock.Mock()
hass_storage["homeassistant.core_config"] = {
'data': {
'elevation': 10,
'latitude': 55,
'location_name': 'Home',
'longitude': 13,
'time_zone': 'Europe/Copenhagen',
'unit_system': 'metric'
},
'key': 'homeassistant.core_config',
'version': 1
}
await config_util.async_process_ha_core_config(
hass, {'whitelist_external_dirs': '/tmp'})

assert hass.config.latitude == 55
assert hass.config.longitude == 13
assert hass.config.elevation == 10
assert hass.config.location_name == 'Home'
assert hass.config.units.name == CONF_UNIT_SYSTEM_METRIC
assert hass.config.time_zone.zone == 'Europe/Copenhagen'
assert len(hass.config.whitelist_external_dirs) == 2
assert '/tmp' in hass.config.whitelist_external_dirs
assert hass.config.config_source == config_util.SOURCE_STORAGE


async def test_override_stored_configuration(hass, hass_storage):
"""Test loading core config onto hass object."""
Comment thread
balloob marked this conversation as resolved.
Outdated
hass.config = mock.Mock()
Comment thread
balloob marked this conversation as resolved.
Outdated
hass_storage["homeassistant.core_config"] = {
'data': {
'elevation': 10,
'latitude': 55,
'location_name': 'Home',
'longitude': 13,
'time_zone': 'Europe/Copenhagen',
'unit_system': 'metric'
},
'key': 'homeassistant.core_config',
'version': 1
}
await config_util.async_process_ha_core_config(hass, {
'latitude': 60,
'whitelist_external_dirs': '/tmp',
})

assert hass.config.latitude == 60
assert hass.config.longitude == 13
assert hass.config.elevation == 10
assert hass.config.location_name == 'Home'
assert hass.config.units.name == CONF_UNIT_SYSTEM_METRIC
assert hass.config.time_zone.zone == 'Europe/Copenhagen'
assert len(hass.config.whitelist_external_dirs) == 2
assert '/tmp' in hass.config.whitelist_external_dirs
assert hass.config.config_source == config_util.SOURCE_YAML


async def test_loading_configuration(hass):
"""Test loading core config onto hass object."""
hass.config = mock.Mock()
Expand All @@ -436,6 +496,7 @@ async def test_loading_configuration(hass):
assert hass.config.time_zone.zone == 'America/New_York'
assert len(hass.config.whitelist_external_dirs) == 2
assert '/tmp' in hass.config.whitelist_external_dirs
assert hass.config.config_source == config_util.SOURCE_YAML


async def test_loading_configuration_temperature_unit(hass):
Expand All @@ -457,6 +518,7 @@ async def test_loading_configuration_temperature_unit(hass):
assert hass.config.location_name == 'Huis'
assert hass.config.units.name == CONF_UNIT_SYSTEM_METRIC
assert hass.config.time_zone.zone == 'America/New_York'
assert hass.config.config_source == config_util.SOURCE_YAML


async def test_loading_configuration_from_packages(hass):
Expand Down Expand Up @@ -515,6 +577,7 @@ async def test_discovering_configuration(mock_detect, mock_elevation, hass):
assert hass.config.units.name == CONF_UNIT_SYSTEM_METRIC
assert hass.config.units.is_metric
assert hass.config.time_zone.zone == 'America/Los_Angeles'
assert hass.config.config_source == config_util.SOURCE_DISCOVERED


@asynctest.mock.patch('homeassistant.util.location.async_detect_location_info',
Expand All @@ -539,6 +602,7 @@ async def test_discovering_configuration_auto_detect_fails(mock_detect,
assert hass.config.time_zone == blankConfig.time_zone
assert len(hass.config.whitelist_external_dirs) == 1
assert "/test/config/www" in hass.config.whitelist_external_dirs
assert hass.config.config_source == config_util.SOURCE_DISCOVERED


@asynctest.mock.patch(
Expand Down
1 change: 1 addition & 0 deletions tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -900,6 +900,7 @@ def test_as_dict(self):
'config_dir': '/tmp/ha-config',
'whitelist_external_dirs': set(),
'version': __version__,
'config_source': None,
}

assert expected == self.config.as_dict()
Expand Down