feat(py): add ergonomic route update iterator - #3229
Conversation
Co-Authored-By: GPT-5.6 <noreply@openai.com>
Co-Authored-By: GPT-5.6 <noreply@openai.com>
|
Warning Review limit reachedNext included review available in 37 minutes. View limit detailsLimit details: You’ve used all 4 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65201348cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """Async iterator of a broadcast's route changes. | ||
|
|
||
| Created by :meth:`BroadcastConsumer.route_updates`. The first item is the | ||
| current route, followed by each update. Iteration ends when the broadcast |
There was a problem hiding this comment.
When a producer calls set_route more than once before the consumer requests its next item, this iterator does not yield each update as documented. poll_route_changed in rs/moq-net/src/model/broadcast.rs only compares the saved epoch with the current epoch and returns the single current state.route, so intermediate routes are overwritten. Either buffer route events or document this watch as latest-value/coalescing; otherwise consumers relying on a complete route history silently miss changes.
AGENTS.md reference: AGENTS.md:L117-L117
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
1 participant
Closes #3193.
Summary
RouteWatch, an async iterator and async context manager for the current broadcast route and subsequent updates.route_changed()compatible by backing it with one cached watch cursor.anext()could leave that task holding the watch state forever, causing the next call to hang.Public API changes
BroadcastConsumer.route_updates() -> RouteWatch.RouteWatchwith async iteration, async context management, andcancel().route_changed()behavior remains available.The
moq-ffichanges are internal task-lifecycle fixes; no generated FFI, C, Swift, Kotlin, or Go surface changes are required.Test plan
nix develop --command just fixnix develop --command just checknix develop --command just testroute_changed()compatibility.moq-ffitests pass, including dropped-run state release and cancel-before-run terminal behavior.(Written by GPT-5.6)