Load HA core config from storage#23872
Conversation
| _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]]): |
There was a problem hiding this comment.
Creating a list is unnecessary here (https://twitter.com/raymondh/status/1125487457443532800). Admittedly, this is a micro-optimization though.
There was a problem hiding this comment.
It's even a pico-optimization ;)
Do you have a suggestion for a cleaner way to do it though?
There was a problem hiding this comment.
What do you mean? You just have to remove the [] so that the call inside any is a generator, not a list
| if data: | ||
| hac = hass.config | ||
| hac.config_source = SOURCE_STORAGE | ||
| hac.latitude = data['latitude'] |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
OK, how about validating the data with a schema, and rejecting it if it doesn't pass?
There was a problem hiding this comment.
Yeah that could probably work
emontnemery
changed the title
balloob
left a comment
balloob
left a comment
There was a problem hiding this comment.
Ok to merge when final comment addressed!
* Load HA core config from storage * Tweak * Lint, review comments * Fix test * Add tests * Lint * Address comments
5 participants
Description:
Initial steps of home-assistant/architecture#176:
Checklist:
tox. Your PR cannot be merged unless tests passIf user exposed functionality or configuration variables are added/changed:
If the code does not interact with devices: