Repository navigation
Add option to enable / disable async to new_client_from_env() #84
Description
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_CLIENTenvironment 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.
Activity
mjpieters commentedon Oct 6, 2023
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 aClientor aAsyncClientinstance. With the righttyping.overloadannotations, 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.volodymyrZotov commentedon Oct 9, 2023
Hi, @mjpieters! Thank you for raising this 👍.
That's definitely beneficial to implement. We'll add this to our to-do list.
mjpieters commentedon Oct 9, 2023
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! 😄
volodymyrZotov commentedon Oct 9, 2023
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.