[PR #227] fix: close owned job event streams #227

New issue

Open

opened 2026-10-31 22:02:28 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 22:02:28 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfy-python-sdk/pull/227
Author: @allenflux
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: codex/fix-event-stream-ownership

📝 Commits (3)

2c75eca fix: close owned job event streams
e6e724c chore: resolve event-stream PR changelog conflict
b75126c fix: preserve iterator-only event transports

📊 Changes

4 files changed (+292 additions, -16 deletions)

View changed files

📝 CHANGELOG.md (+5 -0)
📝 src/comfy_sdk/jobs.py (+28 -16)
➕ tests/test_event_iterator_overrides.py (+144 -0)
➕ tests/test_event_stream_cleanup.py (+115 -0)

📄 Description

Exhausting a job event iterator at a terminal frame, or explicitly closing it, does not currently close the low-level SSE generator it owns. That generator holds the HTTP response context across yields, so resource release can depend on generator finalization.

Close each owned stream with closing / aclosing inside the existing per-connection error handling. Preserve polling fallback and reconnection behavior, and accurately annotate the low-level generator return types. This covers terminal exhaustion and explicit iterator closure; it does not promise automatic cleanup for an arbitrary bare break while the caller retains the iterator.

Validation on Python 3.12.11 / macOS arm64:

Four resource regressions fail on unchanged main, while seven controls pass with identical final tests. All eleven pass with the fix.
Tests retain the real low-level generator and observe the real HTTP response context using the existing local API stub; no httpx mocks.
Full suite: 1270 passed, 9 environment-gated gateway tests skipped. Ruff, formatting and mypy src pass. The public-repo hygiene check passes with the new test tracked.
Independent automated verification reproduces the four failures, passes the fixed eleven cases and five additional read-idle/reconnect controls.

No live gateway or GPU was used. The cancellation case is a general control, not proof of cancellation during an active SSE read.

Summary by CodeRabbit

Bug Fixes
- Synchronous and asynchronous event streams now close their underlying HTTP connections when iteration finishes, reaches a terminal status, or is closed early. This also applies when asynchronous iteration is cancelled while waiting for another event, helping prevent connections from remaining open after a stream ends.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfy-python-sdk/pull/227 **Author:** [@allenflux](https://github.com/allenflux) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/fix-event-stream-ownership` --- ### 📝 Commits (3) - [`2c75eca`](https://github.com/Comfy-Org/comfy-python-sdk/commit/2c75ecaf9bb89db26b42a0a6dcdc65c74a4aec9c) fix: close owned job event streams - [`e6e724c`](https://github.com/Comfy-Org/comfy-python-sdk/commit/e6e724c9369f940b1dc3ab94ac2e2910c63423cb) chore: resolve event-stream PR changelog conflict - [`b75126c`](https://github.com/Comfy-Org/comfy-python-sdk/commit/b75126cb07cff0feb61105a4bfc51249e274adeb) fix: preserve iterator-only event transports ### 📊 Changes **4 files changed** (+292 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+5 -0) 📝 `src/comfy_sdk/jobs.py` (+28 -16) ➕ `tests/test_event_iterator_overrides.py` (+144 -0) ➕ `tests/test_event_stream_cleanup.py` (+115 -0) </details> ### 📄 Description Exhausting a job event iterator at a terminal frame, or explicitly closing it, does not currently close the low-level SSE generator it owns. That generator holds the HTTP response context across yields, so resource release can depend on generator finalization. Close each owned stream with `closing` / `aclosing` inside the existing per-connection error handling. Preserve polling fallback and reconnection behavior, and accurately annotate the low-level generator return types. This covers terminal exhaustion and explicit iterator closure; it does not promise automatic cleanup for an arbitrary bare `break` while the caller retains the iterator. Validation on Python 3.12.11 / macOS arm64: - Four resource regressions fail on unchanged main, while seven controls pass with identical final tests. All eleven pass with the fix. - Tests retain the real low-level generator and observe the real HTTP response context using the existing local API stub; no httpx mocks. - Full suite: 1270 passed, 9 environment-gated gateway tests skipped. Ruff, formatting and `mypy src` pass. The public-repo hygiene check passes with the new test tracked. - Independent automated verification reproduces the four failures, passes the fixed eleven cases and five additional read-idle/reconnect controls. No live gateway or GPU was used. The cancellation case is a general control, not proof of cancellation during an active SSE read.   ## Summary by CodeRabbit * **Bug Fixes** * Synchronous and asynchronous event streams now close their underlying HTTP connections when iteration finishes, reaches a terminal status, or is closed early. This also applies when asynchronous iteration is cancelled while waiting for another event, helping prevent connections from remaining open after a stream ends.  --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi added the

pull-request

label

2026-10-31 22:02:28 +00:00