Blocking SSLContext call when using azure-servicebus

[sveinse]

• Package Name: azure-servicebus
• Package Version: 7.11.1 and main
• Operating System: Linux
• Python Version: 3.11.5

Describe the bug
When using azure.servicebus.aio.ServiceBusClient() blocking calls to SSLContext .load_verify_locations() are made. Async libraries should not contain any blocking operations.

The call is made here:

azure-sdk-for-python/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/aio/_transport_async.py

Line 198 in 5ae05fc

context.load_verify_locations(ca_certs)

and here:

azure-sdk-for-python/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp/aio/_transport_async.py

Line 232 in 5ae05fc

ctx.load_verify_locations(cafile=certifi.where())

To Reproduce

from azure.servicebus.aio import ServiceBusClient
client = ServiceBusClient.from_connection_string(...)
async with client:
    receiver = client.get_subscription_receiver(,,,)
    async with receiver:
        ...   # ^^ Observe that load_verify_locations() have been called

Expected behavior
Async libraries should avoid making blocking calls. The blocking operations should be called using executors or threads.

Additional context
For reference, this issue was discovered here: custom-components/zaptec#116 . Home Assistant have a detector which warns about blocking operations.

[github-actions]

Thank you for your feedback. Tagging and routing to the team member best able to assist.

[kashifkhan]

Thank you for the feedback @sveinse . We will investigate and get back to you asap.

[github-actions]

Hi @sveinse. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

[kashifkhan]

@sveinse we have addressed the issue and the next version of servicebus and eventhub will have the fix.

[sveinse]

Hi @kashifkhan Thank you for looking at the issue.

Which of the two solutions will move forward? It seems this #37655 is the preferred solution.

Please be advised that moving the blocking call to the constructor does not resolve the issue. It is completely ordinary to dynamically create a ServiceBusClient while async is running. If that's the case, then the same issue of doing blocking calls within async will prevail.

/unresolve

[graingert]

I think it would be better to allow users to pass in a configured SSLContext then they can choose to build it in a thread if required

[kashifkhan]

@sveinse this change was inspired from sslproto in asyncio https://github.com/python/cpython/blob/7e7223e18f58ec48fb36a68fb75b5c5b7a45042a/Lib/asyncio/sslproto.py#L295.

Wouldn't that cause the same warning to bubble up as well?

[kashifkhan]

@graingert that particular feature would be part of the python amqp library and not the servicebus client.

[sveinse]

@kashifkhan I agree with @graingert, it would be better if the user may provide a SSLContext themselves.

@sveinse this change was inspired from sslproto in asyncio https://github.com/python/cpython/blob/7e7223e18f58ec48fb36a68fb75b5c5b7a45042a/Lib/asyncio/sslproto.py#L295.

Wouldn't that cause the same warning to bubble up as well?

It would, but this code allows for supplying a user-provided SSLContext. By having that capability, the user may setup a SSLContext with ThreadPoolExcutor themselves. This is not supported in the eventhub implementation, unfortunately. If that were the case, the use can opt-out of the blocking IO.

[sveinse]

Any updates on this issue?

[kashifkhan]

@sveinse we will allow folks to pass in their own ssl context so things will look similar to how asyncio handles it.

[sveinse]

@kashifkhan thank you. Is there a planned schedule for the fix? E.g. any estimate on when azure-servicebus will be released containing this fix? I'm trying to coordinate the release for other packages that depend on it. Thanks.

[swathipil]

Hi @sveinse - We've merged the fix to main and it will be going out in our upcoming release. We'll update this thread once it's released. Thanks!

[graingert]

The code was merged here 839584c

[sveinse]

..which is a part of PR #37702 . Thanks @swathipil, looking forward to the release (which allows a release in our end as well).

[swathipil]

Hi @sveinse - The fix has been released in azure-servicebus 7.13.0. Thanks.

[github-actions]

Hi @sveinse. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text "/unresolve" to remove the "issue-addressed" label and continue the conversation.

[sveinse]

I confirm that the the fix for this issue resolves the issue when providing a custom ssl_context. Thank you very much.

[graingert]

Nice, you could also experiment with https://truststore.readthedocs.io/en/latest/ instead of certifi

I confirm that the the fix for this issue resolves the issue when providing a custom ssl_context. Thank you very much.