GoogleCloudPlatform / alloydb-python-connector

A Python library for connecting securely to your AlloyDB instances.
Apache License 2.0
28 stars 7 forks source link

Token refresh uses blocking I/O in async function #245

Open anuraaga opened 8 months ago

anuraaga commented 8 months ago

Bug Description

Token refresh in the async Client class uses the requests.Request transport of the google auth library

https://github.com/GoogleCloudPlatform/alloydb-python-connector/blob/6cc4c8e7bff360ce35525ae84d409ce1ebc284bf/google/cloud/alloydb/connector/client.py#L186-L188

This means it is doing blocking I/O on the asyncio loop. At the minimum, the fetch seems like it needs to be delegated to a thread with asyncio.to_thread.

For reference, google-auth-library-python does have some baking experimental APIs for async

https://github.com/googleapis/google-auth-library-python/blob/main/google/auth/transport/_aiohttp_requests.py https://github.com/googleapis/google-auth-library-python/blob/main/google/auth/_default_async.py

However, assuming using the experimental APIs was ok, it's probably still not ready for general use because at least, impersonated credentials don't yet have an async version (in certain token refresh logic I've written, I use the async APIs for normal creds and to_thread for impersonated, but it seems too complicated for this library).

https://github.com/googleapis/google-auth-library-python/blob/main/google/auth/impersonated_credentials.py

Example code (or command)

No response

Stacktrace

No response

Steps to reproduce?

  1. ?
  2. ?
  3. ? ...

Environment

  1. OS type and version:
  2. Python version:
  3. AlloyDB Python Connector version:

Additional Details

No response

jackwotherspoon commented 8 months ago

Hi @anuraaga thanks for raising an issue on the AlloyDB Python Connector 😄

I've been waiting for the google auth library to GA it's async interface for credentials but still not sure the timeline for when that will be. We want to keep this library stable and thus not rely on those APIs just yet.

We can probably look at writing our own version of to_thread as I believe the function was only added in Python 3.9 while we support Python 3.8+ with the Python Connector.

Will look at hopefully getting to a fix for this shortly, meanwhile we are always open to external contributions if you would like to take a pass at it, just let me know. 😄

Thanks again for raising this issue!

jackwotherspoon commented 6 months ago

Looks like there may be a way to do this by looking at credentials.token_state https://github.com/googleapis/google-auth-library-python/pull/1368