Skip to content
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

Jump to bottom

Store runtime data in entry in Analytics Insights #116441

Merged
merged 4 commits into from
May 1, 2024
Merged

Store runtime data in entry in Analytics Insights #116441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f75d147
Store runtime data in entry in Analytics Insights
joostlek Apr 30, 2024
ff3bd79
Fix
joostlek Apr 30, 2024
780af10
Fix
joostlek Apr 30, 2024
d679dca
Fix
joostlek Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 13 additions & 9 deletions homeassistant/components/analytics_insights/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@
from homeassistant.exceptions import ConfigEntryNotReady
from homeassistant.helpers.aiohttp_client import async_get_clientsession

from .const import CONF_TRACKED_INTEGRATIONS, DOMAIN
from .const import CONF_TRACKED_INTEGRATIONS
from .coordinator import HomeassistantAnalyticsDataUpdateCoordinator

PLATFORMS: list[Platform] = [Platform.SENSOR]
AnalyticsInsightsConfigEntry = ConfigEntry["AnalyticsInsightsData"]


@dataclass(frozen=True)
Expand All @@ -29,7 +30,9 @@ class AnalyticsInsightsData:
names: dict[str, str]


async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
async def async_setup_entry(
hass: HomeAssistant, entry: AnalyticsInsightsConfigEntry
) -> bool:
"""Set up Homeassistant Analytics from a config entry."""
client = HomeassistantAnalyticsClient(session=async_get_clientsession(hass))

Expand All @@ -49,22 +52,23 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:

await coordinator.async_config_entry_first_refresh()

hass.data[DOMAIN] = AnalyticsInsightsData(coordinator=coordinator, names=names)
entry.runtime_data = AnalyticsInsightsData(coordinator=coordinator, names=names)

await hass.config_entries.async_forward_entry_setups(entry, PLATFORMS)
entry.async_on_unload(entry.add_update_listener(update_listener))

return True


async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
async def async_unload_entry(
hass: HomeAssistant, entry: AnalyticsInsightsConfigEntry
) -> bool:
"""Unload a config entry."""
if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS):
hass.data.pop(DOMAIN)
return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)

return unload_ok


async def update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None:
async def update_listener(
hass: HomeAssistant, entry: AnalyticsInsightsConfigEntry
) -> None:
"""Handle options update."""
await hass.config_entries.async_reload(entry.entry_id)
7 changes: 5 additions & 2 deletions homeassistant/components/analytics_insights/coordinator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

from dataclasses import dataclass
from datetime import timedelta
from typing import TYPE_CHECKING

from python_homeassistant_analytics import (
CustomIntegration,
Expand All @@ -12,7 +13,6 @@
HomeassistantAnalyticsNotModifiedError,
)

from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed

Expand All @@ -23,6 +23,9 @@
LOGGER,
)

if TYPE_CHECKING:
from . import AnalyticsInsightsConfigEntry


@dataclass(frozen=True)
class AnalyticsData:
Expand All @@ -35,7 +38,7 @@ class AnalyticsData:
class HomeassistantAnalyticsDataUpdateCoordinator(DataUpdateCoordinator[AnalyticsData]):
"""A Homeassistant Analytics Data Update Coordinator."""

config_entry: ConfigEntry
config_entry: AnalyticsInsightsConfigEntry

def __init__(
self, hass: HomeAssistant, client: HomeassistantAnalyticsClient
Expand Down
7 changes: 3 additions & 4 deletions homeassistant/components/analytics_insights/sensor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,14 @@
SensorEntityDescription,
SensorStateClass,
)
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import EntityCategory
from homeassistant.core import HomeAssistant
from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
from homeassistant.helpers.entity_platform import AddEntitiesCallback
from homeassistant.helpers.typing import StateType
from homeassistant.helpers.update_coordinator import CoordinatorEntity

from . import AnalyticsInsightsData
from . import AnalyticsInsightsConfigEntry
from .const import DOMAIN
from .coordinator import AnalyticsData, HomeassistantAnalyticsDataUpdateCoordinator

Expand Down Expand Up @@ -60,12 +59,12 @@ def get_custom_integration_entity_description(

async def async_setup_entry(
hass: HomeAssistant,
entry: ConfigEntry,
entry: AnalyticsInsightsConfigEntry,
async_add_entities: AddEntitiesCallback,
) -> None:
"""Initialize the entries."""

analytics_data: AnalyticsInsightsData = hass.data[DOMAIN]
analytics_data = entry.runtime_data
coordinator: HomeassistantAnalyticsDataUpdateCoordinator = (
analytics_data.coordinator
)
Expand Down
2 changes: 1 addition & 1 deletion tests/components/analytics_insights/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from python_homeassistant_analytics import CurrentAnalytics
from python_homeassistant_analytics.models import CustomIntegration, Integration

from homeassistant.components.analytics_insights import DOMAIN
from homeassistant.components.analytics_insights.const import (
CONF_TRACKED_CUSTOM_INTEGRATIONS,
CONF_TRACKED_INTEGRATIONS,
DOMAIN,
Comment on lines -10 to +13
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, outside code should not import from submodules (e.g. const). Everything needed should be exported in __init__.py.

In this case CONF_TRACKED_CUSTOM_INTEGRATIONS is the only thing missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

We never really had this requirement. That would require integrations to provide a __all__ block, do we really want that?

Copy link
Member

Choose a reason for hiding this comment

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

We never really had this requirement.

True. It's more a best practice. Basically consider everything within an integration private and only access the top-level from the outside. As long as that stays the same, you're free to move stuff inside around as you like. E.g. move DOMAIN from __init__.py to const.py.

That would require integrations to provide a __all__ block, do we really want that?

Not necessarily. An import with # noqa: F401 is enough.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought this rule only applied to components importing from other components, and didn't apply to component tests importing from their own component.

Copy link
Member

Choose a reason for hiding this comment

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

I thought this rule only applied to components importing from other components, and didn't apply to component tests importing from their own component.

It's not a defined rule in itself as far as I'm aware of. I'd just consider it good practice.
However, that doesn't need to hold up this PR thus I approved it just now.

Copy link
Member Author

Choose a reason for hiding this comment

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

I mean, if this is a call we want to make, we should consult it with a broader audience and support the decision with the right tooling so we don't have to check it out while doing reviews

)

from tests.common import MockConfigEntry, load_fixture, load_json_object_fixture
Expand Down