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.

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

Migrate to paho-mqtt 2.0 #286

Merged
merged 3 commits into from
Mar 28, 2024
Merged

Migrate to paho-mqtt 2.0 #286

merged 3 commits into from
Mar 28, 2024

Conversation

JonathanPlasse
Copy link
Collaborator

Copy link

codecov bot commented Mar 24, 2024

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 ⚠️
Additional details and impacted files
@@           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             

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@JonathanPlasse JonathanPlasse marked this pull request as ready for review March 24, 2024 07:41
@JonathanPlasse
Copy link
Collaborator Author

@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 8c72a1a, use this branch and pass the checks and tests.
The last commit refactor: use paho-mqtt v2.0.0 with type ignores 93014c1, 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.

Copy link
Owner

@empicano empicano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any comments, LGTM, good job 😎

@frederikaalund
Copy link
Collaborator

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.

@frederikaalund frederikaalund merged commit 8321217 into main Mar 28, 2024
13 checks passed
@JonathanPlasse JonathanPlasse deleted the paho-mqtt-2.0 branch March 28, 2024 08:27
@JonathanPlasse
Copy link
Collaborator Author

Thank you.

@JonathanPlasse
Copy link
Collaborator Author

Should we cut a release for this?

@frederikaalund
Copy link
Collaborator

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
Copy link
Collaborator Author

I can do this at noon.

empicano added a commit that referenced this pull request Jun 5, 2024
empicano added a commit that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make aiomqtt compatible with paho v2
3 participants