Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add is_async switch to new_client_from_enviroment #89

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

mjpieters
Copy link
Contributor

@mjpieters mjpieters commented Oct 6, 2023

This change makes it possible to use new_client_from_enviroment() and still
select between async and sync clients based on the current codebase.

The change includes the type hints needed for type checkers to know what kind
of object is returned in what circumstances; e.g.

# client: onepasswordconnectsdk.client.Client
client = new_client_from_environment(is_async=False)

# client: onepasswordconnectsdk.async_client.AsyncClient
async_client = new_client_from_environment(is_async=True)

This makes working with the library a lot easier in IDEs with type checker
support as they now know what exact methods a client supports.

This fixes #84

@@ -15,6 +15,7 @@ repository = "https://github.com/1Password/connect-sdk-python"
python = "^3.7"
python-dateutil = "^2.8.1"
httpx = "^0.23.3"
typing-extensions = "<4.8.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typing-extensions 1.4.8 drops support for Python 3.7, so the version is pinned.

@@ -3,6 +3,8 @@
from httpx import HTTPError
import json
import os
from typing import Optional, Union, overload
from typing_extensions import Literal
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typing.Literal was added in Python 3.8, which is why we need the backported version here.

If this project were to drop support for 3.7, the import could be updated.

@mjpieters
Copy link
Contributor Author

Happy to drop this PR if / when #92 is merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to enable / disable async to new_client_from_env()
1 participant