From abbdc85ee7867f1bb5162d4cc0b60f7da698a4ef Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Tue, 14 May 2024 23:42:26 +0300 Subject: [PATCH 1/9] Migrate to `aiomqtt` * Migrated from `asyncio-mqtt` to `aiomqtt` in a straightforward way using `__aenter__` and `__aexit__` instead of `connect`/`disconnect methods, plus removed overriding the latter ones since those have been removed from the base class * Updated runtime and development packages to most recent versions * Github: Docker images are published on each workflow run to facilitate testing, not only when release is created --- .github/workflows/main.yml | 10 ------ pyproject.toml | 2 ++ setup.cfg | 9 ++--- src/energomera_hass_mqtt/hass_mqtt.py | 8 ++--- src/energomera_hass_mqtt/mqtt_client.py | 46 +++---------------------- tests/conftest.py | 2 +- tests/test_online_sensor.py | 2 +- tox.ini | 13 ++++--- 8 files changed, 21 insertions(+), 71 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5ad9857..26ce227 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -118,13 +118,6 @@ jobs: permissions: contents: read packages: write - # Publishing Docker images only happens when a release published out of the - # main branch - if: >- - github.event_name == 'release' - && github.event.action == 'published' - && (github.event.release.target_commitish == 'main' - || github.event.release.target_commitish == 'master') steps: - name: Checkout the code uses: actions/checkout@v3 @@ -149,9 +142,6 @@ jobs: platforms: linux/arm/v7,linux/arm/v6,linux/arm64 push: true tags: - "ghcr.io/${{ github.repository_owner }}\ - /${{ github.event.repository.name }}:latest - ghcr.io/${{ github.repository_owner }}\ /${{ github.event.repository.name }}\ :${{ needs.tests.outputs.version }}" diff --git a/pyproject.toml b/pyproject.toml index 3a9c617..2060d90 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -22,6 +22,8 @@ markers = [ [tool.pylint.main] load-plugins = "pylint.extensions.no_self_use" +# Newer `pylint` raises errors on using `dict()`, ignore those for now +disable = "use-dict-literal" [tool.pylint.typecheck] signature-mutators = [ diff --git a/setup.cfg b/setup.cfg index 0e53d7f..de0ca63 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,13 +31,10 @@ python_requires = >=3.7 install_requires = iec62056-21 @ git+https://github.com/hostcc/iec62056-21.git@feature/transport-improvements addict==2.4.0 - asyncio-mqtt==0.16.2 - # Pin `paho-mqtt` to 1.x until migrating to `aiomqtt`, to prevent - # https://github.com/sbtinstruments/aiomqtt/issues/289 from occuring - paho-mqtt==1.6.1 + aiomqtt==2.1.0 pyyaml==6.0.1 - schema==0.7.5 - python-dateutil==2.8.2 + schema==0.7.7 + python-dateutil==2.9.0 [options.packages.find] where = src diff --git a/src/energomera_hass_mqtt/hass_mqtt.py b/src/energomera_hass_mqtt/hass_mqtt.py index 33f89e7..9ed2c40 100644 --- a/src/energomera_hass_mqtt/hass_mqtt.py +++ b/src/energomera_hass_mqtt/hass_mqtt.py @@ -166,9 +166,9 @@ async def iec_read_admin(self): # connecting to the broker await self.set_online_sensor(False, setup_only=True) # The connection to MQTT broker is instantiated only once, if not - # connected previously. See `MqttClient.connect()` for more - # details - await self._mqtt_client.connect() + # connected previously. + # pylint:disable=unnecessary-dunder-call + await self._mqtt_client.__aenter__() # Process parameters requested for param in self._config.of.parameters: iec_item = self.iec_read_values( @@ -209,7 +209,7 @@ async def finalize(self): """ try: await self.set_online_sensor(False) - await self._mqtt_client.disconnect() + await self._mqtt_client.__aexit__(None, None, None) except Exception: # pylint: disable=broad-except pass diff --git a/src/energomera_hass_mqtt/mqtt_client.py b/src/energomera_hass_mqtt/mqtt_client.py index f794b77..fa4041c 100644 --- a/src/energomera_hass_mqtt/mqtt_client.py +++ b/src/energomera_hass_mqtt/mqtt_client.py @@ -19,23 +19,17 @@ # SOFTWARE. """ -The package provides additional functionality over `asyncio_mqtt`. +The package provides additional functionality over `aiomqtt`. """ import logging -import asyncio_mqtt +import aiomqtt _LOGGER = logging.getLogger(__name__) -class MqttClient(asyncio_mqtt.Client): +class MqttClient(aiomqtt.Client): """ - Class attribute allowing to provide MQTT keepalive down to MQTT client. - Used only by tests at the moment, thus no interface controlling the - attribute is provided. - """ - _keepalive = None - """ - The class extends the `asyncio_mqtt.Client` to provide better convenience + The class extends the `aiomqtt.Client` to provide better convenience working with last wills. Namely, the original class only allows setting the last will thru its constructor, while the `EnergomeraHassMqtt` consuming it only has required property for that around actual publish calls. @@ -45,40 +39,8 @@ class MqttClient(asyncio_mqtt.Client): """ def __init__(self, *args, **kwargs): self._will_set = 'will' in kwargs - # Pass keepalive option down to parent class if set - if self._keepalive: - kwargs['keepalive'] = self._keepalive super().__init__(logger=_LOGGER, *args, **kwargs) - async def connect(self, *args, **kwargs): - """ - Connects to MQTT broker. - Multiple calls will result only in single call to `connect()` method of - parent class if the MQTT client needs a connection (not being connected - or got disconnected), to allow the method to be called within a process - loop with no risk of constantly reinitializing MQTT broker connection. - - :param args: Pass-through positional arguments for parent class - :param kwargs: Pass-through keyword arguments for parent class - - """ - # Using combination of `self._connected` and `self._disconnected` (both - # inherited from `asyncio_mqtt.client`) to detect of MQTT client needs - # a reconnection upon a network error isn't reliable - the former isn't - # finished even after a disconnect, while the latter stays with - # exception after a successfull reconnect. Neither - # `self._client.is_connected` (from Paho client) is - is returns True - # if socket is disconnected due to network error. Only testing for - # `self._client.socket()` (from Paho client as well) fits the purpose - - # None indicates the client needs `connect` - if self._client.socket(): - _LOGGER.debug( - 'MQTT client is already connected, skipping subsequent attempt' - ) - return - - await super().connect(*args, *kwargs) - def will_set(self, *args, **kwargs): """ Allows setting last will to the underlying MQTT client. diff --git a/tests/conftest.py b/tests/conftest.py index 36a3cc1..cbde6ae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1046,7 +1046,7 @@ def mock_mqtt(): ''' # Mock the calls we interested in with patch.multiple(MqttClient, - publish=DEFAULT, connect=DEFAULT, + publish=DEFAULT, __aenter__=DEFAULT, new_callable=AsyncMock) as mocks: # Python 3.7 can't properly distinguish between regular and async calls # using `MagicMock` or `AsyncMock` respectively, so patch the regular diff --git a/tests/test_online_sensor.py b/tests/test_online_sensor.py index 3b60165..ff01f26 100644 --- a/tests/test_online_sensor.py +++ b/tests/test_online_sensor.py @@ -47,7 +47,7 @@ async def trace_connect(*_, **__): mock_mqtt['will_set'].call_args_list[-1] == online_sensor_off_call ) return DEFAULT - mock_mqtt['connect'].side_effect = trace_connect + mock_mqtt['__aenter__'].side_effect = trace_connect # Perform a normal run main() diff --git a/tox.ini b/tox.ini index f3b7ab1..717eed6 100644 --- a/tox.ini +++ b/tox.ini @@ -16,14 +16,13 @@ isolated_build = true deps = check-manifest==0.49 flake8==5.0.4;python_version=="3.7" - flake8==6.0.0;python_version>"3.7" - pylint==2.15.9 - pytest==7.2.0 - pytest-asyncio==0.20.3 - pytest-cov==4.0.0 + flake8==7.0.0;python_version>"3.7" + pylint==3.2.0 + pytest==8.2.0 + pytest-asyncio==0.23.6 + pytest-cov==5.0.0 mock==4.0.3;python_version<"3.8" - freezegun==1.2.2 - docker==6.1.3 + freezegun==1.5.1 setenv = # Ensure the module under test will be found under `src/` directory, in # case of any test command below will attempt importing it. In particular, From 464bf8d99202b482466faa4d6fa8cd2893aeb515 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Tue, 14 May 2024 23:47:17 +0300 Subject: [PATCH 2/9] * `tox`: Use `pylint` 2.x for Python 3.7 - newer tool has higher requirements --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 717eed6..1ba615b 100644 --- a/tox.ini +++ b/tox.ini @@ -17,7 +17,8 @@ deps = check-manifest==0.49 flake8==5.0.4;python_version=="3.7" flake8==7.0.0;python_version>"3.7" - pylint==3.2.0 + pylint==2.15.9;python_version=="3.7" + pylint==3.2.0;python_version>"3.7" pytest==8.2.0 pytest-asyncio==0.23.6 pytest-cov==5.0.0 From 25f6b9f4df86aed7f01e7f318783461d9d84ea47 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Tue, 14 May 2024 23:49:51 +0300 Subject: [PATCH 3/9] * `tox`: Use `pytest-asyncio` 0.20.x for Python 3.7 - newer tool has higher requirements --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 1ba615b..65f75fc 100644 --- a/tox.ini +++ b/tox.ini @@ -20,7 +20,8 @@ deps = pylint==2.15.9;python_version=="3.7" pylint==3.2.0;python_version>"3.7" pytest==8.2.0 - pytest-asyncio==0.23.6 + pytest-asyncio==0.20.3;python_version=="3.7" + pytest-asyncio==0.23.6;python_version>"3.7" pytest-cov==5.0.0 mock==4.0.3;python_version<"3.8" freezegun==1.5.1 From 440bd68df7b58850b49592893c8b913420693819 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Tue, 14 May 2024 23:51:32 +0300 Subject: [PATCH 4/9] * `tox`: Use `pytest-cov` 4.0.x for Python 3.7 - newer tool has higher requirements --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 65f75fc..7adff29 100644 --- a/tox.ini +++ b/tox.ini @@ -22,7 +22,8 @@ deps = pytest==8.2.0 pytest-asyncio==0.20.3;python_version=="3.7" pytest-asyncio==0.23.6;python_version>"3.7" - pytest-cov==5.0.0 + pytest-cov==4.0.0;python_version=="3.7" + pytest-cov==5.0.0;python_version>"3.7" mock==4.0.3;python_version<"3.8" freezegun==1.5.1 setenv = From 70db68a4e0d5381039af4535eec01196edf0d5d4 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Tue, 14 May 2024 23:54:38 +0300 Subject: [PATCH 5/9] * `tox`: Use `pytest` 7.2.x for Python 3.7 - newer tool has higher requirements --- tox.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 7adff29..8904f9d 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,8 @@ deps = flake8==7.0.0;python_version>"3.7" pylint==2.15.9;python_version=="3.7" pylint==3.2.0;python_version>"3.7" - pytest==8.2.0 + pytest==7.2.0;python_version=="3.7" + pytest==8.2.0;python_version>"3.7" pytest-asyncio==0.20.3;python_version=="3.7" pytest-asyncio==0.23.6;python_version>"3.7" pytest-cov==4.0.0;python_version=="3.7" From ab4f631577b0229dbb0a1f75dcf4109295907874 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Wed, 15 May 2024 00:07:36 +0300 Subject: [PATCH 6/9] * Use `aiomqtt` 1.1.x for Python 3.7 - newer package has higher requirements --- setup.cfg | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index de0ca63..90b91f1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -31,7 +31,8 @@ python_requires = >=3.7 install_requires = iec62056-21 @ git+https://github.com/hostcc/iec62056-21.git@feature/transport-improvements addict==2.4.0 - aiomqtt==2.1.0 + aiomqtt==2.1.0;python_version>"3.7" + aiomqtt==1.1.0;python_version=="3.7" pyyaml==6.0.1 schema==0.7.7 python-dateutil==2.9.0 From ba431939fbb2cb5e398d57ea19fd81c9723817f4 Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Wed, 15 May 2024 00:10:59 +0300 Subject: [PATCH 7/9] * Github workflow: fixed unterminated string for Docker image tag --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 26ce227..0ae040c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -142,6 +142,6 @@ jobs: platforms: linux/arm/v7,linux/arm/v6,linux/arm64 push: true tags: - ghcr.io/${{ github.repository_owner }}\ + "ghcr.io/${{ github.repository_owner }}\ /${{ github.event.repository.name }}\ :${{ needs.tests.outputs.version }}" From 3adc23f886c78a1f9d2897f16f3d97c0dc9091db Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Thu, 16 May 2024 00:19:02 +0300 Subject: [PATCH 8/9] * Wrap `__aenter__` and `__aexit__` methods of `aiomqtt.Client` class into `connect` and `disconnect` methods of `MqttClient` one, as per https://github.com/sbtinstruments/aiomqtt/blob/main/docs/migration-guide-v2.md --- src/energomera_hass_mqtt/hass_mqtt.py | 5 ++--- src/energomera_hass_mqtt/mqtt_client.py | 28 +++++++++++++++++++++++++ tests/conftest.py | 2 +- tests/test_online_sensor.py | 2 +- 4 files changed, 32 insertions(+), 5 deletions(-) diff --git a/src/energomera_hass_mqtt/hass_mqtt.py b/src/energomera_hass_mqtt/hass_mqtt.py index 9ed2c40..137306c 100644 --- a/src/energomera_hass_mqtt/hass_mqtt.py +++ b/src/energomera_hass_mqtt/hass_mqtt.py @@ -167,8 +167,7 @@ async def iec_read_admin(self): await self.set_online_sensor(False, setup_only=True) # The connection to MQTT broker is instantiated only once, if not # connected previously. - # pylint:disable=unnecessary-dunder-call - await self._mqtt_client.__aenter__() + await self._mqtt_client.connect() # Process parameters requested for param in self._config.of.parameters: iec_item = self.iec_read_values( @@ -209,7 +208,7 @@ async def finalize(self): """ try: await self.set_online_sensor(False) - await self._mqtt_client.__aexit__(None, None, None) + await self._mqtt_client.disconnect() except Exception: # pylint: disable=broad-except pass diff --git a/src/energomera_hass_mqtt/mqtt_client.py b/src/energomera_hass_mqtt/mqtt_client.py index fa4041c..81f53b8 100644 --- a/src/energomera_hass_mqtt/mqtt_client.py +++ b/src/energomera_hass_mqtt/mqtt_client.py @@ -41,6 +41,34 @@ def __init__(self, *args, **kwargs): self._will_set = 'will' in kwargs super().__init__(logger=_LOGGER, *args, **kwargs) + async def connect(self): + """ + Connects to MQTT broker using `__aenter__` method of the base class as + recommended in + https://github.com/sbtinstruments/aiomqtt/blob/main/docs/migration-guide-v2.md. + Multiple calls will result only in single call to `__aenter__()` method + of parent class if the MQTT client needs a connection (not being + connected or got disconnected), to allow the method to be called within + a process loop with no risk of hitting non-reentrant error from base + class. + """ + if self._lock.locked(): + _LOGGER.debug( + 'MQTT client is already connected, skipping subsequent attempt' + ) + return + + # pylint:disable=unnecessary-dunder-call + await self.__aenter__() + + async def disconnect(self): + """ + Disconnects from MQTT broker using `__aexit__` method of the base class + as per migration guide above. + """ + # pylint:disable=unnecessary-dunder-call + await self.__aexit__(None, None, None) + def will_set(self, *args, **kwargs): """ Allows setting last will to the underlying MQTT client. diff --git a/tests/conftest.py b/tests/conftest.py index cbde6ae..36a3cc1 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1046,7 +1046,7 @@ def mock_mqtt(): ''' # Mock the calls we interested in with patch.multiple(MqttClient, - publish=DEFAULT, __aenter__=DEFAULT, + publish=DEFAULT, connect=DEFAULT, new_callable=AsyncMock) as mocks: # Python 3.7 can't properly distinguish between regular and async calls # using `MagicMock` or `AsyncMock` respectively, so patch the regular diff --git a/tests/test_online_sensor.py b/tests/test_online_sensor.py index ff01f26..3b60165 100644 --- a/tests/test_online_sensor.py +++ b/tests/test_online_sensor.py @@ -47,7 +47,7 @@ async def trace_connect(*_, **__): mock_mqtt['will_set'].call_args_list[-1] == online_sensor_off_call ) return DEFAULT - mock_mqtt['__aenter__'].side_effect = trace_connect + mock_mqtt['connect'].side_effect = trace_connect # Perform a normal run main() From fddff774cc5edd7c9808245117851f83d6313cbf Mon Sep 17 00:00:00 2001 From: Ilia Sotnikov Date: Thu, 16 May 2024 00:31:38 +0300 Subject: [PATCH 9/9] - Dropped support for Python 3.7 (aiomqtt 2.x supports only 3.8+) and added Python 3.11 - 3.12 --- .github/workflows/main.yml | 6 +++--- setup.cfg | 8 ++++---- tests/conftest.py | 16 +++------------- tox.ini | 18 ++++++------------ 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 0ae040c..8e18dc7 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -17,9 +17,6 @@ jobs: fail-fast: false matrix: include: - - os: ubuntu-latest - python: 3.7 - toxenv: py - os: ubuntu-latest python: 3.8 toxenv: py @@ -32,6 +29,9 @@ jobs: - os: ubuntu-latest python: '3.11' toxenv: py + - os: ubuntu-latest + python: '3.12' + toxenv: py runs-on: ${{ matrix.os }} outputs: version: ${{ steps.package-version.outputs.VALUE }} diff --git a/setup.cfg b/setup.cfg index 90b91f1..88e486e 100644 --- a/setup.cfg +++ b/setup.cfg @@ -17,22 +17,22 @@ classifiers = Topic :: System :: Hardware License :: OSI Approved :: MIT License Programming Language :: Python :: 3 - Programming Language :: Python :: 3.7 Programming Language :: Python :: 3.8 Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 + Programming Language :: Python :: 3.11 + Programming Language :: Python :: 3.12 Programming Language :: Python :: 3 :: Only [options] package_dir = = src packages = find: -python_requires = >=3.7 +python_requires = >=3.8 install_requires = iec62056-21 @ git+https://github.com/hostcc/iec62056-21.git@feature/transport-improvements addict==2.4.0 - aiomqtt==2.1.0;python_version>"3.7" - aiomqtt==1.1.0;python_version=="3.7" + aiomqtt==2.1.0 pyyaml==6.0.1 schema==0.7.7 python-dateutil==2.9.0 diff --git a/tests/conftest.py b/tests/conftest.py index 36a3cc1..d00fa22 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -10,12 +10,7 @@ import pytest import iec62056_21.transports from energomera_hass_mqtt.mqtt_client import MqttClient -try: - from unittest.mock import AsyncMock -except ImportError: - # AsyncMock introduced in Python 3.8, import from alternative package if - # older - from mock import AsyncMock +from unittest.mock import AsyncMock SERIAL_EXCHANGE_COMPLETE = [ @@ -1046,11 +1041,6 @@ def mock_mqtt(): ''' # Mock the calls we interested in with patch.multiple(MqttClient, - publish=DEFAULT, connect=DEFAULT, + publish=DEFAULT, connect=DEFAULT, will_set=DEFAULT, new_callable=AsyncMock) as mocks: - # Python 3.7 can't properly distinguish between regular and async calls - # using `MagicMock` or `AsyncMock` respectively, so patch the regular - # method separately - with patch.object(MqttClient, 'will_set') as will_mock: - mocks.update({'will_set': will_mock}) - yield mocks + yield mocks diff --git a/tox.ini b/tox.ini index 8904f9d..f329f14 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py{37,38,39,310,311} +envlist = py{38,39,310,311,312} # Define the minimal tox version required to run; # if the host tox is less than this the tool with create an environment and @@ -15,17 +15,11 @@ isolated_build = true [testenv] deps = check-manifest==0.49 - flake8==5.0.4;python_version=="3.7" - flake8==7.0.0;python_version>"3.7" - pylint==2.15.9;python_version=="3.7" - pylint==3.2.0;python_version>"3.7" - pytest==7.2.0;python_version=="3.7" - pytest==8.2.0;python_version>"3.7" - pytest-asyncio==0.20.3;python_version=="3.7" - pytest-asyncio==0.23.6;python_version>"3.7" - pytest-cov==4.0.0;python_version=="3.7" - pytest-cov==5.0.0;python_version>"3.7" - mock==4.0.3;python_version<"3.8" + flake8==7.0.0 + pylint==3.2.0 + pytest==8.2.0 + pytest-asyncio==0.23.6 + pytest-cov==5.0.0 freezegun==1.5.1 setenv = # Ensure the module under test will be found under `src/` directory, in