Add option to enable / disable async to new_client_from_env()
mjpieters opened this issue ยท 4 comments
Summary
Allow the program to decide on async vs sync client use when loading the credentials from the environment.
Use cases
Whether or not an async client is needed is a choice for the specific application code, not the environment. Please allow me to load an async client in an async codebase, and a sync client in a sync codebase:
def sync_main():
client = new_client_from_env(is_async=False)
async def async_main():
client = new_client_from_env(is_async=True)
Proposed solution
Add a new is_async
keyword argument to new_client_from_env()
, defaulting to None
. If it is left to the default, the environment variable is used, otherwise the value is passed on directly to new_client()
:
from typing import Optional
# ...
def new_client_from_environment(
url: Optional[str] = None, token: Optional[str] = None, is_async: Optional[bool] = None
):
# ...
if is_async is None:
is_async = os.environ.get(ENV_IS_ASYNC_CLIENT).lower() in ("t", "true", "1")
# ...
return new_client(url, token, is_async=is_async)
(I made the test for ENV_IS_ASYNC_CLIENT
a little broader to also accept T
, true
, 1
, etc, as signalling 'true')
Is there a workaround to accomplish this today?
I have to options right now:
- set the
ENV_IS_ASYNC_CLIENT
environment variable manually before callingnew_client_from_environment()
. This feels very, very janky. I should not have to manipulate the environment variables visible to a 3rd-party SDK to switch between async modes for my code base. - use
new_client()
directly but then miss out on the helpful validation thatnew_client_from_environment()
also provides.
There is another reason why this change is beneficial: type checking.
Currently, a type checker can't know what kind of client is being returned from the new_client_from_environment()
call, as the object can be either a Client
or a AsyncClient
instance. With the right typing.overload
annotations, type checkers can explicitly be told what kind of client to expect. This makes working with this library much, much easier in supporting development environments.
Hi, @mjpieters! Thank you for raising this ๐.
That's definitely beneficial to implement. We'll add this to our to-do list.
Hi, @mjpieters! Thank you for raising this ๐. That's definitely beneficial to implement. We'll add this to our to-do list.
I had already created a PR for this but neglected to reference this issue in the PR. So, if you want, the todo item on your list can be marked as 'done' if you choose to merge that PR! ๐
That's great! I'll take a look ๐
Thank you very much for this and other PRs you opened! I'm going to review them all.