Skip to content

perf(file_utils): import httpx lazily (BE-10537) - #830

Merged
annehe9 merged 1 commit into
mainfrom
anne/be-10537-lazy-httpx
Aug 31, 2026
Merged

annehe9 merged 1 commit into
mainfrom
anne/be-10537-lazy-httpx

Conversation

@annehe9

@annehe9 annehe9 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Linear: BE-10537. Follow-up to BE-9907.

Problem

comfy_cli/file_utils.py imported httpx at module top, and two module-level constants built httpx objects at import time (_DOWNLOAD_TIMEOUT = httpx.Timeout(...) and the transient/retriable exception tuples). Only _download_file_httpx, download_file and _friendly_network_error need httpx. Everything else that imports this module wants atomic_write_text or atomic_write_bytes alone: knowledge.py, build.py, build_spec.py, outdated.py, project.py, workflow.py, cql/loader.py, jobs_state.py, skills/__init__.py.

Change

import httpx moved into those three functions. In download_file it sits after the aria2 early return, so the aria2 path never imports it.

The two constants became three functools.cache helpers built on first download: _download_timeout, _transient_exceptions, _retriable_exceptions. Same classes in the same order, and the cache means except _retriable_exceptions() as exc: compares against the same tuple object every time, so timeout and retry behaviour is unchanged. Nothing outside the module referenced the removed names.

atomic_write_text / atomic_write_bytes were not split into a new module, per the ticket.

Measurements

python -X importtime -c "import comfy_cli.knowledge" on a quiet machine, dependencies from uv sync --locked, min of 11, two alternating rounds against origin/main.

main branch
comfy_cli.knowledge 79.0 ms / 79.5 ms 70.0 ms / 70.9 ms
httpx line 11.3 ms absent

The ticket estimated 0.13 s. The real figure on a warm machine is 11 ms, because rich is already loaded by the time httpx pulls httpx._main.

Tests

uv run --locked --extra dev pytest, matching the CI workflow:

  • origin/main: 7281 passed, 39 skipped
  • this branch: 7282 passed, 39 skipped

The new test is test_knowledge_import_does_not_pull_httpx in tests/test_file_utils_network.py. It runs -X importtime in a subprocess and asserts httpx is absent from the import list. Verified it fails against origin/main.

Download behaviour is covered by the existing suites, which pass unchanged: tests/test_file_utils_network.py and tests/comfy_cli/command/test_model_download_background.py, 301 tests including the timeout and retry paths.

uvx ruff@0.15.15 check and format --diff both clean, matching .github/workflows/ruff_check.yml.

🤖 Generated with Claude Code

`file_utils` imported httpx at module top, and two module-level constants built
httpx objects at import time. Only `_download_file_httpx`, `download_file` and
`_friendly_network_error` need it; `knowledge.py`, `build.py`, `workflow.py`,
`skills` and the rest import this module for `atomic_write_text` /
`atomic_write_bytes` alone.

`import httpx` moved into those three functions, and the timeout and exception
tuples became `functools.cache` helpers built on first download. Timeout and
retry behaviour is unchanged.

`import comfy_cli.knowledge`: 79 ms -> 70 ms (min of 11, two alternating
rounds). httpx cost 11 ms there and no longer appears in `-X importtime`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Team

Run ID: cda7b028-dd2c-404d-81a4-4b12d2e9bff1

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8638f and 2fb6a05.

📒 Files selected for processing (2)
  • comfy_cli/file_utils.py
  • tests/test_file_utils_network.py

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

Changes

The change defers httpx loading in comfy_cli.file_utils until network operations use it. Cached helper functions provide the timeout and exception tuples. A subprocess test verifies that importing comfy_cli.knowledge does not load httpx.

Lazy httpx loading

Layer / File(s) Summary
Lazy httpx contracts
comfy_cli/file_utils.py
Replaces module-level httpx-dependent constants with cached helper functions.
Network call-site integration
comfy_cli/file_utils.py, tests/test_file_utils_network.py
Updates error handling and download calls to use local imports and cached helpers. Adds a regression test for comfy_cli.knowledge imports.

Suggested reviewers: mattmillerai

Merge Risk: ⚪ Minimal · up to 2fb6a

This change defers the httpx dependency until network functionality is used, reducing import-time overhead while preserving download behavior. No actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch anne/be-10537-lazy-httpx
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch anne/be-10537-lazy-httpx

Comment @coderabbitai help to get the list of available commands.

@skishore23 skishore23 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving. Verified the three things that matter for a lazy-import move: nothing outside file_utils.py references the removed names (_DOWNLOAD_TIMEOUT/_TRANSIENT_EXCEPTIONS/_RETRIABLE_EXCEPTIONS — the only grep hit is transfer.py's unrelated _DOWNLOAD_TIMEOUT_S); the functools.cache helpers return the same tuple object every call so except-clause behaviour is identical; and the aria2 early-return really does precede the import httpx in download_file. The -X importtime subprocess test pins the win. Ran test_file_utils_network.py + test_file_utils.py on the branch: 129 passed; CI fully green.

@annehe9
annehe9 merged commit d958159 into main Aug 31, 2026
18 checks passed
@annehe9
annehe9 deleted the anne/be-10537-lazy-httpx branch August 31, 2026 22:42
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 31, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants