Skip to content

Commit

Permalink
Enforce more lazy imports of requests (#1072)
Browse files Browse the repository at this point in the history
* Enforce more lazy imports of `requests`

The benefits of this change are twofold:

- it ensures that the lazy import semantics provided by the SDK's
  top-level `__init__` are preserved when a greater variety of symbols
  are imported from it

- it pushes for a more uniform import style, in which the SDK itself
  demonstrates the best practice of importing the package and then
  accessing its members -- this avoids eagerly doing an import when a
  module is loaded and is especially valuable when modules only need
  an SDK symbol for type annotations

The change was made first by expanding and reorganizing the list of
modules which are tested for lazy imports, and then making any
necessary src/ changes to satisfy the test.

* Update src/globus_sdk/login_flows/login_flow_manager.py

Co-authored-by: Kurt McKee <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: Kurt McKee <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Oct 22, 2024
1 parent 48a8935 commit 9030598
Show file tree
Hide file tree
Showing 11 changed files with 72 additions and 67 deletions.
20 changes: 9 additions & 11 deletions src/globus_sdk/login_flows/command_line_login_flow_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,7 @@
import textwrap
import typing as t

from globus_sdk import (
AuthLoginClient,
ConfidentialAppAuthClient,
GlobusSDKUsageError,
OAuthTokenResponse,
)
import globus_sdk
from globus_sdk.gare import GlobusAuthorizationParameters
from globus_sdk.utils import get_nice_hostname

Expand Down Expand Up @@ -40,7 +35,7 @@ class CommandLineLoginFlowManager(LoginFlowManager):

def __init__(
self,
login_client: AuthLoginClient,
login_client: globus_sdk.AuthLoginClient,
*,
redirect_uri: str | None = None,
request_refresh_tokens: bool = False,
Expand All @@ -54,17 +49,20 @@ def __init__(

if redirect_uri is None:
# Confidential clients must always define their own custom redirect URI.
if isinstance(login_client, ConfidentialAppAuthClient):
if isinstance(login_client, globus_sdk.ConfidentialAppAuthClient):
msg = "Use of a Confidential client requires an explicit redirect_uri."
raise GlobusSDKUsageError(msg)
raise globus_sdk.GlobusSDKUsageError(msg)

# Native clients may infer the globus-provided helper page if omitted.
redirect_uri = login_client.base_url + "v2/web/auth-code"
self.redirect_uri = redirect_uri

@classmethod
def for_globus_app(
cls, app_name: str, login_client: AuthLoginClient, config: GlobusAppConfig
cls,
app_name: str,
login_client: globus_sdk.AuthLoginClient,
config: GlobusAppConfig,
) -> CommandLineLoginFlowManager:
"""
Create a ``CommandLineLoginFlowManager`` for use in a GlobusApp.
Expand All @@ -91,7 +89,7 @@ def for_globus_app(
def run_login_flow(
self,
auth_parameters: GlobusAuthorizationParameters,
) -> OAuthTokenResponse:
) -> globus_sdk.OAuthTokenResponse:
"""
Run an interactive login flow on the command line to get tokens for the user.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,14 @@
from string import Template
from urllib.parse import parse_qsl, urlparse

from . import html_files
from .errors import LocalServerLoginError

if sys.version_info >= (3, 9):
import importlib.resources as importlib_resources
else: # Python < 3.9
import importlib_resources

import globus_sdk.login_flows.local_server_login_flow_manager.html_files as html_files # noqa: E501

_IS_WINDOWS = os.name == "nt"

DEFAULT_HTML_TEMPLATE = Template(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,7 @@
from contextlib import contextmanager
from string import Template

from globus_sdk import (
AuthLoginClient,
GlobusSDKUsageError,
NativeAppAuthClient,
OAuthTokenResponse,
)
import globus_sdk
from globus_sdk.gare import GlobusAuthorizationParameters
from globus_sdk.login_flows.login_flow_manager import LoginFlowManager
from globus_sdk.utils import get_nice_hostname
Expand Down Expand Up @@ -97,7 +92,7 @@ class LocalServerLoginFlowManager(LoginFlowManager):

def __init__(
self,
login_client: AuthLoginClient,
login_client: globus_sdk.AuthLoginClient,
*,
request_refresh_tokens: bool = False,
native_prefill_named_grant: str | None = None,
Expand All @@ -114,7 +109,10 @@ def __init__(

@classmethod
def for_globus_app(
cls, app_name: str, login_client: AuthLoginClient, config: GlobusAppConfig
cls,
app_name: str,
login_client: globus_sdk.AuthLoginClient,
config: GlobusAppConfig,
) -> LocalServerLoginFlowManager:
"""
Create a ``LocalServerLoginFlowManager`` for use in a GlobusApp.
Expand All @@ -128,14 +126,14 @@ def for_globus_app(
# A "local server" relies on the user being redirected back to the server
# running on the local machine, so it can't use a custom redirect URI.
msg = "Cannot define a custom redirect_uri for LocalServerLoginFlowManager."
raise GlobusSDKUsageError(msg)
if not isinstance(login_client, NativeAppAuthClient):
raise globus_sdk.GlobusSDKUsageError(msg)
if not isinstance(login_client, globus_sdk.NativeAppAuthClient):
# Globus Auth has special provisions for native clients which allow implicit
# redirect url grant to localhost:<any-port>. This is required for the
# LocalServerLoginFlowManager to work and is not reproducible in
# confidential clients.
msg = "LocalServerLoginFlowManager is only supported for Native Apps."
raise GlobusSDKUsageError(msg)
raise globus_sdk.GlobusSDKUsageError(msg)

hostname = get_nice_hostname()
if hostname:
Expand All @@ -152,7 +150,7 @@ def for_globus_app(
def run_login_flow(
self,
auth_parameters: GlobusAuthorizationParameters,
) -> OAuthTokenResponse:
) -> globus_sdk.OAuthTokenResponse:
"""
Run an interactive login flow using a locally hosted server to get tokens
for the user.
Expand Down
23 changes: 9 additions & 14 deletions src/globus_sdk/login_flows/login_flow_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@

import abc

from globus_sdk import (
AuthLoginClient,
ConfidentialAppAuthClient,
GlobusSDKUsageError,
NativeAppAuthClient,
OAuthTokenResponse,
)
import globus_sdk
from globus_sdk.gare import GlobusAuthorizationParameters


Expand All @@ -30,15 +24,16 @@ class LoginFlowManager(metaclass=abc.ABCMeta):

def __init__(
self,
login_client: AuthLoginClient,
login_client: globus_sdk.AuthLoginClient,
*,
request_refresh_tokens: bool = False,
native_prefill_named_grant: str | None = None,
) -> None:
if not isinstance(login_client, NativeAppAuthClient) and not isinstance(
login_client, ConfidentialAppAuthClient
if not isinstance(
login_client,
(globus_sdk.NativeAppAuthClient, globus_sdk.ConfidentialAppAuthClient),
):
raise GlobusSDKUsageError(
raise globus_sdk.GlobusSDKUsageError(
f"{type(self).__name__} requires a NativeAppAuthClient or "
f"ConfidentialAppAuthClient, but got a {type(login_client).__name__}."
)
Expand Down Expand Up @@ -76,14 +71,14 @@ def _oauth2_start_flow(
requested_scopes = auth_parameters.required_scopes
# Native and Confidential App clients have different signatures for this method,
# so they must be type checked & called independently.
if isinstance(login_client, NativeAppAuthClient):
if isinstance(login_client, globus_sdk.NativeAppAuthClient):
login_client.oauth2_start_flow(
requested_scopes,
redirect_uri=redirect_uri,
refresh_tokens=self.request_refresh_tokens,
prefill_named_grant=self.native_prefill_named_grant,
)
elif isinstance(login_client, ConfidentialAppAuthClient):
elif isinstance(login_client, globus_sdk.ConfidentialAppAuthClient):
login_client.oauth2_start_flow(
redirect_uri,
requested_scopes,
Expand All @@ -94,7 +89,7 @@ def _oauth2_start_flow(
def run_login_flow(
self,
auth_parameters: GlobusAuthorizationParameters,
) -> OAuthTokenResponse:
) -> globus_sdk.OAuthTokenResponse:
"""
Run a login flow to get tokens for a user.
Expand Down
4 changes: 2 additions & 2 deletions src/globus_sdk/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@
import logging
import typing as t

from requests import Response

from globus_sdk import _guards

log = logging.getLogger(__name__)

if t.TYPE_CHECKING:
from requests import Response

import globus_sdk

__all__ = (
Expand Down
6 changes: 3 additions & 3 deletions src/globus_sdk/tokenstorage/v1/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
import os
import typing as t

from globus_sdk.services.auth import OAuthTokenResponse
import globus_sdk


class StorageAdapter(metaclass=abc.ABCMeta):
@abc.abstractmethod
def store(self, token_response: OAuthTokenResponse) -> None:
def store(self, token_response: globus_sdk.OAuthTokenResponse) -> None:
"""
Store an `OAuthTokenResponse` in the underlying storage for this adapter.
Expand All @@ -30,7 +30,7 @@ def get_token_data(self, resource_server: str) -> dict[str, t.Any] | None:
token data to retriever from storage
"""

def on_refresh(self, token_response: OAuthTokenResponse) -> None:
def on_refresh(self, token_response: globus_sdk.OAuthTokenResponse) -> None:
"""
By default, the on_refresh handler for a token storage adapter simply
stores the token response.
Expand Down
7 changes: 4 additions & 3 deletions src/globus_sdk/tokenstorage/v1/file_adapters.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@
import pathlib
import typing as t

from globus_sdk.services.auth import OAuthTokenResponse
from globus_sdk.tokenstorage.v1.base import FileAdapter
import globus_sdk
from globus_sdk.version import __version__

from .base import FileAdapter

# use the non-annotation form of TypedDict to apply a non-identifier key
_JSONFileData_0 = t.TypedDict("_JSONFileData_0", {"globus-sdk.version": str})

Expand Down Expand Up @@ -103,7 +104,7 @@ def _load(self) -> _JSONFileData:
}
return self._handle_formats(data)

def store(self, token_response: OAuthTokenResponse) -> None:
def store(self, token_response: globus_sdk.OAuthTokenResponse) -> None:
"""
By default, ``self.on_refresh`` is just an alias for this function.
Expand Down
7 changes: 4 additions & 3 deletions src/globus_sdk/tokenstorage/v1/memory_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@

import typing as t

from globus_sdk.services.auth import OAuthTokenResponse
from globus_sdk.tokenstorage.v1.base import StorageAdapter
import globus_sdk

from .base import StorageAdapter


class MemoryAdapter(StorageAdapter):
Expand All @@ -16,7 +17,7 @@ class MemoryAdapter(StorageAdapter):
def __init__(self) -> None:
self._tokens: dict[str, dict[str, t.Any]] = {}

def store(self, token_response: OAuthTokenResponse) -> None:
def store(self, token_response: globus_sdk.OAuthTokenResponse) -> None:
self._tokens.update(token_response.by_resource_server)

def get_token_data(self, resource_server: str) -> dict[str, t.Any] | None:
Expand Down
7 changes: 4 additions & 3 deletions src/globus_sdk/tokenstorage/v1/sqlite_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
import sqlite3
import typing as t

from globus_sdk.services.auth import OAuthTokenResponse
from globus_sdk.tokenstorage.v1.base import FileAdapter
import globus_sdk
from globus_sdk.version import __version__

from .base import FileAdapter


class SQLiteAdapter(FileAdapter):
"""
Expand Down Expand Up @@ -164,7 +165,7 @@ def remove_config(self, config_name: str) -> bool:
self._connection.commit()
return rowcount != 0

def store(self, token_response: OAuthTokenResponse) -> None:
def store(self, token_response: globus_sdk.OAuthTokenResponse) -> None:
"""
:param token_response: a globus_sdk.OAuthTokenResponse object containing token
data to store
Expand Down
17 changes: 10 additions & 7 deletions src/globus_sdk/tokenstorage/v2/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
import sys
import typing as t

from globus_sdk import GlobusSDKUsageError
import globus_sdk
from globus_sdk._types import UUIDLike
from globus_sdk.services.auth import OAuthDependentTokenResponse, OAuthTokenResponse

from .token_data import TokenStorageData

Expand Down Expand Up @@ -68,7 +67,9 @@ def remove_token_data(self, resource_server: str) -> bool:
:returns: True if token data was deleted, False if none was found to delete.
"""

def store_token_response(self, token_response: OAuthTokenResponse) -> None:
def store_token_response(
self, token_response: globus_sdk.OAuthTokenResponse
) -> None:
"""
Store token data from an :class:`OAuthTokenResponse` in the current namespace.
Expand All @@ -90,7 +91,9 @@ def store_token_response(self, token_response: OAuthTokenResponse) -> None:
)
self.store_token_data_by_resource_server(token_data_by_resource_server)

def _extract_identity_id(self, token_response: OAuthTokenResponse) -> str | None:
def _extract_identity_id(
self, token_response: globus_sdk.OAuthTokenResponse
) -> str | None:
"""
Get identity_id from id_token if available.
Expand All @@ -105,7 +108,7 @@ def _extract_identity_id(self, token_response: OAuthTokenResponse) -> str | None
"""
# dependent token responses cannot contain an `id_token` field, as the
# top-level data is an array
if isinstance(token_response, OAuthDependentTokenResponse):
if isinstance(token_response, globus_sdk.OAuthDependentTokenResponse):
return None

if token_response.get("id_token"):
Expand Down Expand Up @@ -268,10 +271,10 @@ def _slugify_app_name(app_name: str) -> str:
f'App name results in a reserved filename ("{app_name}"). '
"Please choose a different name."
)
raise GlobusSDKUsageError(msg)
raise globus_sdk.GlobusSDKUsageError(msg)

if not app_name:
msg = "App name results in the empty string. Please choose a different name."
raise GlobusSDKUsageError(msg)
raise globus_sdk.GlobusSDKUsageError(msg)

return app_name
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,26 @@
@pytest.mark.parametrize(
"module_name",
(
# experimental modules
"experimental",
"experimental.scope_parser",
# parts which are expected to be standalone
"scopes",
"gare",
# most of the SDK should not pull in 'requests', making the parts which do
# not handle request sending easy to use without the perf penalty from
# requests/urllib3
"authorizers",
"config",
"gare",
"local_endpoint",
"login_flows",
"paging",
"response",
"scopes",
"tokenstorage",
# the top-level of the 'exc' subpackage (but not necessarily its contents)
# should similarly be standalone, for exception handlers
"exc",
# internal bits and bobs
# internal components and utilities are a special case:
# failing to ensure that these avoid 'requests' can make it more difficult
# to ensure that the main parts (above) do not transitively pick it up
"_guards",
"_serializable",
"_types",
"utils",
"version",
Expand Down

0 comments on commit 9030598

Please sign in to comment.