search

empicano / aiomqtt

The idiomatic asyncio MQTT client, wrapped around paho-mqtt
https://aiomqtt.felixboehm.dev/
BSD 3-Clause "New" or "Revised" License
408 stars 73 forks source link

Migrate to paho-mqtt 2.0 #286

Closed JonathanPlasse closed 5 months ago

JonathanPlasse commented 6 months ago
codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 89.47368% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 88.5%. Comparing base (f7697de) to head (93014c1).

Files Patch % Lines
aiomqtt/client.py 77.7% 0 Missing and 2 partials :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #286 +/- ## ======================================= + Coverage 88.4% 88.5% +0.1% ======================================= Files 6 6 Lines 432 438 +6 Branches 83 83 ======================================= + Hits 382 388 +6 Misses 29 29 Partials 21 21 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

JonathanPlasse commented 5 months ago

@empicano, @frederikaalund, this is ready for review. I fixed all MyPy errors and opened a pull request on paho.mqtt.python to upstream the type annotations fixes.

The commit fix: MyPy errors 8c72a1ae5cba421df4eda2507a0157c97dd32c0a, use this branch and pass the checks and tests. The last commit refactor: use paho-mqtt v2.0.0 with type ignores 93014c141ea6cd6c371b071fed3a527bb9cca312, use paho-mqtt v2.0.0 with type ignores.

Thus, we can merge this PR and close #279. And once paho-mqtt integrates eclipse/paho.mqtt.python#828, we can remove the type ignores. In the future, we could also migrate to the new callback API.

frederikaalund commented 5 months ago

Well done with this migration! 👍 I looked through the code and it looks good to me too. :) I like that you found some typing fixes for paho-mqtt as well.

JonathanPlasse commented 5 months ago

Thank you.

JonathanPlasse commented 5 months ago

Should we cut a release for this?

frederikaalund commented 5 months ago

Should we cut a release for this?

Yeah, that is a good idea. 👍 We have to mention that we no longer support paho-mqtt 1.x in that one as well.

I'm currently reviewing the #287, can you do a release? :)

JonathanPlasse commented 5 months ago

I can do this at noon.