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

Update to paho-mqtt 2.1.0 #405

Merged
merged 2 commits into from
Feb 4, 2025
Merged

Update to paho-mqtt 2.1.0 #405

merged 2 commits into from
Feb 4, 2025

Conversation

Tabisch
Copy link

@Tabisch Tabisch commented Jan 25, 2025

Hi,

i wanna help get this PR in home-assistant get done.
home-assistant/core#136130

As far as i understand the paho-mqtt documentation, bumping the version and adding CallbackAPIVersion.Version1, when creating the Client object should be enough.
https://eclipse.dev/paho/files/paho.mqtt.python/html/migrations.html

You might want to remove types-paho-mqtt, because it isnt needed anymore.
https://pypi.org/project/types-paho-mqtt/1.6.0.20240321/

I cant test this, because i dont own a roomba.

Thank you.

@jbouwh
Copy link

jbouwh commented Jan 25, 2025

It might be possible to allow the paho client to be >= 1.6.1 and <=3.00. This way it does not break fro users that still depend on the old client.

For example: Python-roborock/python-roborock#288

@Tabisch
Copy link
Author

Tabisch commented Jan 25, 2025

Good point.
Didn't think of that.

@jbouwh
Copy link

jbouwh commented Jan 28, 2025

@Tabisch It seems the CI test is failing because of the state of poetry.lock vs the changes made in pyproject.toml. Can you have a look at the failed CI test logs and try to fix this?

@Tabisch
Copy link
Author

Tabisch commented Jan 28, 2025

All the CI test logs told me to run poetry lock.
Did that. Hope that it goes through now.

@pschmitt
Copy link
Owner

CI passed.
Just one last thing: please sign your commits

@Tabisch
Copy link
Author

Tabisch commented Jan 28, 2025

Sorry that you have to rerun the workflow.
Thank you for linking the guide.

@pschmitt
Copy link
Owner

Erm why did you set the readme executable?

Copy link
Owner

@pschmitt pschmitt left a comment

Choose a reason for hiding this comment

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

Please squash (and sign) your commits

README.md Outdated
Copy link
Owner

Choose a reason for hiding this comment

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

Please revert this.

Copy link
Author

Choose a reason for hiding this comment

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

To be completely honest i did this, because git was telling me that i had nothing to commit.
I only do this stuff as a hobby and don't really know my way around git that well.
Gonna look into this.

Sorry that im making this such a hassle.

Copy link

Choose a reason for hiding this comment

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

May be change it back?

Copy link
Author

Choose a reason for hiding this comment

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

Wrote that while i was at work.
Just came back.

I guess i was more an obstacle that a help.

@Tabisch
Copy link
Author

Tabisch commented Jan 29, 2025

I hope this is correct now.
Made it harder for anyone than it needed to be.

@jbouwh
Copy link

jbouwh commented Feb 4, 2025

@pschmitt Can you have another look at this one. It is currently blocking home-assistant/core#136130

It would be nice to see the lib to be bumped to a version that included this changes.

@pschmitt
Copy link
Owner

pschmitt commented Feb 4, 2025

Sure

@pschmitt pschmitt merged commit 5be05e5 into pschmitt:main Feb 4, 2025
6 checks passed
@pschmitt
Copy link
Owner

pschmitt commented Feb 4, 2025

@jbouwh
Copy link

jbouwh commented Feb 4, 2025

Thanks!

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.

3 participants