Skip to content

refactor: remove dead branches in async _pump_iterator and drop put_terminal#34

Merged
mvallebr merged 1 commit into
mainfrom
refactor/max-in-flight-cleanup
Jun 18, 2026
Merged

refactor: remove dead branches in async _pump_iterator and drop put_terminal#34
mvallebr merged 1 commit into
mainfrom
refactor/max-in-flight-cleanup

Conversation

@mvallebr

@mvallebr mvallebr commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Cleanup follow-up to #33 (max_in_flight).

async _pump_iterator dead branches + dead parameters

Two isinstance(q, AsyncQueueBranch): await q.put(item) else: await q.put(item) checks for item enqueue where both arms were byte-for-byte identical — leftover from an incomplete refactor. Collapsed them into a single await q.put(item).

The materialize_before_enqueue and consumer_type parameters were also dead: every caller passed materialize_before_enqueue=False, so the True branch (which called _apply_materializer before re-iterating) was never executed. Removed both parameters and the dead branch, simplifying the pump to a single async for loop.

AsyncQueueBranch.put_terminal removal

put_terminal was functionally identical to put (same active-gated put_nowait retry loop), so it is removed and the except / finally blocks in _pump_iterator now call put for terminal markers too. AsyncQueueBranch keeps close() / active for backpressure / cancellation.

Per-test timeout

Added pytest-timeout as a dev dependency and timeout = 10 (thread method) in [tool.pytest.ini_options] so a hung streaming test fails loudly instead of stalling the whole suite. Baseline run is ~2.2s, so 10s gives ample headroom.

Scope intentionally left out — needs its own PR

SyncFanout._put_terminal (sync engine) keeps its busy-wait + get_nowait() drop on a full queue. Removing the drop without a full shutdown redesign of SyncFanout turns it into a hard hang: a terminal consumer whose output stays lazy never drains its SyncQueueIterator, the bounded queue fills up, and the pump blocks forever trying to deliver the EOF marker. The drop is the current workaround for that design gap and touching it belongs in a dedicated PR (shutdown semantics of SyncFanout, non-enqueueing abort, drain-on-cleanup).

Verification

  • uv run ruff format / ruff check --select F401: clean
  • uv run pytest tests/: 445 passed in 2.21s (baseline intact, timeout enabled)
  • Patch coverage: 80.0% (threshold: 80%) — the one uncovered line is the on_error == OnError.STOP branch inside the async pump, which is unreachable in the current engine: STOP streams are eagerly materialized in _publish_output before the pump is ever used.

@mvallebr mvallebr force-pushed the refactor/max-in-flight-cleanup branch from 2cd726d to bf0b602 Compare June 18, 2026 12:02
@mvallebr mvallebr changed the title refactor: remove dead branches in async _pump_iterator refactor: remove dead branches in async _pump_iterator and drop put_terminal Jun 18, 2026
…erminal

- Collapse the two isinstance(q, AsyncQueueBranch): await q.put(item)
  else: await q.put(item) blocks in async _pump_iterator (both arms
  were byte-for-byte identical) into a single await q.put(item), and
  unify the materialize_before_enqueue / plain paths under one async
  for loop.
- Remove AsyncQueueBranch.put_terminal: it was functionally identical
  to put (same active-gated put_nowait loop), so collapse callers in
  _pump_iterator to use put for terminal markers too.
- Add pytest-timeout dev dependency and a per-test timeout=10s in
  pyproject.toml so a hung streaming test fails loudly instead of
  stalling the suite.

The sync SyncFanout._put_terminal busy-wait + get_nowait() drop is
intentionally left untouched here: removing the drop without a full
shutdown redesign of SyncFanout (terminal consumer that never drains
its SyncQueueIterator deadlocks the pump) turns it into a hard hang.
That belongs in its own PR.
@mvallebr mvallebr force-pushed the refactor/max-in-flight-cleanup branch from bf0b602 to dc8bebf Compare June 18, 2026 12:08
@mvallebr mvallebr merged commit 9d9a325 into main Jun 18, 2026
4 checks passed
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