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

Bugfix/server connection release #114

Merged
merged 3 commits into from
Feb 11, 2023

Conversation

HerrMuellerluedenscheid
Copy link
Contributor

@HerrMuellerluedenscheid HerrMuellerluedenscheid commented Apr 28, 2022

Fixing #101

@HerrMuellerluedenscheid
Copy link
Contributor Author

I'm a little confused about the general exception handling in this block. I'm wondering if it wouldn't make more sense to change the base class to AMQTTException of NoDataException, MQTTException and CodecException here https://github.com/Yakifo/amqtt/blob/master/amqtt/errors.py
This would allow to catch them all at once. But I saw that the writer was not closed here and I'm wondering what the reason was and if the writer needs to be closed in the added exception handling of NoDataException as well. But that can be tested/treated in another MR.

@not-f-elsner
Copy link
Collaborator

LGTM

@HerrMuellerluedenscheid HerrMuellerluedenscheid merged commit 1a2812c into main Feb 11, 2023
@HerrMuellerluedenscheid HerrMuellerluedenscheid deleted the bugfix/server-connection-release branch February 11, 2023 14:55
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.

2 participants