Skip to content

fix(complete): pre-install Impact/Subpack/Inspire deps to avoid load race - #44

Merged
ductiletoaster merged 2 commits into
mainfrom
fix/impact-inspire-deps
May 27, 2026
Merged

fix(complete): pre-install Impact/Subpack/Inspire deps to avoid load race#44
ductiletoaster merged 2 commits into
mainfrom
fix/impact-inspire-deps

Conversation

@ductiletoaster

Copy link
Copy Markdown
Member

Summary

Bakes segment-anything, dill, and webcolors into the complete image so that ComfyUI-Impact-Pack, ComfyUI-Impact-Subpack, and ComfyUI-Inspire-Pack load successfully on a fresh container start.

Why

ComfyUI loads custom nodes synchronously during startup. ComfyUI-Manager installs missing pip dependencies after nodes are loaded. On every fresh pod boot in a K8s environment where /app/.venv is part of the container image (not on a PVC), this ordering produces a hard race:

### Loading: ComfyUI-Impact-Subpack (V1.3.5)
ModuleNotFoundError: No module named 'dill'

### Loading: ComfyUI-Impact-Pack (V8.28.3)
ModuleNotFoundError: No module named 'segment_anything'

### Loading: ComfyUI-Inspire-Pack (V1.23)
ModuleNotFoundError: No module named 'webcolors'

Manager installs each pkg successfully a few seconds later and prints After restarting ComfyUI, please refresh the browser., but the pod is already Ready and K8s never restarts it — so the nodes stay missing until manual intervention. Confirmed in a live cluster: after Manager finished, import segment_anything worked from a shell inside the pod; the dep files were present in /app/.venv/lib/python3.12/site-packages/. Only the load order is wrong.

Change

Three new lines under the existing pack-deps sections in services/comfy/complete/extra-requirements.txt:

# ComfyUI Impact Pack + Subpack dependencies
piexif>=1.1
ultralytics>=8.3
segment-anything>=1.0      # NEW — Impact Pack SAM integration
dill>=0.3                  # NEW — Impact Subpack ultralytics-detector serialization

# ComfyUI Inspire Pack dependencies
webcolors>=24.0            # NEW — Inspire Pack regional_nodes

ultralytics>=8.3 was already baked in; this just completes the set.

Test plan

  • CI builds the complete-cuda target without resolver errors
  • In-cluster: pull the new image SHA, update harmony's infrastructure/kubernetes/apps/comfyui/deployment.yaml, ArgoCD sync
  • On fresh pod boot, the three packs load without ModuleNotFoundError (check via kubectl logs -n comfyui $(kubectl get pods -n comfyui -l app.kubernetes.io/name=comfyui -o name) | grep -E '(Impact|Inspire)')
  • ComfyUI-Manager still re-validates them on startup but each install is a no-op (deps already present)

Out of scope

  • peft was also seen being installed by Manager (visible in startup logs from harmony cluster) — not added here since it's not in the three packs the user flagged. Worth a follow-up survey of every custom-node dep that Manager currently installs on each boot.
  • sam2 (the Meta SAM2 git repo, separate from segment-anything/SAM1) — Manager pulls it from git on each boot. Baking a git-pinned sam2 would need a pip install git+https://...@<sha> line; deferred.

🤖 Generated with Claude Code

ductiletoaster and others added 2 commits May 26, 2026 21:48
…race

ComfyUI loads custom nodes synchronously during startup, but
ComfyUI-Manager installs their pip dependencies *after* node loading.
On every fresh pod boot in K8s (where /app/.venv is ephemeral), this
race causes:

- comfyui-impact-pack       → ModuleNotFoundError: segment_anything
- comfyui-impact-subpack    → ModuleNotFoundError: dill
- comfyui-inspire-pack      → ModuleNotFoundError: webcolors

Manager later installs each successfully and prints "After restarting
ComfyUI, please refresh the browser." — but K8s never restarts the
pod, so the nodes stay broken until manual intervention.

Bakes the three missing deps into extra-requirements.txt so they're
present in /app/.venv before ComfyUI starts loading nodes. The pack
sections in the file are reorganised:

- "Impact Pack dependencies" → "Impact Pack + Subpack dependencies"
  (piexif, ultralytics, segment-anything, dill)
- New "Inspire Pack dependencies" group (webcolors)

ultralytics was already baked in; this just completes the set.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ductiletoaster
ductiletoaster merged commit a490935 into main May 27, 2026
2 checks passed
@ductiletoaster
ductiletoaster deleted the fix/impact-inspire-deps branch May 27, 2026 04:50
ductiletoaster added a commit that referenced this pull request Jun 12, 2026
…h loop (#48)

* fix(core): use --index-url for torch family + parameterise via TORCH_INDEX arg

ComfyUI's requirements.txt has bare `torch / torchaudio / torchvision`
lines with no version pins or wheel URLs. The core Dockerfile used
\`--extra-index-url https://download.pytorch.org/whl/cu128\` when installing
those, but \`--extra-index-url\` is **additive**: pip picks the highest
version found across PyPI + the extra index.

When PyPI ships a newer torch than the cu128 index (which happens
regularly — e.g. torch 2.12.0 landed on PyPI built for cu130 while the
cu128 index still topped out at 2.11.0+cu128), pip picks the PyPI build
for torch and the cu128 build for torchaudio (no newer torchaudio on
PyPI). Result: torch compiled against cu130 + torchaudio compiled
against cu128 → runtime crash on import:

    RuntimeError: Detected that PyTorch and TorchAudio were compiled
    with different CUDA versions. PyTorch has CUDA version 13.0 whereas
    TorchAudio has CUDA version 12.8.

The current production image (built before torch 2.12.0 hit PyPI) works
by luck; the next clean rebuild — with or without recent
extra-requirements changes — would have failed regardless.

## Change

Two-stage install in the core layer:

1. \`pip install --index-url https://download.pytorch.org/whl/${TORCH_INDEX} torch torchaudio torchvision\` — replaces PyPI for this command so all three are pulled from the same wheel index with a consistent CUDA ABI.
2. \`pip install -r /app/ComfyUI/requirements.txt\` then \`manager_requirements.txt\` — runs normally; torch family is already satisfied and pip skips re-resolution.

Also adds \`ARG TORCH_INDEX=cu128\` parameterised via docker-bake.hcl:
- \`core-cuda\` → \`TORCH_INDEX=cu128\`
- \`core-cpu\` → \`TORCH_INDEX=cpu\`

Adding a future cu130 variant becomes a single bake-target addition with
\`args = { TORCH_INDEX = "cu130" }\`, no Dockerfile changes needed.

## Related

Supersedes the closed #46 (which targeted the wrong layer with
--extra-index-url cu130 in \`complete\` — would have masked the symptom
while leaving the index-priority bug unfixed and also unnecessarily
locking the image to cu130).

Verified by local \`docker buildx bake complete-cuda\` rebuild: produces
matching cu128 wheels for all torch-family packages; ComfyUI imports
torch/torchaudio without the CUDA-mismatch error.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(complete): bake ComfyUI-Lora-Manager deps to prevent startup crash loop

lora-manager is runtime-installed onto the custom_nodes volume; after a
self-update it began importing natsort, which isn't in the image venv.
Every fresh container boot then dies on ModuleNotFoundError before the
server binds — observed as CrashLoopBackOff on harmony talos-2101.

Bake all of lora-manager's requirements that aren't already provided by
core/extra deps, same pattern as the Impact/Subpack/Inspire fix (#44).

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant