tavern
tavern copied to clipboard
Bind all MQTT Client callbacks
Fix for #753 where I bound all the available MQTT callbacks that would be useful to a developer facing connection issues. There are still a few callbacks that could be bound, but I haven't ever needed them.
Ran the code formatter and smoke tests on the code. Unit tests seem to have unrelated failures, and there is only calls to the logger in this. If there is a good way you wanted to unit test the new callbacks, let me know.
Seems the failures in the test report are related to two things:
- Using f-strings in logger calls is not optimal performance-wise; will switch those.
- Accessing the protected _client_id in the paho client is frowned upon. I want to give the client ID that paho is using in the logger calls, but there isn't a public getter, so I'll remove that debugging; the client ID is already printed in debug should you need it earlier in the logs.
It's fine to access the private client ID, I think I already do it in a few other places. If they don't offer an API to get that data then it's the only option :woman_shrugging:
Ok, got it. Given that, is that actually what's causing the errors? I'm not super familiar with pylint.
Your code has been rated at 9.97/10
ERROR: InvocationError for command /home/runner/work/tavern/tavern/.tox/py39lint/bin/pylint tavern/ -j 4 (exited with code 4)
___________________________________ summary ____________________________________
ERROR: py39lint: commands failed
Error: Process completed with exit code 1.
It's just more warnings about logging and access to a protected member
tavern/_plugins/mqtt/client.py:209:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:210:23: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:217:12: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:218:27: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:221:12: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:222:27: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:228:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:229:41: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:209:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:210:23: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:217:12: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:218:27: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:221:12: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:222:27: W0212: Access to a protected member _client_id of a client class (protected-access)
tavern/_plugins/mqtt/client.py:228:8: W1203: Use lazy % formatting in logging functions (logging-fstring-interpolation)
tavern/_plugins/mqtt/client.py:229:41: W0212: Access to a protected member _client_id of a client class (protected-access)
If there is nothing my code caused in terms of tests, can this be merged?
To fix the lint errors, you need to change all the instances of
# pylint: disable=unused-argument
to
# pylint: disable=unused-argument,protected-access
And all the f-string usage like
logger.debug(
f"Client '{client._client_id.decode()}' successfully connected to the broker with result code '{paho.connack_string(rc)}'"
)
to
logger.debug(
"Client '%s' successfully connected to the broker with result code '%s'", client._client_id.decode(), paho.connack_string(rc))
)
To fix the lint errors, you need to change all the instances of
# pylint: disable=unused-argumentto
# pylint: disable=unused-argument,protected-accessAnd all the f-string usage like
logger.debug( f"Client '{client._client_id.decode()}' successfully connected to the broker with result code '{paho.connack_string(rc)}'" )to
logger.debug( "Client '%s' successfully connected to the broker with result code '%s'", client._client_id.decode(), paho.connack_string(rc)) )
Thanks, will do.
I've updated the PR, but Github is showing I have 20 commits and 24 files changed on here, but that's not true.

ryan@rfriedm-us-dl01:~/Development/RFRIEDM-Trimble/tavern$ git log -n 3
commit 7e516525ee4d3c17245d73d5aa37d3d3b5682de6 (HEAD -> feature/bind-all-mqtt-callbacks, origin/feature/bind-all-mqtt-callbacks)
Author: Ryan Friedman <[email protected]>
Date: Sat May 7 12:59:59 2022 -0600
Use lazy f-string formatting and allow private member access
commit a4cc0c03a2167da870cf44e64c8a6cf2348949f2
Author: Ryan Friedman <[email protected]>
Date: Sun Jan 9 20:44:21 2022 -0600
Bind all MQTT client callbacks (#753)
commit 6c45900e97b2fcd4aae27f5892535852943da5f3 (origin/master, origin/HEAD, master)
Author: Konstantinos Xynos <[email protected]>
Date: Mon May 2 17:20:44 2022 +0200
Updated documentation to show how to enable tavern-merge-ext-function-values (#767)
* Update basics.md
Updated basics.md to provide an example of how to include tavern-merge-ext-function-values in pytest.ini
```
That's probably from branching off master then putting in a PR against the feature-2.0 branch - you are picking commits up from master which aren't on that branch.
That's probably from branching off master then putting in a PR against the feature-2.0 branch - you are picking commits up from master which aren't on that branch.
Fixed, ready for final review
Rebased back on top of feature-2.0, no changes otherwise.
After last night's CI run, it appears to be failing on the python3.9 invocation of black.
py39black run-test: commands[0] | black -t py39 --check tavern
would reformat tavern/_plugins/mqtt/client.py
Oh no! 💥 💔 💥
1 file would be reformatted, 41 files would be left unchanged.
ERROR: InvocationError for command /home/runner/work/tavern/tavern/.tox/py39black/bin/black -t py39 --check tavern (exited with code 1)
However, my local workspace I am unable to reproduce the issue. This is affecting all my other PR's.
(.venv) rfriedm@rfriedm-us-ll6:~/Development/RFRIEDM-Trimble/tavern$ black -t py39 --check tavern
All done! ✨ 🍰 ✨
42 files would be left unchanged.
After last night's CI run, it appears to be failing on the python3.9 invocation of black.
I'll fix it after the merge, probably a Black update. I was looking at using a pyproject.toml instead to try and lock dependencies a bit better