Allow ssl.SSLContext instance to be supplied to connect and from_url …

[pwistrand]

Its impossible to use client side certificates with a TLS termination proxy to connect to rabbitmq without having control over the ssl.SSLContext. This change is a simple one that makes the from_url and connect methods behaviour like the loop.create_connection() they ultimately call. This will also make using self signed certificates since they don't have to be installed in an OS dependant manner but instead can just be referenced by the path in the supplied context. See #91

[skewty]

What happened with this? Is this issue actually still open? I see the pull request and all checks have passed with a few unit tests. Is something missing?

[dzen]

Hi, I'm very sorry for this late answer, but we need some more changes: as the ssl can be both a bool or a SSLContext, this must be clarifed and I don't like having multiple type for a single argument.

[pwistrand]

Hi @dzen , is it just the method documentation that needs to be changed or supporting either a boolean or SSLContext in the ssl keyword argument? If the later then I tend to agree with you but I have taken the same approach as the standard library (see https://docs.python.org/3/library/asyncio-eventloop.html#opening-network-connections.

[dzen]

Ping @RemiCardona for any opinion on this

[RemiCardona]

If anything, this PR doesn't go far enough. We should just get rid of the few lines where we try to create a context ourselves and set options on it. We should just have a single ssl argument and simply forward it to loop.create_connection() and point to the relevant CPython documentation on how to use it.

[pwistrand]

@dzen @RemiCardona removing bool support in the ssl parameter is going to break backwards compatibility which IMO is a big call and seems a step away from "batteries included". In the PR I tried to come up with what I felt was the least intrusive compromise but I'm happy to rework whatever way you decided.

[dzen]

@pwistrand : IMHO since we're still not in 1.0 version the API may change, and it already did.

Feel free to contribute as suggested :)

[notmeta]

Happy to help work on this as it's something I need too.

I agree with what @dzen said, it's only at 0.12 so breaking changes may occur if they're important enough.

[pwistrand]

@notmeta I'm overloaded at the moment so if you want to go ahead and make the changes go for it.

[notmeta]

@pwistrand mind giving me access to your forked repo? cheers

[pwistrand]

@notmeta invite sent

[dzen]

To be honest, the PR would be more readable if you rebase it from master

[notmeta]

@dzen

To be honest, the PR would be more readable if you rebase it from master

Done, I've cleaned it up - sorry about that, must have messed something up whilst merging originally

[dzen]

We will merge soon :)

[dzen]

Rebased & Merged

[adamhooper]

This is a killer feature. I'd love to use it in a library I maintain, channels_rabbitmq. Could we please have a new release? (Or if releasing is hard, could you please share an idea of when the next release will happen?)

[dzen]

Hello @adamhooper,

we've done a lots of changes on master last two past month. Did you tried your https://github.com/CJWorkbench/channels_rabbitmq with aioamqp master ? Did you find any bugs or so ?

[adamhooper]

@dzen At long last I've tested channel_rabbitmq with aioamqp master. The unit tests all pass, and it's given me immense pleasure to migrate our test suite to SSL. Is there anything else I can do to help with a release?

[notmeta]

Agreed, a release would be much appreciated; it's been a while. Means I can remove the monkey patching from my code to add proper SSL support 😄

[dzen]

Just released aioamqp 0.13.0.

Feel free to report new incorrect behaviour and such !

Thanks for your patience :pray: