search

eandersson / amqpstorm

Thread-safe Python RabbitMQ Client & Management library
MIT License
188 stars 36 forks source link

use port 5671 when use_ssl=True #119

Closed s-at-ik closed 2 years ago

s-at-ik commented 2 years ago

Hello,

As per the AMQP spec, connection should be established on port 5671 when using amqps. Therefore I propose this pull request to make 5671 the default port when using ssl.

Thank you for the great library and have a nice day.

eandersson commented 2 years ago

The CI failures are probably highlighting a bug that seems to be magically fixed with this PR. The test failing is probably not valid any more with this patch, but we would need to remove that test and replace it with two new tests checking for the correct default ports before we could merge this PR.

s-at-ik commented 2 years ago

The CI was correct and my code was bogous. Fixed it and added tests for default port.

eandersson commented 2 years ago

Awesome! Thanks. Btw one minor pep8 warning and then I can merge the fix ./amqpstorm/tests/unit/uri_connection/test_uri_connection.py:42:5: E303 too many blank lines (2)

s-at-ik commented 2 years ago

Awesome! Thanks. Btw one minor pep8 warning and then I can merge the fix ./amqpstorm/tests/unit/uri_connection/test_uri_connection.py:42:5: E303 too many blank lines (2)

my bad, fixed

eandersson commented 2 years ago

LGTM! Thanks

s-at-ik commented 2 years ago

wonderful, thank you :+1: