Remove log flooding prevention logic from discovery info (#61243)
Co-authored-by: epenet <epenet@users.noreply.github.com>
This commit is contained in:
parent
9c11bb8ba1
commit
c05eca1c82
12 changed files with 163 additions and 159 deletions
|
@ -68,22 +68,17 @@ class DhcpServiceInfo(BaseServiceInfo):
|
|||
hostname: str
|
||||
macaddress: str
|
||||
|
||||
# Used to prevent log flooding. To be removed in 2022.6
|
||||
_warning_logged: bool = False
|
||||
|
||||
def __getitem__(self, name: str) -> Any:
|
||||
"""
|
||||
Enable method for compatibility reason.
|
||||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
return getattr(self, name)
|
||||
|
||||
def get(self, name: str, default: Any = None) -> Any:
|
||||
|
@ -92,13 +87,11 @@ class DhcpServiceInfo(BaseServiceInfo):
|
|||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
if hasattr(self, name):
|
||||
return getattr(self, name)
|
||||
return default
|
||||
|
|
|
@ -261,22 +261,17 @@ class MqttServiceInfo(BaseServiceInfo):
|
|||
subscribed_topic: str
|
||||
timestamp: dt.datetime
|
||||
|
||||
# Used to prevent log flooding. To be removed in 2022.6
|
||||
_warning_logged: bool = False
|
||||
|
||||
def __getitem__(self, name: str) -> Any:
|
||||
"""
|
||||
Allow property access by name for compatibility reason.
|
||||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={"mqtt"},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
return getattr(self, name)
|
||||
|
||||
|
||||
|
|
|
@ -103,22 +103,17 @@ class SsdpServiceInfo(
|
|||
):
|
||||
"""Prepared info from ssdp/upnp entries."""
|
||||
|
||||
# Used to prevent log flooding. To be removed in 2022.6
|
||||
_warning_logged: bool = False
|
||||
|
||||
def __getitem__(self, name: str) -> Any:
|
||||
"""
|
||||
Allow property access by name for compatibility reason.
|
||||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
# Use a property if it is available, fallback to upnp data
|
||||
if hasattr(self, name):
|
||||
return getattr(self, name)
|
||||
|
@ -132,13 +127,11 @@ class SsdpServiceInfo(
|
|||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
if hasattr(self, name):
|
||||
return getattr(self, name)
|
||||
return self.upnp.get(name, self.ssdp_headers.get(name, default))
|
||||
|
@ -149,14 +142,12 @@ class SsdpServiceInfo(
|
|||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
"accessed discovery_info.__contains__() instead of discovery_info.upnp.__contains__() "
|
||||
"or discovery_info.ssdp_headers.__contains__(); this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info.__contains__('{name}') instead of discovery_info.upnp.__contains__('{name}') "
|
||||
f"or discovery_info.ssdp_headers.__contains__('{name}'); this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
if hasattr(self, name):
|
||||
return getattr(self, name) is not None
|
||||
return name in self.upnp or name in self.ssdp_headers
|
||||
|
|
|
@ -44,22 +44,17 @@ class UsbServiceInfo(BaseServiceInfo):
|
|||
manufacturer: str | None
|
||||
description: str | None
|
||||
|
||||
# Used to prevent log flooding. To be removed in 2022.6
|
||||
_warning_logged: bool = False
|
||||
|
||||
def __getitem__(self, name: str) -> Any:
|
||||
"""
|
||||
Allow property access by name for compatibility reason.
|
||||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={"usb"},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
return getattr(self, name)
|
||||
|
||||
|
||||
|
|
|
@ -107,22 +107,17 @@ class ZeroconfServiceInfo(BaseServiceInfo):
|
|||
name: str
|
||||
properties: dict[str, Any]
|
||||
|
||||
# Used to prevent log flooding. To be removed in 2022.6
|
||||
_warning_logged: bool = False
|
||||
|
||||
def __getitem__(self, name: str) -> Any:
|
||||
"""
|
||||
Enable method for compatibility reason.
|
||||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info['{name}'] instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
return getattr(self, name)
|
||||
|
||||
def get(self, name: str, default: Any = None) -> Any:
|
||||
|
@ -131,13 +126,11 @@ class ZeroconfServiceInfo(BaseServiceInfo):
|
|||
|
||||
Deprecated, and will be removed in version 2022.6.
|
||||
"""
|
||||
if not self._warning_logged:
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
self._warning_logged = True
|
||||
report(
|
||||
f"accessed discovery_info.get('{name}') instead of discovery_info.{name}; this will fail in version 2022.6",
|
||||
exclude_integrations={DOMAIN},
|
||||
error_if_core=False,
|
||||
)
|
||||
if hasattr(self, name):
|
||||
return getattr(self, name)
|
||||
return default
|
||||
|
|
|
@ -3,7 +3,8 @@ import datetime
|
|||
import threading
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from scapy import arch # pylint: unused-import # noqa: F401
|
||||
import pytest
|
||||
from scapy import arch # pylint: disable=unused-import # noqa: F401
|
||||
from scapy.error import Scapy_Exception
|
||||
from scapy.layers.dhcp import DHCP
|
||||
from scapy.layers.l2 import Ether
|
||||
|
@ -845,6 +846,7 @@ async def test_aiodiscover_finds_new_hosts_after_interval(hass):
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("mock_integration_frame")
|
||||
async def test_service_info_compatibility(hass, caplog):
|
||||
"""Test compatibility with old-style dict.
|
||||
|
||||
|
@ -856,21 +858,13 @@ async def test_service_info_compatibility(hass, caplog):
|
|||
macaddress="b8b7f16db533",
|
||||
)
|
||||
|
||||
# Ensure first call get logged
|
||||
assert discovery_info["ip"] == "192.168.210.56"
|
||||
assert discovery_info.get("ip") == "192.168.210.56"
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info["ip"] == "192.168.210.56"
|
||||
assert "Detected integration that accessed discovery_info['ip']" in caplog.text
|
||||
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info.get("ip") == "192.168.210.56"
|
||||
assert "Detected integration that accessed discovery_info.get('ip')" in caplog.text
|
||||
|
||||
assert discovery_info.get("ip", "fallback_host") == "192.168.210.56"
|
||||
assert discovery_info.get("invalid_key", "fallback_host") == "fallback_host"
|
||||
assert "Detected code that accessed discovery_info['ip']" in caplog.text
|
||||
assert "Detected code that accessed discovery_info.get('ip')" not in caplog.text
|
||||
|
||||
# Ensure second call doesn't get logged
|
||||
caplog.clear()
|
||||
assert discovery_info["ip"] == "192.168.210.56"
|
||||
assert discovery_info.get("ip") == "192.168.210.56"
|
||||
assert "Detected code that accessed discovery_info['ip']" not in caplog.text
|
||||
assert "Detected code that accessed discovery_info.get('ip')" not in caplog.text
|
||||
|
||||
discovery_info._warning_logged = False # pylint: disable=[protected-access]
|
||||
assert discovery_info.get("ip") == "192.168.210.56"
|
||||
assert "Detected code that accessed discovery_info.get('ip')" in caplog.text
|
||||
|
|
|
@ -1814,6 +1814,7 @@ async def test_publish_json_from_template(hass, mqtt_mock):
|
|||
assert mqtt_mock.async_publish.call_args[0][1] == test_str
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("mock_integration_frame")
|
||||
async def test_service_info_compatibility(hass, caplog):
|
||||
"""Test compatibility with old-style dict.
|
||||
|
||||
|
@ -1828,11 +1829,6 @@ async def test_service_info_compatibility(hass, caplog):
|
|||
timestamp=None,
|
||||
)
|
||||
|
||||
# Ensure first call get logged
|
||||
assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config"
|
||||
assert "Detected code that accessed discovery_info['topic']" in caplog.text
|
||||
|
||||
# Ensure second call doesn't get logged
|
||||
caplog.clear()
|
||||
assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config"
|
||||
assert "Detected code that accessed discovery_info['topic']" not in caplog.text
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info["topic"] == "tasmota/discovery/DC4F220848A2/config"
|
||||
assert "Detected integration that accessed discovery_info['topic']" in caplog.text
|
||||
|
|
|
@ -793,3 +793,61 @@ async def test_ipv4_does_additional_search_for_sonos(
|
|||
),
|
||||
)
|
||||
assert ssdp_listener.async_search.call_args[1] == {}
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("mock_integration_frame")
|
||||
async def test_service_info_compatibility(hass, caplog):
|
||||
"""Test compatibility with old-style dict.
|
||||
|
||||
To be removed in 2022.6
|
||||
"""
|
||||
discovery_info = ssdp.SsdpServiceInfo(
|
||||
ssdp_st="mock-st",
|
||||
ssdp_location="http://1.1.1.1",
|
||||
ssdp_usn="uuid:mock-udn::mock-st",
|
||||
ssdp_server="mock-server",
|
||||
ssdp_ext="",
|
||||
ssdp_headers=_ssdp_headers(
|
||||
{
|
||||
"st": "mock-st",
|
||||
"location": "http://1.1.1.1",
|
||||
"usn": "uuid:mock-udn::mock-st",
|
||||
"server": "mock-server",
|
||||
"ext": "",
|
||||
}
|
||||
),
|
||||
upnp={ssdp.ATTR_UPNP_DEVICE_TYPE: "ABC"},
|
||||
)
|
||||
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info["ssdp_st"] == "mock-st"
|
||||
assert "Detected integration that accessed discovery_info['ssdp_st']" in caplog.text
|
||||
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info.get("ssdp_location") == "http://1.1.1.1"
|
||||
assert (
|
||||
"Detected integration that accessed discovery_info.get('ssdp_location')"
|
||||
in caplog.text
|
||||
)
|
||||
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert "ssdp_usn" in discovery_info
|
||||
assert (
|
||||
"Detected integration that accessed discovery_info.__contains__('ssdp_usn')"
|
||||
in caplog.text
|
||||
)
|
||||
|
||||
# Root item
|
||||
assert discovery_info["ssdp_usn"] == "uuid:mock-udn::mock-st"
|
||||
assert discovery_info.get("ssdp_usn") == "uuid:mock-udn::mock-st"
|
||||
assert "ssdp_usn" in discovery_info
|
||||
|
||||
# SSDP header
|
||||
assert discovery_info["st"] == "mock-st"
|
||||
assert discovery_info.get("st") == "mock-st"
|
||||
assert "st" in discovery_info
|
||||
|
||||
# UPnP item
|
||||
assert discovery_info[ssdp.ATTR_UPNP_DEVICE_TYPE] == "ABC"
|
||||
assert discovery_info.get(ssdp.ATTR_UPNP_DEVICE_TYPE) == "ABC"
|
||||
assert ssdp.ATTR_UPNP_DEVICE_TYPE in discovery_info
|
||||
|
|
|
@ -779,6 +779,7 @@ def test_human_readable_device_name():
|
|||
assert "8A2A" in name
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("mock_integration_frame")
|
||||
async def test_service_info_compatibility(hass, caplog):
|
||||
"""Test compatibility with old-style dict.
|
||||
|
||||
|
@ -794,10 +795,6 @@ async def test_service_info_compatibility(hass, caplog):
|
|||
)
|
||||
|
||||
# Ensure first call get logged
|
||||
assert discovery_info["vid"] == 12345
|
||||
assert "Detected code that accessed discovery_info['vid']" in caplog.text
|
||||
|
||||
# Ensure second call doesn't get logged
|
||||
caplog.clear()
|
||||
assert discovery_info["vid"] == 12345
|
||||
assert "Detected code that accessed discovery_info['vid']" not in caplog.text
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info["vid"] == 12345
|
||||
assert "Detected integration that accessed discovery_info['vid']" in caplog.text
|
||||
|
|
|
@ -3,6 +3,7 @@ from ipaddress import ip_address
|
|||
from typing import Any
|
||||
from unittest.mock import call, patch
|
||||
|
||||
import pytest
|
||||
from zeroconf import InterfaceChoice, IPVersion, ServiceStateChange
|
||||
from zeroconf.asyncio import AsyncServiceInfo
|
||||
|
||||
|
@ -1032,6 +1033,7 @@ async def test_no_name(hass, mock_async_zeroconf):
|
|||
assert info.name == "Home._home-assistant._tcp.local."
|
||||
|
||||
|
||||
@pytest.mark.usefixtures("mock_integration_frame")
|
||||
async def test_service_info_compatibility(hass, caplog):
|
||||
"""Test compatibility with old-style dict.
|
||||
|
||||
|
@ -1046,21 +1048,15 @@ async def test_service_info_compatibility(hass, caplog):
|
|||
properties={},
|
||||
)
|
||||
|
||||
# Ensure first call get logged
|
||||
assert discovery_info["host"] == "mock_host"
|
||||
assert discovery_info.get("host") == "mock_host"
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info["host"] == "mock_host"
|
||||
assert "Detected integration that accessed discovery_info['host']" in caplog.text
|
||||
|
||||
with patch("homeassistant.helpers.frame._REPORTED_INTEGRATIONS", set()):
|
||||
assert discovery_info.get("host") == "mock_host"
|
||||
assert (
|
||||
"Detected integration that accessed discovery_info.get('host')" in caplog.text
|
||||
)
|
||||
|
||||
assert discovery_info.get("host", "fallback_host") == "mock_host"
|
||||
assert discovery_info.get("invalid_key", "fallback_host") == "fallback_host"
|
||||
assert "Detected code that accessed discovery_info['host']" in caplog.text
|
||||
assert "Detected code that accessed discovery_info.get('host')" not in caplog.text
|
||||
|
||||
# Ensure second call doesn't get logged
|
||||
caplog.clear()
|
||||
assert discovery_info["host"] == "mock_host"
|
||||
assert discovery_info.get("host") == "mock_host"
|
||||
assert "Detected code that accessed discovery_info['host']" not in caplog.text
|
||||
assert "Detected code that accessed discovery_info.get('host')" not in caplog.text
|
||||
|
||||
discovery_info._warning_logged = False
|
||||
assert discovery_info.get("host") == "mock_host"
|
||||
assert "Detected code that accessed discovery_info.get('host')" in caplog.text
|
||||
|
|
|
@ -6,7 +6,7 @@ import logging
|
|||
import socket
|
||||
import ssl
|
||||
import threading
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, Mock, patch
|
||||
|
||||
from aiohttp.test_utils import make_mocked_request
|
||||
import freezegun
|
||||
|
@ -778,3 +778,30 @@ def hass_recorder(enable_statistics, hass_storage):
|
|||
|
||||
yield setup_recorder
|
||||
hass.stop()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_integration_frame():
|
||||
"""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
|
||||
|
|
|
@ -1,31 +0,0 @@
|
|||
"""Fixtures for helpers."""
|
||||
from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_integration_frame():
|
||||
"""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
|
Loading…
Add table
Reference in a new issue