Repository navigation
Conversation
A replica that finished an interrupted turn keeps the task registered. Tapping that entry returned the old snapshot and never saw the turn running elsewhere. The local tap now stays for a turn this replica is executing.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/agent_execution/active_task.py | 95.02% | 94.84% | 🔴 -0.18% |
| src/a2a/server/cluster/database_event_stream.py | 96.94% | 92.86% | 🔴 -4.08% |
| src/a2a/server/request_handlers/default_request_handler_v2.py | 87.46% | 90.09% | 🟢 +2.62% |
| Total | 93.14% | 93.18% | 🟢 +0.04% |
Generated by coverage-comment.yml
| await self._maybe_cleanup() | ||
|
|
||
| def request_in_flight(self) -> bool: | ||
| """True while this replica is executing a turn for the task. |
There was a problem hiding this comment.
Please remove comment (or at least shorten it to max 1 informative short sentence)
Also, please considering setting _request_log as a property, so we don't need to create a separate method for it (in that case, comment shouldn't be necessary)
There was a problem hiding this comment.
Thanks for the review. request_in_flight is a property now, so the extra method and the docstring are gone. The handler note is one sentence. Commit a8e2b99.
There was a problem hiding this comment.
My bad - I shouldn't have asked you to drop the docstring entirely. The repo's Ruff config enforces docstring required on public methods, and since request_in_flight is public, the property still needs one
Could you add a single line back, f.e:
"""True while this replica is executing a turn for the task."""
Keeping it to one sentence addresses my original concern too. Sorry for the back-and-forth.
Once that's in, we're good to merge. :)
The review asked for one sentence on the handler and a property instead of a method.
Once it is a property, the review said the comment is not needed.
Fixes #1323.
A replica that finished an interrupted turn keeps the task in its registry.
SubscribeToTasktreated that entry as the live producer, yielded the cached snapshot, and closed. The turn running on another replica was on the shared stream.The local tap now stays only while this replica holds the request lock. A parked entry follows the snapshot already read from the store and tails the shared stream. A turn that is executing here is still tapped locally. Single-process mode, where there is no shared stream, is unchanged.
The caller is resubscribing to their own task. The store already has the current snapshot. The parked registry entry was the stale one.
Restoring
if local is not Nonemakestest_paused_replica_resubscribe_follows_the_shared_streamtime out on the parked tap. The in-flight test still receives the local task and leaves the shared stream untapped.Test plan
pytest tests/server/request_handlers/test_default_request_handler_v2.py -k 'paused_replica or in_flight_replica'pytest tests/server/request_handlers/test_default_request_handler_v2.py -k subscribe_to_task