Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 23 additions & 7 deletions homeassistant/helpers/entity_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from collections import UserDict
from collections.abc import Callable, Iterable, Mapping
import logging
from typing import TYPE_CHECKING, Any, cast
from typing import TYPE_CHECKING, Any, TypeVar, cast

import attr
import voluptuous as vol
Expand Down Expand Up @@ -52,6 +52,9 @@

from .entity import EntityCategory

_StrEnumT = TypeVar("_StrEnumT", bound="StrEnum")


PATH_REGISTRY = "entity_registry.yaml"
DATA_REGISTRY = "entity_registry"
EVENT_ENTITY_REGISTRY_UPDATED = "entity_registry_updated"
Expand Down Expand Up @@ -703,6 +706,19 @@ async def async_load(self) -> None:

from .entity import EntityCategory # pylint: disable=import-outside-toplevel

def parse_enum(
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.

We should not define that as inline function, let's place that as standalone function into that file

cls: type[_StrEnumT], value: str | None, default: _StrEnumT | None
) -> _StrEnumT | None:
"""Parse enum and ignore invalid values."""

if not value:
return None
try:
return cls(value)
except ValueError:
_LOGGER.warning("Ignoring unexpected value %s for enum %s", value, cls)
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.

I don't think we should just ignore every unknown value.

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.

Breaking startup of home assistant due to a single errounous value here is not a good ide.

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.

An alternative would be to ignore the complete entity with bad data.

Copy link
Copy Markdown
Member

@frenck frenck May 5, 2022

Choose a reason for hiding this comment

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

Breaking startup of home assistant due to a single errounous value here is not a good ide.

There are thousands of ways custom integrations could break the registries 🤷

Have we pinpointed the root cause/code/integration?

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.

Nope, we have two reported issues that i know of so far.

return default

if data is not None:
for entity in data["entities"]:
# Some old installations can have some bad entities.
Expand All @@ -717,12 +733,12 @@ async def async_load(self) -> None:
config_entry_id=entity["config_entry_id"],
device_class=entity["device_class"],
device_id=entity["device_id"],
disabled_by=RegistryEntryDisabler(entity["disabled_by"])
if entity["disabled_by"]
else None,
entity_category=EntityCategory(entity["entity_category"])
if entity["entity_category"]
else None,
disabled_by=parse_enum(
RegistryEntryDisabler, entity["disabled_by"], None
),
entity_category=parse_enum(
EntityCategory, entity["entity_category"], None
),
entity_id=entity["entity_id"],
hidden_by=entity["hidden_by"],
icon=entity["icon"],
Expand Down
1 change: 1 addition & 0 deletions tests/helpers/test_entity_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,7 @@ async def test_loading_extra_values(hass, hass_storage):
"entity_id": "test.no_name",
"platform": "super_platform",
"unique_id": "without-name",
"disabled_by": "ignored-invalid-disabled-by",
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.

We should also test the logger message

},
{
"entity_id": "test.disabled_user",
Expand Down