From ddd4d20f1b1c879dfdb75a04a32b5dfee6105528 Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Fri, 21 Aug 2026 23:52:56 +0100 Subject: [PATCH 1/2] perf(no-ticket): keep the SDK and requests out of startup Move create_requests_session to core/session.py, which reads the SDK Configuration defaults only when the SDK is already imported. Defer the SDK, session and saml imports in the decorators, the keyring provider and the OIDC detectors to the code paths that use them. Co-Authored-By: Claude Fable 5 --- cloudsmith_cli/cli/decorators.py | 10 +- .../cli/tests/test_startup_imports.py | 2 +- cloudsmith_cli/core/api/files.py | 2 +- cloudsmith_cli/core/credentials/models.py | 6 +- .../oidc/detectors/azure_devops.py | 5 +- .../oidc/detectors/github_actions.py | 5 +- .../core/credentials/oidc/exchange.py | 2 +- .../credentials/providers/keyring_provider.py | 6 +- cloudsmith_cli/core/download.py | 2 +- cloudsmith_cli/core/rest.py | 121 +-------------- cloudsmith_cli/core/session.py | 139 ++++++++++++++++++ cloudsmith_cli/core/tests/test_rest.py | 3 +- 12 files changed, 172 insertions(+), 131 deletions(-) create mode 100644 cloudsmith_cli/core/session.py diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 3eb1fe94..830f2847 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,10 @@ def initialise_session(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring + # The session module pulls in requests (~30ms). Import it here so + # that commands without a session skip that cost. + 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 +577,10 @@ def initialise_api(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring + # The cloudsmith_api SDK costs ~70ms to import. Import it here so + # that only the commands that call the API pay that cost. + 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..9870fe3a 100644 --- a/cloudsmith_cli/core/credentials/models.py +++ b/cloudsmith_cli/core/credentials/models.py @@ -3,9 +3,11 @@ from __future__ import annotations from dataclasses import dataclass -from typing import Literal +from typing import TYPE_CHECKING, Literal -import requests +# requests costs ~30ms to import and is only used in annotations here. +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..f86224d6 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,10 @@ def get_token(self) -> str: separator = "&" if "?" in request_uri else "?" url = f"{request_uri}{separator}api-version={API_VERSION}" + # The session module pulls in requests (~30ms). Import it here + # so that only a detector match pays that cost. + 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..32f72eb7 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,10 @@ def get_token(self) -> str: separator = "&" if "?" in request_url else "?" url = f"{request_url}{separator}audience={quote(audience, safe='')}" + # The session module pulls in requests (~30ms). Import it here + # so that only a detector match pays that cost. + 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..1f5157f4 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,11 @@ def resolve(self, context: CredentialContext) -> CredentialResult | None: "Session unavailable; skipping token refresh, using existing token" ) else: + # The saml module pulls in requests and the SDK + # (~100ms). Import it here so that only the + # token-refresh path pays that cost. + 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) From 8799332d35857c95e3464b1cc52aaf6ae87e8fdf Mon Sep 17 00:00:00 2001 From: Ian Duffy Date: Sat, 22 Aug 2026 00:03:52 +0100 Subject: [PATCH 2/2] chore(no-ticket): drop the import-cost comments Co-Authored-By: Claude Fable 5 --- cloudsmith_cli/cli/decorators.py | 4 ---- cloudsmith_cli/core/credentials/models.py | 1 - .../core/credentials/oidc/detectors/azure_devops.py | 2 -- .../core/credentials/oidc/detectors/github_actions.py | 2 -- cloudsmith_cli/core/credentials/providers/keyring_provider.py | 3 --- 5 files changed, 12 deletions(-) diff --git a/cloudsmith_cli/cli/decorators.py b/cloudsmith_cli/cli/decorators.py index 830f2847..908ee104 100644 --- a/cloudsmith_cli/cli/decorators.py +++ b/cloudsmith_cli/cli/decorators.py @@ -352,8 +352,6 @@ def initialise_session(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring - # The session module pulls in requests (~30ms). Import it here so - # that commands without a session skip that cost. from ..core.session import create_requests_session as _create_session opts = config.get_or_create_options(ctx) @@ -577,8 +575,6 @@ def initialise_api(f): @functools.wraps(f) def wrapper(ctx, *args, **kwargs): # pylint: disable=missing-docstring - # The cloudsmith_api SDK costs ~70ms to import. Import it here so - # that only the commands that call the API pay that cost. from ..core.api.init import initialise_api as _initialise_api opts = config.get_or_create_options(ctx) diff --git a/cloudsmith_cli/core/credentials/models.py b/cloudsmith_cli/core/credentials/models.py index 9870fe3a..a0ca2020 100644 --- a/cloudsmith_cli/core/credentials/models.py +++ b/cloudsmith_cli/core/credentials/models.py @@ -5,7 +5,6 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Literal -# requests costs ~30ms to import and is only used in annotations here. if TYPE_CHECKING: import requests diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py index f86224d6..2e24b7ba 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/azure_devops.py @@ -45,8 +45,6 @@ def get_token(self) -> str: separator = "&" if "?" in request_uri else "?" url = f"{request_uri}{separator}api-version={API_VERSION}" - # The session module pulls in requests (~30ms). Import it here - # so that only a detector match pays that cost. from ....session import create_requests_session as create_session session = self.context.session or create_session() diff --git a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py index 32f72eb7..c7985729 100644 --- a/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py +++ b/cloudsmith_cli/core/credentials/oidc/detectors/github_actions.py @@ -41,8 +41,6 @@ def get_token(self) -> str: separator = "&" if "?" in request_url else "?" url = f"{request_url}{separator}audience={quote(audience, safe='')}" - # The session module pulls in requests (~30ms). Import it here - # so that only a detector match pays that cost. from ....session import create_requests_session as create_session session = self.context.session or create_session() diff --git a/cloudsmith_cli/core/credentials/providers/keyring_provider.py b/cloudsmith_cli/core/credentials/providers/keyring_provider.py index 1f5157f4..143e631b 100644 --- a/cloudsmith_cli/core/credentials/providers/keyring_provider.py +++ b/cloudsmith_cli/core/credentials/providers/keyring_provider.py @@ -33,9 +33,6 @@ def resolve(self, context: CredentialContext) -> CredentialResult | None: "Session unavailable; skipping token refresh, using existing token" ) else: - # The saml module pulls in requests and the SDK - # (~100ms). Import it here so that only the - # token-refresh path pays that cost. from ....cli.saml import refresh_access_token refresh_token = keyring.get_refresh_token(api_host)