search

Polyconseil / aioamqp

AMQP implementation using asyncio
Other
280 stars 88 forks source link

Add SSL support #42

Closed mwfrojdman closed 9 years ago

mwfrojdman commented 9 years ago

Here's a patch to add SSL support. There was already an open one, but it's not mergeable anymore and has some deficiencies: The default port for SSL connections is different than for unencrypted connections and it always uses the default SSL context.

I added the verify_ssl keyword argument, which is True by default. Setting it to False skips validating the server's certificate, which may be useful when developing.

I tested both SSL and non-SSL connections against RabbitMQ, and SSL connections with verify_ssl on and off (also with and without having the server's CA certificate available), and all combinations worked as expected.

Adding automated tests against RabbitMQ is possible and quite trivial, but doing it in a manner that works on many distributions and without leaving nasty surprises in the environment needs some investigation.

dzen commented 9 years ago

Note: I rather prefer to deal with rebased branches. We're really fond of clean history, which will be more easier to read later.

Thank you for all your contributions by the way !

mwfrojdman commented 9 years ago

Ah yeah sorry. I've had to use Stash lately which auto merges forks by default and assumed Github would do the same because Github is just better in most ways. Not sure if I'd need to create a new branch and pick the changes manually (whatever that means) from the merged branch. Rebase -i didn't work for the merged branch, so that's why it stayed as it is. One more thing to learn!