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

Update mqttplugin.py to paho-mqtt version 2.0.0 #31

Closed
wants to merge 15 commits into from
Closed

Update mqttplugin.py to paho-mqtt version 2.0.0 #31

wants to merge 15 commits into from

Conversation

jdodgen
Copy link
Contributor

@jdodgen jdodgen commented Apr 15, 2024

Not sure why they did this but paho-mqtt 2.0.0 breaks ALL existing implementations. more info here
https://github.com/eclipse/paho.mqtt.python/releases/tag/v2.0.0

Description

During a reinstall of Raspbian and install of my code

installing paho-mqtt brought in version 2.0.0 which broke a few things.
On raspbian:
pip install paho-mqtt
also on an existing ubuntu system:
pip install paho-mqtt --upgrade
both are now 2.0.0

Status

I'm using this new version

Related Issues

My other use of paho-mqtt have been upgraded to VERSION2.
I did not want to do this to mqttplugin at this time.
To keep this change simple, three lines if changed code

Todos

VERSION2 in the future

Steps to Test or Reproduce

install/upgrade paho-mqtt version 2.0.0
run your stuff

Other notes

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 15, 2024

Also when pip upgrading on w11 it also now has 2.0.0

@n8henrie n8henrie changed the base branch from master to dev April 25, 2024 23:20
@n8henrie
Copy link
Owner

Thanks for the PR -- can you please get the tests passing?

First step will be updating the paho-mqtt version in requirements-test.txt. You should be able to test locally with tox.

If you're up for it, I think you'll be able to fix #32 at the same time by passing initial_state to the parent class like so.

@jdodgen
Update requirements-test.txt
fbfe96d 
paho-mqtt now 2.0.0
@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 26, 2024 via email

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 26, 2024 via email

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 26, 2024 via email

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 26, 2024

Ok, a little into tox but a lot of strange errors so I figure I'm doing something wrong.

C:\Users\jim\Desktop\fauxmo-plugins>tox
py37: skipped because could not find python interpreter with spec(s): py37
py37: SKIP ⚠ in 0.19 seconds
py38: skipped because could not find python interpreter with spec(s): py38
py38: SKIP ⚠ in 0.05 seconds
py39: skipped because could not find python interpreter with spec(s): py39
py39: SKIP ⚠ in 0.03 seconds
py310: commands[0]> python -m pytest --showlocals tests/
================================================= test session starts =================================================
platform win32 -- Python 3.10.2, pytest-7.3.0, pluggy-1.5.0
cachedir: .tox\py310\.pytest_cache
rootdir: C:\Users\jim\Desktop\fauxmo-plugins
collected 7 items

tests\test_mqttplugin.py ..F.F                                                                                   [ 71%]
tests\test_restapiplugin.py EE                                                                                   [100%]

======================================================= ERRORS ========================================================
__________________________________ ERROR at setup of test_restapiplugin_integration ___________________________________

    @pytest.fixture(scope="function")
    def restapiplugin_target() -> Generator:
        """Simulate the endpoints triggered by RESTAPIPlugin."""
        httpbin_address = ("127.0.0.1", 8000)
>       ctx = mp.get_context("fork")

httpbin_address = ('127.0.0.1', 8000)

tests\test_restapiplugin.py:22:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\AppData\Local\Programs\Python\Python310\lib\multiprocessing\context.py:239: in get_context
    return super().get_context(method)
        __class__  = <class 'multiprocessing.context.DefaultContext'>
        method     = 'fork'
        self       = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>, method = 'fork'

    def get_context(self, method=None):
        if method is None:
            return self
        try:
            ctx = _concrete_contexts[method]
        except KeyError:
>           raise ValueError('cannot find context for %r' % method) from None
E           ValueError: cannot find context for 'fork'

method     = 'fork'
self       = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>

..\..\AppData\Local\Programs\Python\Python310\lib\multiprocessing\context.py:193: ValueError
______________________________________ ERROR at setup of test_restapiplugin_unit ______________________________________

    @pytest.fixture(scope="function")
    def restapiplugin_target() -> Generator:
        """Simulate the endpoints triggered by RESTAPIPlugin."""
        httpbin_address = ("127.0.0.1", 8000)
>       ctx = mp.get_context("fork")

httpbin_address = ('127.0.0.1', 8000)

tests\test_restapiplugin.py:22:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\AppData\Local\Programs\Python\Python310\lib\multiprocessing\context.py:239: in get_context
    return super().get_context(method)
        __class__  = <class 'multiprocessing.context.DefaultContext'>
        method     = 'fork'
        self       = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>, method = 'fork'

    def get_context(self, method=None):
        if method is None:
            return self
        try:
            ctx = _concrete_contexts[method]
        except KeyError:
>           raise ValueError('cannot find context for %r' % method) from None
E           ValueError: cannot find context for 'fork'

method     = 'fork'
self       = <multiprocessing.context.DefaultContext object at 0x0000017D10AAB250>

..\..\AppData\Local\Programs\Python\Python310\lib\multiprocessing\context.py:193: ValueError
====================================================== FAILURES =======================================================
__________________________________________________ test_mqtt_nostate __________________________________________________

mock_client = <MagicMock name='Client' spec='Client' id='1636710863328'>

    @patch("mqttplugin.Client", autospec=True)
    def test_mqtt_nostate(mock_client: MagicMock) -> None:
        """If state_cmd is not specified, loop_start is not called."""
        mock_instance = mock_client.return_value
        with open(config_path_str) as f:
            config: dict = json.load(f)

        device_conf = next(
            device
            for device in config["PLUGINS"]["MQTTPlugin"]["DEVICES"]
            if device["mqtt_server"] == "mqtt.yes_auth.no_state"
        )
        device = MQTTPlugin(**device_conf)
>       mock_instance.loop_start.assert_not_called()

config     = {'FAUXMO': {'ip_address': 'auto'}, 'PLUGINS': {'MQTTPlugin': {'DEVICES': [{'mqtt_client_id': 'test_client_1', 'mqtt_po...f_cmd': ['Device/Without/Auth/With/State', 'OFF'], ...}], 'path': '/Users/n8henrie/git/fauxmo-plugins/mqttplugin.py'}}}
device     = MQTTPlugin(on_cmd='Device/With/Auth/No/State', on_value='ON', off_cmd='Device/With/Auth/No/State', off_value='OFF', st...gicMock name='Client()' spec='Client' id='1636711006160'>, _name='MQTT Device with auth but no state_cmd', _port=12351)
device_conf = {'mqtt_port': 1883, 'mqtt_pw': 'MyPassword', 'mqtt_server': 'mqtt.yes_auth.no_state', 'mqtt_user': 'MyUser', ...}
f          = <_io.TextIOWrapper name='tests/test_mqttplugin_config.json' mode='r' encoding='cp1252'>
mock_client = <MagicMock name='Client' spec='Client' id='1636710863328'>
mock_instance = <NonCallableMagicMock name='Client()' spec='Client' id='1636711006160'>

tests\test_mqttplugin.py:80:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <MagicMock name='Client().loop_start' spec='function' id='1636711296512'>

    def assert_not_called(self):
        """assert that the mock was never called.
        """
        if self.call_count != 0:
            msg = ("Expected '%s' to not have been called. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'loop_start' to not have been called. Called 1 times.
E           Calls: [call()].

msg        = "Expected 'loop_start' to not have been called. Called 1 times.\nCalls: [call()]."
self       = <MagicMock name='Client().loop_start' spec='function' id='1636711296512'>

..\..\AppData\Local\Programs\Python\Python310\lib\unittest\mock.py:880: AssertionError
_________________________________________________ test_mqtt_clientid __________________________________________________
TypeError: missing a required argument: 'callback_api_version'

The above exception was the direct cause of the following exception:

mock_client = <MagicMock name='Client' spec='Client' id='1636714409760'>

    @patch("mqttplugin.Client", autospec=True)
    def test_mqtt_clientid(mock_client: MagicMock) -> None:
        """Ensure mqtt client id is properly set when configured."""
        with open(config_path_str) as f:
            config: dict = json.load(f)

        device_conf = next(
            device
            for device in config["PLUGINS"]["MQTTPlugin"]["DEVICES"]
            if device["mqtt_client_id"]
        )
        MQTTPlugin(**device_conf)
>       mock_client.assert_called_once_with(
            client_id=device_conf["mqtt_client_id"]
        )

config     = {'FAUXMO': {'ip_address': 'auto'}, 'PLUGINS': {'MQTTPlugin': {'DEVICES': [{'mqtt_client_id': 'test_client_1', 'mqtt_po...f_cmd': ['Device/Without/Auth/With/State', 'OFF'], ...}], 'path': '/Users/n8henrie/git/fauxmo-plugins/mqttplugin.py'}}}
device_conf = {'mqtt_client_id': 'test_client_1', 'mqtt_port': 1883, 'mqtt_server': 'test.mosquitto.org', 'name': 'MQTT Study Light 1', ...}
f          = <_io.TextIOWrapper name='tests/test_mqttplugin_config.json' mode='r' encoding='cp1252'>
mock_client = <MagicMock name='Client' spec='Client' id='1636714409760'>

tests\test_mqttplugin.py:113:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
..\..\AppData\Local\Programs\Python\Python310\lib\unittest\mock.py:931: in assert_called_once_with
    return self.assert_called_with(*args, **kwargs)
        args       = ()
        kwargs     = {'client_id': 'test_client_1'}
        self       = <MagicMock name='Client' spec='Client' id='1636714409760'>
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <MagicMock name='Client' spec='Client' id='1636714409760'>, args = (), kwargs = {'client_id': 'test_client_1'}
expected = TypeError("missing a required argument: 'callback_api_version'")
actual = call('', (<CallbackAPIVersion.VERSION2: 2>, 'test_client_1'), {})
_error_message = <function NonCallableMock.assert_called_with.<locals>._error_message at 0x0000017D14DB4B80>
cause = TypeError("missing a required argument: 'callback_api_version'")

    def assert_called_with(self, /, *args, **kwargs):
        """assert that the last call was made with the specified arguments.

        Raises an AssertionError if the args and keyword args passed in are
        different to the last call to the mock."""
        if self.call_args is None:
            expected = self._format_mock_call_signature(args, kwargs)
            actual = 'not called.'
            error_message = ('expected call not found.\nExpected: %s\nActual: %s'
                    % (expected, actual))
            raise AssertionError(error_message)

        def _error_message():
            msg = self._format_mock_failure_message(args, kwargs)
            return msg
        expected = self._call_matcher(_Call((args, kwargs), two=True))
        actual = self._call_matcher(self.call_args)
        if actual != expected:
            cause = expected if isinstance(expected, Exception) else None
>           raise AssertionError(_error_message()) from cause
E           AssertionError: expected call not found.
E           Expected: Client(client_id='test_client_1')
E           Actual: Client(<CallbackAPIVersion.VERSION2: 2>, client_id='test_client_1')

_error_message = <function NonCallableMock.assert_called_with.<locals>._error_message at 0x0000017D14DB4B80>
actual     = call('', (<CallbackAPIVersion.VERSION2: 2>, 'test_client_1'), {})
args       = ()
cause      = TypeError("missing a required argument: 'callback_api_version'")
expected   = TypeError("missing a required argument: 'callback_api_version'")
kwargs     = {'client_id': 'test_client_1'}
self       = <MagicMock name='Client' spec='Client' id='1636714409760'>

..\..\AppData\Local\Programs\Python\Python310\lib\unittest\mock.py:919: AssertionError
=============================================== short test summary info ===============================================
FAILED tests/test_mqttplugin.py::test_mqtt_nostate - AssertionError: Expected 'loop_start' to not have been called. C...
FAILED tests/test_mqttplugin.py::test_mqtt_clientid - AssertionError: expected call not found.
ERROR tests/test_restapiplugin.py::test_restapiplugin_integration - ValueError: cannot find context for 'fork'
ERROR tests/test_restapiplugin.py::test_restapiplugin_unit - ValueError: cannot find context for 'fork'
======================================== 2 failed, 3 passed, 2 errors in 3.29s ========================================
py310: exit 1 (3.70 seconds) C:\Users\jim\Desktop\fauxmo-plugins> python -m pytest --showlocals tests/ pid=19964
py310: FAIL ✖ in 3.78 seconds
py311: skipped because could not find python interpreter with spec(s): py311
py311: SKIP ⚠ in 0.05 seconds
lint: commands[0]> black --check --diff .
would reformat C:\Users\jim\Desktop\fauxmo-plugins\mqttplugin.py
--- C:\Users\jim\Desktop\fauxmo-plugins\mqttplugin.py   2024-04-26 21:03:35.048337 +0000
+++ C:\Users\jim\Desktop\fauxmo-plugins\mqttplugin.py   2024-04-26 22:00:48.996712 +0000
@@ -132,11 +132,13 @@
         self.status = "unknown"
         self._subscribed = False
         self.initial_state = initial_state
         self.use_fake_state = use_fake_state

-        self.client = Client(CallbackAPIVersion.VERSION2, client_id=mqtt_client_id)
+        self.client = Client(
+            CallbackAPIVersion.VERSION2, client_id=mqtt_client_id
+        )
         if mqtt_user or mqtt_pw:
             self.client.username_pw_set(mqtt_user, mqtt_pw)
         self.client.on_connect = self.on_connect
         self.client.on_subscribe = self.on_subscribe
         self.client.on_message = self.on_message
@@ -155,17 +157,23 @@
     def on_subscribe(
         self,
         client: Client,
         userdata: str,
         mid: int,
-        reason_code_list, properties,
+        reason_code_list,
+        properties,
     ) -> None:
         """Set attribute to show that initial subscription is complete."""
         self._subscribed = True

     def on_connect(
-        self, client: Client, userdata: str, flags: dict, reason_code: int, properties
+        self,
+        client: Client,
+        userdata: str,
+        flags: dict,
+        reason_code: int,
+        properties,
     ) -> None:
         """Subscribe to state command on connect (or reconnect)."""
         if self.state_cmd is not None:
             self.client.subscribe(self.state_cmd)

@@ -225,6 +233,6 @@
             return state

         if self.use_fake_state is True:
             return super().get_state()

-        return "unknown"
\ No newline at end of file
+        return "unknown"

Oh no! 💥 💔 💥
1 file would be reformatted, 6 files would be left unchanged.
lint: exit 1 (0.81 seconds) C:\Users\jim\Desktop\fauxmo-plugins> black --check --diff . pid=11980
  py37: SKIP (0.19 seconds)
  py38: SKIP (0.05 seconds)
  py39: SKIP (0.03 seconds)
  py310: FAIL code 1 (3.78=setup[0.08]+cmd[3.70] seconds)
  py311: SKIP (0.05 seconds)
  lint: FAIL code 1 (0.83=setup[0.02]+cmd[0.81] seconds)
  evaluation failed :( (5.08 seconds)

C:\Users\jim\Desktop\fauxmo-plugins>

@n8henrie
Copy link
Owner

Oh darn, you're on Windows.

It looks like you're using python310; you can run specifically this environment with tox -e py310.

Once all tests are passing, you'll also want to make sure the formatting is up to snuff: tox -e lint.

You can run both at the same time: tox -e py310,lint

@n8henrie
Copy link
Owner

I think the first hint is in your error message: TypeError: missing a required argument: 'callback_api_version' -- looks like the mqtt tests need to be updated for the new version.

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 26, 2024 via email

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 28, 2024 via email •
edited
Loading

@n8henrie
Copy link
Owner

Yes, that's the same error: TypeError: missing a required argument: 'callback_api_version'

@jdodgen
Copy link
Contributor Author

jdodgen commented Apr 30, 2024

Not familiar with how tox works. Is it looking for the actual "callback_api_version" instead of "CallbackAPIVersion" ?

@n8henrie
Copy link
Owner

tox just sets up a clean temporary environment for running the test suite, which is implemented in pytest. You can use pytest directly for local testing. You'll need to look into and modify the mqtt test file in the tests directory.

@n8henrie
Copy link
Owner

Fixed in 141220f

@n8henrie n8henrie closed this May 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jdodgen @n8henrie