Skip to content

chore: ban module-level imports of startup-heavy modules - #380

Draft
cloudsmith-iduffy wants to merge 2 commits into
lint/enforce-type-checking-importsfrom
lint/ban-heavy-module-level-imports
Draft

chore: ban module-level imports of startup-heavy modules#380
cloudsmith-iduffy wants to merge 2 commits into
lint/enforce-type-checking-importsfrom
lint/ban-heavy-module-level-imports

Conversation

@cloudsmith-iduffy

@cloudsmith-iduffy cloudsmith-iduffy commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Description

The perf work in this stack got mcp, httpx, cloudsmith_api, requests, rich, semver, urllib3 and keyring out of CLI startup. The startup tests guard the two main entry points, but someone could still add import requests to the top of any other eagerly-imported file and quietly hand every docker pull its ~30ms back.

This closes that gap with ruff's banned-module-level-import rule (TID253): the eight modules above can now only be imported inside functions. Files that are themselves loaded lazily — the command modules, the core/api/* call sites, the MCP server, and the session/rest/saml/download leaves — are allowlisted per file, so their existing top-level imports stay put. Anything new fails lint, in pre-commit and CI, via the existing ruff hook.

Turning the rule on immediately found two real problems:

  1. The PyInstaller entrypoint (packaging/pyinstaller/entry.py) imported keyring.backend at module level, but only the packaging selftest uses it — every invocation of the frozen binary paid for an import it didn't need. It now lives inside the selftest helper. (Frozen imports are pre-compiled, so this one is below measurement noise — the binary clocks 0.26s before and after. Fixed because the rule is right, not because it's measurable.)
  2. core/keyring.py imported the keyring library at module level, and that file is on the eager path (it's pulled in through the credential chain). My first instinct was to allowlist it; on review that was just papering over a real cost. The library is only used inside five functions, so the imports moved there — and cloudsmith --version dropped from 0.12s to 0.09s.

No flame graphs on this one — the only import-graph change is the small keyring subtree, and the numbers above tell the story. The real value is that the next accidental heavy import gets caught by the linter instead of a profiler.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Refactoring
  • Other (please describe)

Lint guard for startup performance, plus the two import deferrals it flagged.

Additional Notes

  • Known hole: the allowlist covers cli/commands/* as a directory, which also exempts the eagerly-imported main.py and registry.py. The subprocess tests in cli/tests/test_startup_imports.py cover exactly that hole.
  • The frozen binary was rebuilt and its packaging selftest passes (SELFTEST: OK, 129 modules).
  • Full suite passes (801 passed, 40 skipped).

🤖 Generated with Claude Code

@cloudsmith-iduffy cloudsmith-iduffy changed the title chore(no-ticket): ban module-level imports of startup-heavy modules chore: ban module-level imports of startup-heavy modules Aug 21, 2026
cloudsmith-iduffy and others added 2 commits August 22, 2026 00:40
Select TID253 with a ban list of the modules that dominated CLI
startup. Files that load lazily are allowlisted per file. Defer the
keyring import in the frozen entrypoint to the selftest that uses it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
core/keyring.py sits on the eager import path through the credential
chain. Import the keyring library inside the functions that use it and
remove the file from the TID253 allowlist.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cloudsmith-iduffy
cloudsmith-iduffy force-pushed the lint/ban-heavy-module-level-imports branch from 82a5203 to f7601a4 Compare August 21, 2026 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant