Skip to content

Switch from hbmqtt to amqtt, websockets 9#79

Merged
decompil3d merged 2 commits intomasterfrom
hbmqtt-amqtt
Jan 25, 2022
Merged

Switch from hbmqtt to amqtt, websockets 9#79
decompil3d merged 2 commits intomasterfrom
hbmqtt-amqtt

Conversation

@decompil3d
Copy link
Collaborator

Resolves #78

I have not verified this change, as I do not use the MQTT aspects of this package. So I'm opening this as a draft PR until someone else can verify that the changes work.

@decompil3d decompil3d self-assigned this Jan 13, 2022
@decompil3d
Copy link
Collaborator Author

@fabaff, @cdce8p: Can y'all please verify these changes?

Copy link

@cdce8p cdce8p left a comment

Choose a reason for hiding this comment

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

I would also suggest adding python_requires=">=3.7" to setup.py. That is a requirement of amqtt anyway. That just makes it more visible.

amqtt - pyproject.toml

requirements.txt Outdated
certifi
hbmqtt
websockets<=8.1
amqtt
Copy link

Choose a reason for hiding this comment

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

Suggested change
amqtt
amqtt>=0.10.0,<0.11.0

I would suggest to require at least 0.10.0. Looking at the changelog, this seems to be a good starting point. The upper bound is just in case any more additional breaking changes are introduced.

requirements.txt Outdated
hbmqtt
websockets<=8.1
amqtt
websockets<10.0
Copy link

Choose a reason for hiding this comment

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

Is there a reason to limit it to <10.0? At least amqtt sets >=9.0.
If you would like to prevent issues in the future, may I suggest <11.0 instead? There are already libs in Home Assistant that require > 10.0.

Of course that depends on compatibility foremost.
https://websockets.readthedocs.io/en/stable/project/changelog.html#id2

packages=["volvooncall"],
install_requires=list(open("requirements.txt").read().strip().split("\n")),
extras_require={"console": ["docopt"]},
python_requires=">=3.8"
Copy link

Choose a reason for hiding this comment

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

Suggested change
python_requires=">=3.8"
python_requires=">=3.7"

3.7 should work too :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The package already only supports 3.8+ per the readme and CI workflow.

Copy link

Choose a reason for hiding this comment

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

Didn't saw that 🙈

@decompil3d decompil3d marked this pull request as ready for review January 13, 2022 19:39
@decompil3d decompil3d merged commit 97c662f into master Jan 25, 2022
@decompil3d decompil3d deleted the hbmqtt-amqtt branch January 25, 2022 18:11
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.

Migrate away from hbmqtt

2 participants