diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 3eb1fe94..908ee104 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -9,14 +9,12 @@ from cloudsmith_cli.cli import validators -from ..core.api.init import initialise_api as _initialise_api from ..core.credentials.chain import CredentialProviderChain from ..core.credentials.models import CredentialContext from ..core.credentials.oidc.detectors import ( disabled_detectors_from_env, registered_detectors, ) -from ..core.rest import create_requests_session as _create_session from . import config, utils @@ -354,6 +352,8 @@ def initialise_session(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring + from ..core.session import create_requests_session as _create_session + opts = config.get_or_create_options(ctx) host_suffixes = _parse_suffixes(kwargs.pop("allowed_api_host_suffixes")) proxy_suffixes = _parse_suffixes(kwargs.pop("allowed_api_proxy_suffixes")) @@ -575,6 +575,8 @@ def initialise_api(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring + from ..core.api.init import initialise_api as _initialise_api + opts = config.get_or_create_options(ctx) opts.rate_limit = _pop_boolean_flag(kwargs, "without_rate_limit", invert=True) opts.rate_limit_warning = kwargs.pop("rate_limit_warning") diff --git a/cloudsmith_cli/cli/tests/test_startup_imports.py b/cloudsmith_cli/cli/tests/test_startup_imports.py index 0e117b4e..9cd997ea 100644 --- a/cloudsmith_cli/cli/tests/test_startup_imports.py +++ b/cloudsmith_cli/cli/tests/test_startup_imports.py @@ -9,7 +9,7 @@ import subprocess import sys -HEAVY_PREFIXES = ("mcp", "httpx") +HEAVY_PREFIXES = ("mcp", "httpx", "cloudsmith_api", "requests") def modules_loaded_by_cli_import(): diff --git a/cloudsmith_cli/core/api/files.py b/cloudsmith_cli/core/api/files.py index 6c40ba5f..7bac880b 100644 --- a/cloudsmith_cli/core/api/files.py +++ b/cloudsmith_cli/core/api/files.py @@ -8,7 +8,7 @@ from requests_toolbelt import MultipartEncoder, MultipartEncoderMonitor from .. import ratelimits -from ..rest import create_requests_session +from ..session import create_requests_session from ..utils import calculate_file_md5 from .exceptions import ApiException, catch_raise_api_exception from .init import get_api_client diff --git a/cloudsmith_cli/core/credentials/models.py b/cloudsmith_cli/core/credentials/models.py index f50744e7..a0ca2020 100644 --- a/cloudsmith_cli/core/credentials/models.py +++ b/cloudsmith_cli/core/credentials/models.py @@ -3,9 +3,10 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Literal +from typing import TYPE_CHECKING, Literal -import requests +if TYPE_CHECKING: + import requests @dataclass diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py index eba5ce19..2e24b7ba 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py @@ -20,7 +20,6 @@ import os -from ....rest import create_requests_session as create_session from .base import EnvironmentDetector API_VERSION = "7.1" @@ -46,6 +45,8 @@ def get_token(self) -> str: separator = "&" if "?" in request_uri else "?" url = f"{request_uri}{separator}api-version={API_VERSION}" + from ....session import create_requests_session as create_session + session = self.context.session or create_session() try: response = session.post( diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py index c2985d6c..c7985729 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py @@ -15,7 +15,6 @@ import os from urllib.parse import quote -from ....rest import create_requests_session as create_session from .base import EnvironmentDetector DEFAULT_AUDIENCE = "cloudsmith" @@ -42,6 +41,8 @@ def get_token(self) -> str: separator = "&" if "?" in request_url else "?" url = f"{request_url}{separator}audience={quote(audience, safe='')}" + from ....session import create_requests_session as create_session + session = self.context.session or create_session() try: response = session.get( diff --git a/cloudsmith_cli/core/credentials/oidc/exchange.py b/cloudsmith_cli/core/credentials/oidc/exchange.py index 05d8c514..671d86d9 100644 --- a/cloudsmith_cli/core/credentials/oidc/exchange.py +++ b/cloudsmith_cli/core/credentials/oidc/exchange.py @@ -14,7 +14,7 @@ import requests -from ...rest import create_requests_session as create_session +from ...session import create_requests_session as create_session if TYPE_CHECKING: from ... import CredentialContext diff --git a/cloudsmith_cli/core/credentials/providers/keyring_provider.py b/cloudsmith_cli/core/credentials/providers/keyring_provider.py index 814a8e4b..143e631b 100644 --- a/cloudsmith_cli/core/credentials/providers/keyring_provider.py +++ b/cloudsmith_cli/core/credentials/providers/keyring_provider.py @@ -4,7 +4,6 @@ import logging -from ....cli.saml import refresh_access_token from ....core import keyring from ..models import CredentialContext, CredentialResult from ..provider import CredentialProvider @@ -34,6 +33,8 @@ def resolve(self, context: CredentialContext) -> CredentialResult | None: "Session unavailable; skipping token refresh, using existing token" ) else: + from ....cli.saml import refresh_access_token + refresh_token = keyring.get_refresh_token(api_host) new_access_token, new_refresh_token = refresh_access_token( api_host, diff --git a/cloudsmith_cli/core/download.py b/cloudsmith_cli/core/download.py index d561d438..5bc49f54 100644 --- a/cloudsmith_cli/core/download.py +++ b/cloudsmith_cli/core/download.py @@ -12,7 +12,7 @@ from . import ratelimits, utils from .api.exceptions import catch_raise_api_exception from .api.packages import get_packages_api, list_packages -from .rest import create_requests_session +from .session import create_requests_session def resolve_auth( diff --git a/cloudsmith_cli/core/rest.py b/cloudsmith_cli/core/rest.py index fb1afe7a..4b6f6219 100644 --- a/cloudsmith_cli/core/rest.py +++ b/cloudsmith_cli/core/rest.py @@ -4,132 +4,15 @@ import json import logging import re -import time from urllib.parse import urlencode import requests import requests.exceptions -from cloudsmith_api.configuration import Configuration from cloudsmith_api.rest import ApiException, RESTClientObject -from requests.adapters import HTTPAdapter -from urllib3.util.retry import Retry -logger = logging.getLogger(__name__) - - -class RetryWithCallback(Retry): - """A urllib3 Retry with a callback on retries.""" +from .session import create_requests_session - def __init__(self, *args, **kwargs): - self.error_retry_cb = kwargs.pop("error_retry_cb", None) - super().__init__(*args, **kwargs) - - def new(self, **kw): - kw["error_retry_cb"] = self.error_retry_cb - return super().new(**kw) - - def sleep_for_retry(self, response=None): - retry_after = self.get_retry_after(response) - if retry_after: - self._sleep_with_callback(retry_after, context="retry-after") - return True - - return False - - def _sleep_backoff(self): - backoff = self.get_backoff_time() - if backoff <= 0: - return - self._sleep_with_callback(backoff, context="backoff") - - def _sleep_with_callback(self, seconds, context=None): - """Sleep, but generate a callback before it.""" - if self.error_retry_cb and callable(self.error_retry_cb): - self.error_retry_cb(seconds, context=context) - return time.sleep(seconds) - - -def create_requests_session( - retries=None, - backoff_factor=None, - status_forcelist=None, - pools_size=4, - maxsize=4, - ssl_verify=None, - ssl_cert=None, - proxy=None, - session=None, - error_retry_cb=None, - respect_retry_after_header=True, - user_agent=None, - headers=None, -): - """Create a requests session that retries some errors.""" - # pylint: disable=too-many-branches - config = Configuration() - - if retries is None: - retry_max = getattr(config, "error_retry_max", None) - retries = retry_max if retry_max is not None else 5 - - if backoff_factor is None: - retry_backoff = getattr(config, "error_retry_backoff", None) - backoff_factor = retry_backoff if retry_backoff is not None else 0.23 - - if status_forcelist is None: - retry_codes = getattr(config, "error_retry_codes", None) - status_forcelist = ( - retry_codes if retry_codes is not None else [500, 502, 503, 504] - ) - - if ssl_verify is None: - ssl_verify = config.verify_ssl - - if ssl_cert is None: - if config.cert_file and config.key_file: - ssl_cert = (config.cert_file, config.key_file) - elif config.cert_file: - ssl_cert = config.cert_file - - if proxy is None: - proxy = Configuration().proxy - - session = session or requests.Session() - session.verify = ssl_verify - session.cert = ssl_cert - - if proxy: - session.proxies = {"http": proxy, "https": proxy} - - retry = RetryWithCallback( - backoff_factor=backoff_factor, - connect=retries, - allowed_methods=False, - read=retries, - status_forcelist=tuple(status_forcelist), - status=retries, - total=retries, - error_retry_cb=error_retry_cb, - respect_retry_after_header=respect_retry_after_header, - ) - - adapter = HTTPAdapter( - max_retries=retry, - pool_connections=pools_size, - pool_maxsize=maxsize, - pool_block=True, - ) - - session.mount("http://", adapter) - session.mount("https://", adapter) - - if user_agent: - session.headers["User-Agent"] = user_agent - - if headers: - session.headers.update(headers) - - return session +logger = logging.getLogger(__name__) class RestResponse(io.IOBase): diff --git a/cloudsmith_cli/core/session.py b/cloudsmith_cli/core/session.py new file mode 100644 index 00000000..750a032f --- /dev/null +++ b/cloudsmith_cli/core/session.py @@ -0,0 +1,139 @@ +"""HTTP session creation with retry support.""" + +import sys +import time + +import requests +from requests.adapters import HTTPAdapter +from urllib3.util.retry import Retry + + +class RetryWithCallback(Retry): + """A urllib3 Retry with a callback on retries.""" + + def __init__(self, *args, **kwargs): + self.error_retry_cb = kwargs.pop("error_retry_cb", None) + super().__init__(*args, **kwargs) + + def new(self, **kw): + kw["error_retry_cb"] = self.error_retry_cb + return super().new(**kw) + + def sleep_for_retry(self, response=None): + retry_after = self.get_retry_after(response) + if retry_after: + self._sleep_with_callback(retry_after, context="retry-after") + return True + + return False + + def _sleep_backoff(self): + backoff = self.get_backoff_time() + if backoff <= 0: + return + self._sleep_with_callback(backoff, context="backoff") + + def _sleep_with_callback(self, seconds, context=None): + """Sleep, but generate a callback before it.""" + if self.error_retry_cb and callable(self.error_retry_cb): + self.error_retry_cb(seconds, context=context) + return time.sleep(seconds) + + +def _sdk_configuration(): + """Return the SDK configuration defaults, or None when the SDK is not loaded. + + initialise_api() stores the CLI retry/SSL/proxy settings on + cloudsmith_api.Configuration via set_default(). When the SDK is not in + sys.modules, set_default() cannot have run, so the defaults equal the + fallback values in create_requests_session(). Skipping the import in + that case keeps the SDK out of commands that never call the API. + """ + if "cloudsmith_api" not in sys.modules: + return None + from cloudsmith_api.configuration import Configuration + + return Configuration() + + +def create_requests_session( + retries=None, + backoff_factor=None, + status_forcelist=None, + pools_size=4, + maxsize=4, + ssl_verify=None, + ssl_cert=None, + proxy=None, + session=None, + error_retry_cb=None, + respect_retry_after_header=True, + user_agent=None, + headers=None, +): + """Create a requests session that retries some errors.""" + # pylint: disable=too-many-branches + config = _sdk_configuration() + + if retries is None: + retry_max = getattr(config, "error_retry_max", None) + retries = retry_max if retry_max is not None else 5 + + if backoff_factor is None: + retry_backoff = getattr(config, "error_retry_backoff", None) + backoff_factor = retry_backoff if retry_backoff is not None else 0.23 + + if status_forcelist is None: + retry_codes = getattr(config, "error_retry_codes", None) + status_forcelist = ( + retry_codes if retry_codes is not None else [500, 502, 503, 504] + ) + + if ssl_verify is None: + ssl_verify = config.verify_ssl if config is not None else True + + if ssl_cert is None and config is not None: + if config.cert_file and config.key_file: + ssl_cert = (config.cert_file, config.key_file) + elif config.cert_file: + ssl_cert = config.cert_file + + if proxy is None and config is not None: + proxy = config.proxy + + session = session or requests.Session() + session.verify = ssl_verify + session.cert = ssl_cert + + if proxy: + session.proxies = {"http": proxy, "https": proxy} + + retry = RetryWithCallback( + backoff_factor=backoff_factor, + connect=retries, + allowed_methods=False, + read=retries, + status_forcelist=tuple(status_forcelist), + status=retries, + total=retries, + error_retry_cb=error_retry_cb, + respect_retry_after_header=respect_retry_after_header, + ) + + adapter = HTTPAdapter( + max_retries=retry, + pool_connections=pools_size, + pool_maxsize=maxsize, + pool_block=True, + ) + + session.mount("http://", adapter) + session.mount("https://", adapter) + + if user_agent: + session.headers["User-Agent"] = user_agent + + if headers: + session.headers.update(headers) + + return session diff --git a/cloudsmith_cli/core/tests/test_rest.py b/cloudsmith_cli/core/tests/test_rest.py index d2a6e6d8..732286be 100644 --- a/cloudsmith_cli/core/tests/test_rest.py +++ b/cloudsmith_cli/core/tests/test_rest.py @@ -2,7 +2,8 @@ import pytest from ..api.init import initialise_api -from ..rest import RestClient, create_requests_session +from ..rest import RestClient +from ..session import create_requests_session @pytest.fixture(autouse=True)