Skip to content

[Brook] Moved Streams to temporalio.common with the reviewed API. - #365

Closed
moetemp wants to merge 45 commits into
moe/brook/v4/p34-client-objectsfrom
moe/brook/v5/p35-reviewed-api
Closed

moetemp wants to merge 45 commits into
moe/brook/v4/p34-client-objectsfrom
moe/brook/v5/p35-reviewed-api

Conversation

@moetemp

@moetemp moetemp commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

This PR moves the Streams API into temporalio.common, with the API of the SDK review and the fixes of an independent review.

The SDK review asked for Streams as a first-class SDK API, with native as the default backend. It also asked for backends registered on the client, and an owner that closes each Stream. An earlier commit of this PR put the API in temporalio.common.streams. It's now in temporalio.common itself, with the same public names.

What changed?

Modules.

  • temporalio/common.py is now a package. The Stream code is in the private temporalio.common._streams. temporalio.common loads the Stream names on first use, so import temporalio.common loads no Stream code.
  • Redis is temporalio.contrib.stream (RedisStreamBackend, create(client, ...) and workflow.*). The memory backend is temporalio.contrib.stream.testing.MemoryStreamBackend. temporalio.contrib.streams is gone.

Registration and entry points.

  • Backends register with Client.connect(..., stream_backends=[...]). A Worker uses the backends of its client, and Replayer and WorkflowEnvironment take stream_backends=. With more than one backend, exactly one has default=True. A backend in plugins= is refused.
  • With no backend marked, the default name is native. Brook has no native backend (Jetty adds it), so a create with no backend raises StreamConfigError.
  • The entry points are workflow.create_stream, create_stream_nowait and stream_handle, Client.stream, and WorkflowHandle.stream. StreamClient is gone.
  • Reads take offset and topic_prefixes. Only a Stream subscribes, not a Topic.
  • WorkflowTopic.publish is a plain method. It starts the write when called and returns its asyncio.Future[int], so the write has its place in the Stream's order also with no await. await gives the offset or raises as before. A failed write that no caller awaits or checks fails the run at the run end.
  • The *_nowait docstrings say that a StreamError fails the run and a config error fails the Workflow Task. The behavior is the same as before.

Lifecycle.

  • No auto_close and no close_with_run. On Redis, a run end closes nothing.
  • A Workflow close is a regular Activity, so it commits with its Workflow Task. A Local Activity can't close a Workflow's Stream.
  • A block end closes the Stream at once only when the run can't get a retry run. Else the run end closes it, unless the run fails and the server starts a retry run. A cron run leaves the Stream open. When a cancel of the run ends the block, the Stream closes as CANCELED.
  • Plugin.on_workflow_evicted, workflow.Info.execution_expiration_time and the plugin shutdown wait of the Worker are removed.

Other review fixes.

  • Writes. An Activity retry continues with a closed or expired Stream argument, and the first write raises the real error. Pending Workflow appends are keyed by namespace, Stream and Producer, and are limited to 64 MiB. A refused append is dropped. A create of an expired Stream raises StreamExpiredError. A Record or batch over the Store limit raises the new StreamTooLargeError.
  • Reads. A no-wait read continues while an empty page advances next_offset. A Closed item's failure is decoded with the reader's converter. A Record that fails to decode raises StreamError with its offset, and next_offset is the offset after it. The reader stores no offsets of client Records.
  • API. async with on a client handle of a Workflow's Stream raises StreamNotOwnerError before the block runs. Client.stream refuses an empty id. End is public. A run adds the Stream key header to its Activities only when it uses Streams.
  • Converter. A transfer type dataclass in a top-level list is written as its fields.
  • Docs. Retention is a Record's age, also on an open Stream. The docstrings of Subscription.aclose, the errors and the eviction match the code.

Pins and CI.

  • Pins crate s23 ee6d1e51 and Core c8 f7826366, with regenerated protos and bindings.
  • The Stream tests decode the crate's vectors as UTF-8, for Windows. The backends take a client config with no interceptors. The generated Stream Nexus clients are hidden from the API docs. Upstream's pydantic 2.14 annotation (fix: annotate pydantic TypeAdapter cache for pydantic 2.14 temporalio/sdk-python#2011) is cherry-picked for test-latest-deps. Changelog fragments in added/ and changed/ describe the new API.

Known limits.

  • A Worker with a Redis backend polls Activities on its task queue for the close Activity. So every Worker on that queue must register the backend. A Worker with Streams and no_remote_activities=True logs a warning, since another Worker must run the close.
  • The run-end check doesn't see the execution timeout. If the chain's execution timeout stops a retry, the Stream stays open.

Why?

A close must not apply if its Workflow Task fails, so it runs as an Activity. The independent review of Brook and Jetty found that a Local Activity could still close early. It also found that a cancel closed a Stream as failed, that filtered reads could stall, and that an Activity retry failed on a closed Stream.

How did you test it?

Each review fix has a test that fails without it, apart from the docstring fixes. At the head, pytest tests/contrib/streams and the worker, replayer and testing suites pass on memory and Redis. New tests cover a publish with no await and a failed publish that no caller gets. A test checks that import temporalio.common loads no temporalio.common._streams in a fresh interpreter. The client, plugin, pydantic and converter suites pass. The upstream test_schedule_basics failed once on a shared dev server with schedules of other tests, and it passes alone. poe lint (ruff, pyright, mypy, basedpyright and pydocstyle) and poe gen-docs are clean.

On the fork CI, every job passes except check-protos and the arm64 jobs. check-protos waits on a nexgen release with temporalio/nexgen#260, which merged after v0.2.10. The arm64 jobs have no runner on the fork.

The superproject branch temporalio/internal-ai-prototypes 2026/10/ai198-streams pins the head, 415fb36a. sdk-python j9 and the harness use the new temporalio.common names.

  • Unit Tests
  • Staging
  • End to End Tests

moetemp and others added 30 commits October 24, 2026 09:23
The Streams API moves into temporalio.common.streams, so the module becomes a package with the same contents.
The first-class API lives next to the converter it registers with. The paths of the generators and the generated imports follow.
A Stream's lifecycle is its owner's to manage, so a run end closes nothing. drain() still lets unawaited writes finish before the run ends.
Only the run-end close on Redis used them, along with workflow.Info.execution_expiration_time.
Backends go in stream_backends= of Client.connect, a Worker takes its client's, and the Replayer and WorkflowEnvironment take them too. temporalio.workflow, Client.stream and WorkflowHandle.stream replace StreamClient, and Redis moves to temporalio.contrib.stream.
A read is stream-level only, so Topic objects lose subscribe and pass to the Stream's subscribe as topics. The proto field names stay.
The crate reserves the auto_close fields, so the pb2 and the system bindings lose them. The generators write into the new package.
s23 drops auto_close from the Stores and the protos, and c8 drops the cache-off eviction reason that only the run-end close used.
A Local Activity close outlived a Workflow Task that failed after it, so the retry's create failed the run. The block end now leaves the Stream open in a cron run and on a failure that a retry run follows.
A ClientStream kept a failed or cancelled Describe forever, and the handle map kept a failed one with its handle. A found id now stays as text, and a failed call goes.
A Replayer serves one backend as a Worker does, a Worker configures its backends after its plugins, and a generator of backends stays. A backend in plugins= fails before the connect, and the per-client calls are private.
A Worker serves one backend, so the backend-move recipe can't run. A continued run reads its Streams' backends after @workflow.init.
Redis keeps its regular Activity close, and a backend that closes another way overrides _close_in_workflow. Workers that share a task queue all register the Redis backend, since it polls Activities there.
A Local Activity writes before its Workflow Task commits, so a close there stayed when the task failed, and the task's retry could not create the Stream again.
A run's cancel reaches the block as the error of an Activity, a Local Activity or a child, and no retry run follows a cancelled run.
The block end decided from the exception and the retry policy alone, so it left Streams open when no retry run followed.
A Store scans a limited number of entries per call, so a filtered read ended early after 10,000 entries of other Topics.
The open of a new attempt withdraws earlier Records, and a Stream that takes no write failed the attempt for good before user code ran.
The read path converts Record bodies later, so the failure kept RawValue details and an encoded message.
The decode of a dataclass inside a JSON value reads its fields, so the encoder's transfer type broke the round trip.
An entry left only on success, a rerun under a new epoch added another, and the count bound let up to 2 GiB stay.
The close of a Workflow's Stream is a regular Activity, so such a Worker alone left every close waiting. Another Worker on the queue may run it, so it is a warning.
This was referenced Oct 25, 2026
@moetemp

moetemp commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Closed in favor of the review series. The net change is G1 to G3, B1 and S1 to S9, #367 to #379, cut fresh onto upstream main (plan: Brook: code review and upstream plan). The branch stays, since the superproject pins it and Jetty's #366 builds on it.

@moetemp moetemp closed this Oct 25, 2026
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.

2 participants