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

Deprecate hass.components and log warning if used inside custom component #111508

Merged
merged 5 commits into from
Feb 29, 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
7 changes: 5 additions & 2 deletions homeassistant/helpers/frame.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ def report(
exclude_integrations: set | None = None,
error_if_core: bool = True,
level: int = logging.WARNING,
log_custom_component_only: bool = False,
) -> None:
"""Report incorrect usage.

Expand All @@ -99,10 +100,12 @@ def report(
msg = f"Detected code that {what}. Please report this issue."
if error_if_core:
raise RuntimeError(msg) from err
_LOGGER.warning(msg, stack_info=True)
if not log_custom_component_only:
_LOGGER.warning(msg, stack_info=True)
return

_report_integration(what, integration_frame, level)
if not log_custom_component_only or integration_frame.custom_integration:
_report_integration(what, integration_frame, level)


def _report_integration(
Expand Down
13 changes: 13 additions & 0 deletions homeassistant/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -1247,6 +1247,19 @@ def __getattr__(self, comp_name: str) -> ModuleWrapper:
if component is None:
raise ImportError(f"Unable to load {comp_name}")

# Local import to avoid circular dependencies
from .helpers.frame import report # pylint: disable=import-outside-toplevel
jpbede marked this conversation as resolved.
Show resolved Hide resolved

report(
(
f"accesses hass.components.{comp_name}."
" This is deprecated and will stop working in Home Assistant 2024.6, it"
jpbede marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member Author

Choose a reason for hiding this comment

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

Lets make it 6 months (home-assistant/developers.home-assistant#2093 (review))

Suggested change
" This is deprecated and will stop working in Home Assistant 2024.6, it"
" This is deprecated and will stop working in Home Assistant 2024.9, it"

Copy link
Member

Choose a reason for hiding this comment

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

Was this forgotten?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yikes, yes, thought I've hit "Commit suggestion". Will create a PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in #111903. Thanks for noticing it.

f" should be updated to import functions used from {comp_name} directly"
),
error_if_core=False,
log_custom_component_only=True,
)

wrapped = ModuleWrapper(self._hass, component)
setattr(self, comp_name, wrapped)
return wrapped
Expand Down
27 changes: 27 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,33 @@ def mock_bleak_scanner_start() -> Generator[MagicMock, None, None]:
yield mock_bleak_scanner_start


@pytest.fixture
def mock_integration_frame() -> Generator[Mock, None, None]:
"""Mock as if we're calling code from inside an integration."""
correct_frame = Mock(
filename="/home/paulus/homeassistant/components/hue/light.py",
lineno="23",
line="self.light.is_on",
)
with patch(
"homeassistant.helpers.frame.extract_stack",
return_value=[
Mock(
filename="/home/paulus/homeassistant/core.py",
lineno="23",
line="do_something()",
),
correct_frame,
Mock(
filename="/home/paulus/aiohue/lights.py",
lineno="2",
line="something()",
),
],
):
yield correct_frame


@pytest.fixture
def mock_bluetooth(
mock_bleak_scanner_start: MagicMock, mock_bluetooth_adapters: None
Expand Down
33 changes: 5 additions & 28 deletions tests/helpers/test_frame.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
"""Test the frame helper."""

from collections.abc import Generator
from unittest.mock import ANY, Mock, patch

import pytest
Expand All @@ -9,33 +8,6 @@
from homeassistant.helpers import frame


@pytest.fixture
def mock_integration_frame() -> Generator[Mock, None, None]:
"""Mock as if we're calling code from inside an integration."""
correct_frame = Mock(
filename="/home/paulus/homeassistant/components/hue/light.py",
lineno="23",
line="self.light.is_on",
)
with patch(
"homeassistant.helpers.frame.extract_stack",
return_value=[
Mock(
filename="/home/paulus/homeassistant/core.py",
lineno="23",
line="do_something()",
),
correct_frame,
Mock(
filename="/home/paulus/aiohue/lights.py",
lineno="2",
line="something()",
),
],
):
yield correct_frame


async def test_extract_frame_integration(
caplog: pytest.LogCaptureFixture, mock_integration_frame: Mock
) -> None:
Expand Down Expand Up @@ -174,3 +146,8 @@ async def test_report_missing_integration_frame(
frame.report(what, error_if_core=False)
assert what in caplog.text
assert caplog.text.count(what) == 1

caplog.clear()

frame.report(what, error_if_core=False, log_custom_component_only=True)
assert caplog.text == ""
34 changes: 33 additions & 1 deletion tests/test_loader.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
"""Test to verify that we can load components."""
import asyncio
from unittest.mock import patch
from unittest.mock import Mock, patch

import pytest

from homeassistant import loader
from homeassistant.components import http, hue
from homeassistant.components.hue import light as hue_light
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers import frame

from .common import MockModule, async_get_persistent_notifications, mock_integration

Expand Down Expand Up @@ -287,6 +288,7 @@ async def test_get_integration_custom_component(
) -> None:
"""Test resolving integration."""
integration = await loader.async_get_integration(hass, "test_package")

assert integration.get_component().DOMAIN == "test_package"
assert integration.name == "Test Package"

Expand Down Expand Up @@ -1001,3 +1003,33 @@ async def test_config_folder_not_in_path(hass):

# Verify that we are able to load the file with absolute path
import tests.testing_config.check_config_not_in_path # noqa: F401


async def test_hass_components_use_reported(
hass: HomeAssistant, caplog: pytest.LogCaptureFixture, mock_integration_frame: Mock
) -> None:
"""Test that use of hass.components is reported."""
mock_integration_frame.filename = (
"/home/paulus/homeassistant/custom_components/demo/light.py"
)
integration_frame = frame.IntegrationFrame(
custom_integration=True,
frame=mock_integration_frame,
integration="test_integration_frame",
module="custom_components.test_integration_frame",
relative_filename="custom_components/test_integration_frame/__init__.py",
)

with patch(
"homeassistant.helpers.frame.get_integration_frame",
return_value=integration_frame,
), patch(
"homeassistant.components.http.start_http_server_and_save_config",
return_value=None,
):
hass.components.http.start_http_server_and_save_config(hass, [], None)

assert (
"Detected that custom integration 'test_integration_frame'"
" accesses hass.components.http. This is deprecated"
) in caplog.text
Loading