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.

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

Use aiohasupervisor for addon info calls #125926

Merged
merged 3 commits into from
Sep 17, 2024
Merged
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
11 changes: 6 additions & 5 deletions homeassistant/components/analytics/analytics.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ async def send_analytics(self, _: datetime | None = None) -> None:
hass = self.hass
supervisor_info = None
operating_system_info: dict[str, Any] = {}
supervisor_client = hassio.get_supervisor_client(hass)

if not self.onboarded or not self.preferences.get(ATTR_BASE, False):
LOGGER.debug("Nothing to submit")
Expand Down Expand Up @@ -263,16 +264,16 @@ async def send_analytics(self, _: datetime | None = None) -> None:
if supervisor_info is not None:
installed_addons = await asyncio.gather(
*(
hassio.async_get_addon_info(hass, addon[ATTR_SLUG])
supervisor_client.addons.addon_info(addon[ATTR_SLUG])
for addon in supervisor_info[ATTR_ADDONS]
)
)
addons.extend(
{
ATTR_SLUG: addon[ATTR_SLUG],
ATTR_PROTECTED: addon[ATTR_PROTECTED],
ATTR_VERSION: addon[ATTR_VERSION],
ATTR_AUTO_UPDATE: addon[ATTR_AUTO_UPDATE],
ATTR_SLUG: addon.slug,
ATTR_PROTECTED: addon.protected,
ATTR_VERSION: addon.version,
ATTR_AUTO_UPDATE: addon.auto_update,
}
for addon in installed_addons
)
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/hassio/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@
HassioAPIError,
async_create_backup,
async_get_addon_discovery_info,
async_get_addon_info,
async_get_addon_store_info,
async_get_green_settings,
async_get_yellow_settings,
Expand All @@ -120,6 +119,7 @@
async_update_diagnostics,
async_update_os,
async_update_supervisor,
get_supervisor_client,
)
from .http import HassIOView
from .ingress import async_setup_ingress_view
Expand Down
27 changes: 17 additions & 10 deletions homeassistant/components/hassio/addon_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,19 @@
import logging
from typing import Any, Concatenate

from aiohasupervisor import SupervisorError
from aiohasupervisor.models import (
AddonState as SupervisorAddonState,
InstalledAddonComplete,
)

from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError

from .handler import (
HassioAPIError,
async_create_backup,
async_get_addon_discovery_info,
async_get_addon_info,
async_get_addon_store_info,
async_install_addon,
async_restart_addon,
Expand All @@ -26,6 +31,7 @@
async_stop_addon,
async_uninstall_addon,
async_update_addon,
get_supervisor_client,
)

type _FuncType[_T, **_P, _R] = Callable[Concatenate[_T, _P], Awaitable[_R]]
Expand Down Expand Up @@ -53,7 +59,7 @@ async def wrapper(
"""Wrap an add-on manager method."""
try:
return_value = await func(self, *args, **kwargs)
except HassioAPIError as err:
except (HassioAPIError, SupervisorError) as err:
raise AddonError(
f"{error_message.format(addon_name=self.addon_name)}: {err}"
) from err
Expand Down Expand Up @@ -140,6 +146,7 @@ async def async_get_addon_discovery_info(self) -> dict:
@api_error("Failed to get the {addon_name} add-on info")
async def async_get_addon_info(self) -> AddonInfo:
"""Return and cache manager add-on info."""
supervisor_client = get_supervisor_client(self._hass)
addon_store_info = await async_get_addon_store_info(self._hass, self.addon_slug)
self._logger.debug("Add-on store info: %s", addon_store_info)
if not addon_store_info["installed"]:
Expand All @@ -152,23 +159,23 @@ async def async_get_addon_info(self) -> AddonInfo:
version=None,
)

addon_info = await async_get_addon_info(self._hass, self.addon_slug)
addon_info = await supervisor_client.addons.addon_info(self.addon_slug)
addon_state = self.async_get_addon_state(addon_info)
return AddonInfo(
available=addon_info["available"],
hostname=addon_info["hostname"],
options=addon_info["options"],
available=addon_info.available,
hostname=addon_info.hostname,
options=addon_info.options,
state=addon_state,
update_available=addon_info["update_available"],
version=addon_info["version"],
update_available=addon_info.update_available,
version=addon_info.version,
)

@callback
def async_get_addon_state(self, addon_info: dict[str, Any]) -> AddonState:
def async_get_addon_state(self, addon_info: InstalledAddonComplete) -> AddonState:
"""Return the current state of the managed add-on."""
addon_state = AddonState.NOT_RUNNING

if addon_info["state"] == "started":
if addon_info.state == SupervisorAddonState.STARTED:
addon_state = AddonState.RUNNING
if self._install_task and not self._install_task.done():
addon_state = AddonState.INSTALLING
Expand Down
12 changes: 9 additions & 3 deletions homeassistant/components/hassio/coordinator.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import logging
from typing import TYPE_CHECKING, Any

from aiohasupervisor import SupervisorError

from homeassistant.config_entries import ConfigEntry
from homeassistant.const import ATTR_MANUFACTURER, ATTR_NAME
from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
Expand Down Expand Up @@ -514,11 +516,15 @@ async def _update_addon_changelog(self, slug: str) -> tuple[str, str | None]:
async def _update_addon_info(self, slug: str) -> tuple[str, dict[str, Any] | None]:
"""Return the info for an add-on."""
try:
info = await self.hassio.get_addon_info(slug)
except HassioAPIError as err:
info = await self.hassio.client.addons.addon_info(slug)
except SupervisorError as err:
_LOGGER.warning("Could not fetch info for %s: %s", slug, err)
return (slug, None)
return (slug, info)
# Translate to legacy hassio names for compatibility
info_dict = info.to_dict()
info_dict["hassio_api"] = info_dict.pop("supervisor_api")
info_dict["hassio_role"] = info_dict.pop("supervisor_role")
return (slug, info_dict)

@callback
def async_enable_container_updates(
Expand Down
11 changes: 6 additions & 5 deletions homeassistant/components/hassio/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

from homeassistant import config_entries
from homeassistant.components.http import HomeAssistantView
from homeassistant.const import ATTR_NAME, ATTR_SERVICE, EVENT_HOMEASSISTANT_START
from homeassistant.const import ATTR_SERVICE, EVENT_HOMEASSISTANT_START
from homeassistant.core import Event, HomeAssistant, callback
from homeassistant.data_entry_flow import BaseServiceInfo
from homeassistant.helpers import discovery_flow
Expand Down Expand Up @@ -99,20 +99,21 @@ async def async_process_new(self, data: dict[str, Any]) -> None:

# Read additional Add-on info
try:
addon_info = await self.hassio.get_addon_info(slug)
addon_info = await self.hassio.client.addons.addon_info(slug)
except HassioAPIError as err:
_LOGGER.error("Can't read add-on info: %s", err)
return

name: str = addon_info[ATTR_NAME]
config_data[ATTR_ADDON] = name
config_data[ATTR_ADDON] = addon_info.name

# Use config flow
discovery_flow.async_create_flow(
self.hass,
service,
context={"source": config_entries.SOURCE_HASSIO},
data=HassioServiceInfo(config=config_data, name=name, slug=slug, uuid=uuid),
data=HassioServiceInfo(
config=config_data, name=addon_info.name, slug=slug, uuid=uuid
),
)

async def async_process_del(self, data: dict[str, Any]) -> None:
Expand Down
37 changes: 17 additions & 20 deletions homeassistant/components/hassio/handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os
from typing import Any

from aiohasupervisor import SupervisorClient
import aiohttp
from yarl import URL

Expand Down Expand Up @@ -62,17 +63,6 @@ async def _wrapper(*argv: _P.args, **kwargs: _P.kwargs) -> Any:
return _wrapper


@bind_hass
async def async_get_addon_info(hass: HomeAssistant, slug: str) -> dict:
"""Return add-on info.

The add-on must be installed.
The caller of the function should handle HassioAPIError.
"""
hassio: HassIO = hass.data[DOMAIN]
return await hassio.get_addon_info(slug)


@api_data
async def async_get_addon_store_info(hass: HomeAssistant, slug: str) -> dict:
"""Return add-on store info.
Expand Down Expand Up @@ -332,7 +322,16 @@ def __init__(
self.loop = loop
self.websession = websession
self._ip = ip
self._base_url = URL(f"http://{ip}")
base_url = f"http://{ip}"
self._base_url = URL(base_url)
self._client = SupervisorClient(
base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession
mdegat01 marked this conversation as resolved.
Show resolved Hide resolved
)

@property
def client(self) -> SupervisorClient:
"""Return aiohasupervisor client."""
return self._client
Comment on lines +327 to +334
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
self._client = SupervisorClient(
base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession
)
@property
def client(self) -> SupervisorClient:
"""Return aiohasupervisor client."""
return self._client
self.client = SupervisorClient(
base_url, os.environ.get("SUPERVISOR_TOKEN", ""), session=websession
)

Why make it protected if you're going to expose it anyway :)

Copy link
Contributor Author

@mdegat01 mdegat01 Sep 17, 2024

Choose a reason for hiding this comment

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

Because if you do it that way then someone can do this:

hassio = HassIO(...)
hassio.client = SupervisorClient(...)

By making it a property like that client is read-only on instances of HassIO. Others can access it to make calls to supervisor, they cannot change its value to something new.

Well They can but they'd have to set hassio._client in my example above and then linting would tell them not to do that.

Copy link
Member

Choose a reason for hiding this comment

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

I am not aware if we have places in the code where we explictly do this, let me check with others

Copy link
Member

Choose a reason for hiding this comment

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

Okay, talked with Martin and this would be good in its current form


@_api_bool
def is_connected(self) -> Coroutine:
Expand Down Expand Up @@ -390,14 +389,6 @@ def get_network_info(self) -> Coroutine:
"""
return self.send_command("/network/info", method="get")

@api_data
def get_addon_info(self, addon: str) -> Coroutine:
"""Return data for a Add-on.

This method returns a coroutine.
"""
return self.send_command(f"/addons/{addon}/info", method="get")

@api_data
def get_core_stats(self) -> Coroutine:
"""Return stats for the core.
Expand Down Expand Up @@ -617,3 +608,9 @@ async def send_command(
_LOGGER.error("Client error on %s request %s", command, err)

raise HassioAPIError


def get_supervisor_client(hass: HomeAssistant) -> SupervisorClient:
"""Return supervisor client."""
hassio: HassIO = hass.data[DOMAIN]
return hassio.client
Comment on lines +613 to +616
Copy link
Member

Choose a reason for hiding this comment

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

You can use HassKey in a future PR to add typing to the hass.data so you can avoid this type overwrite (if wanted)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't aware of that, that would be nice. It would definitely have to be a follow-up PR though as doing that would require a significant refactor to this integration looking at how much we use that currently.

3 changes: 2 additions & 1 deletion homeassistant/components/hassio/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@
"dependencies": ["http", "repairs"],
"documentation": "https://www.home-assistant.io/integrations/hassio",
"iot_class": "local_polling",
"quality_scale": "internal"
"quality_scale": "internal",
"requirements": ["aiohasupervisor==0.1.0b0"]
}
2 changes: 1 addition & 1 deletion homeassistant/components/hassio/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,5 +304,5 @@ async def async_install(
await async_update_core(self.hass, version=version, backup=backup)
except HassioAPIError as err:
raise HomeAssistantError(
f"Error updating Home Assistant Core {err}"
f"Error updating Home Assistant Core: {err}"
) from err
21 changes: 12 additions & 9 deletions homeassistant/components/otbr/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,12 @@
import voluptuous as vol
import yarl

from homeassistant.components.hassio import (
HassioAPIError,
HassioServiceInfo,
async_get_addon_info,
)
from homeassistant.components.hassio import AddonError, AddonManager, HassioServiceInfo
from homeassistant.components.homeassistant_yellow import hardware as yellow_hardware
from homeassistant.components.thread import async_get_preferred_dataset
from homeassistant.config_entries import SOURCE_HASSIO, ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_URL
from homeassistant.core import HomeAssistant
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.aiohttp_client import async_get_clientsession

Expand All @@ -43,6 +39,12 @@ class AlreadyConfigured(HomeAssistantError):
"""Raised when the router is already configured."""


@callback
def get_addon_manager(hass: HomeAssistant, slug: str) -> AddonManager:
Copy link
Member

Choose a reason for hiding this comment

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

Each add-on manager should be a singleton per add-on. We can use the singleton decorator. See other integrations that create add-on managers.

"""Get the add-on manager."""
return AddonManager(hass, _LOGGER, "OpenThread Border Router", slug)


def _is_yellow(hass: HomeAssistant) -> bool:
"""Return True if Home Assistant is running on a Home Assistant Yellow."""
try:
Expand All @@ -55,10 +57,11 @@ def _is_yellow(hass: HomeAssistant) -> bool:
async def _title(hass: HomeAssistant, discovery_info: HassioServiceInfo) -> str:
"""Return config entry title."""
device: str | None = None
addon_manager = get_addon_manager(hass, discovery_info.slug)

with suppress(HassioAPIError):
addon_info = await async_get_addon_info(hass, discovery_info.slug)
device = addon_info.get("options", {}).get("device")
with suppress(AddonError):
addon_info = await addon_manager.async_get_addon_info()
device = addon_info.options.get("device")

if _is_yellow(hass) and device == "/dev/ttyAMA1":
return f"Home Assistant Yellow ({discovery_info.name})"
Expand Down
3 changes: 3 additions & 0 deletions requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,9 @@ aioguardian==2022.07.0
# homeassistant.components.harmony
aioharmony==0.2.10

# homeassistant.components.hassio
aiohasupervisor==0.1.0b0

# homeassistant.components.homekit_controller
aiohomekit==3.2.3

Expand Down
3 changes: 3 additions & 0 deletions requirements_test_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,9 @@ aioguardian==2022.07.0
# homeassistant.components.harmony
aioharmony==0.2.10

# homeassistant.components.hassio
aiohasupervisor==0.1.0b0

# homeassistant.components.homekit_controller
aiohomekit==3.2.3

Expand Down
Loading
Loading