Preserve busy ComfyUI jobs and talking-head review lane - #491
Draft
stone84949 wants to merge 3 commits into
Draft
Conversation
ComfyUI serves HTTP from the same process that loads models. The Wan 2.2 i2v workflow swaps a ~29GB checkpoint pair per clip, blocking the event loop for tens of seconds while weights stream off disk. The client's socket timeouts were short enough (5s health, 10s history) that a healthy server mid-generation was reported unreachable, losing whole clips. - health/object_info/history timeouts raised to 30s/30s/60s: they exist to bound a hung server, not a working one. - poll() now tolerates transport errors and keeps polling until its own deadline, instead of letting one slow /history response abort a job that is still running. The deadline already bounds the wait; a single timed out request should not pre-empt it. - The timeout error now reports the last transport error when there was one, so "server was busy" is distinguishable from "job never finished". Verified against a live ComfyUI on :8188 plus fault injection: poll now recovers after repeated ReadTimeouts and still gives up at the deadline. tests/contracts/test_comfyui_tools.py: 67 passed. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
The stale OpenMontage checkout was removed after its unique lane files were copied here. Record those files in canonical Git history so cleanup does not leave them as an untracked single copy. Tested: .venv\Scripts\python.exe -m pytest tests/qa/test_09_talking_head_review_lane.py -q
Add contract coverage for transient ComfyUI transport failures, deadline-bounded polling, and HTTP status propagation; add a lane-level review export contract; and remove unrelated historical payload from the PR without rewriting prior commits. Tested: .venv\\Scripts\\python.exe -m pytest tests/contracts/test_comfyui_tools.py -q; .venv\\Scripts\\python.exe -m pytest tests/qa/test_09_talking_head_review_lane.py -q; .venv\\Scripts\\python.exe -m pytest tests/contracts/test_phase1_contracts.py -q
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This draft contains three scoped commits:
User impact
Large ComfyUI workflows can survive temporary unresponsive periods while models load, without exceeding the caller's requested wait budget. A real HTTP failure is surfaced immediately instead of being misreported as a busy server. The talking-head review path remains reproducible and now has a mocked lane-level regression contract.
Safety boundary
requests.Timeoutandrequests.ConnectionErrorare retried; HTTP errors and other request failures propagate..json, and an empty payload file were removed from the final PR diff.Validation