fix: make event queue sink removal idempotent#1134
Conversation
Use set.discard so repeated sink closure does not raise inside the traced remove_sink method and export a false ERROR span. Fixes a2aproject#1133 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42ca73c6-08fb-4954-96f4-b7e32e46613a
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the remove_sink method in EventQueueSource to use discard instead of remove, making the operation idempotent and allowing the removal of a contextlib.suppress(KeyError) block in the close method. A new test is added to verify that closing a sink multiple times does not record exceptions on the telemetry span. The review feedback suggests asserting that the tracer mock's start_as_current_span method was actually called to prevent a vacuous test pass if the mock is not active during execution.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/events/event_queue_v2.py | 91.19% | 91.15% | 🔴 -0.05% |
| Total | 92.95% | 92.95% | ⚪️ -0.00% |
Generated by coverage-comment.yml
Assert that span creation uses the patched tracer so the regression cannot pass silently if telemetry instrumentation changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 42ca73c6-08fb-4954-96f4-b7e32e46613a
1 participant
Description
Thank you for opening a Pull Request!
This pull request changed
EventQueueSource.remove_sink()to useset.discard(), so removing an already-absent sink completes without raisingKeyError.EventQueueSink.close()now delegates directly to that idempotentoperation instead of suppressing the exception after telemetry has recorded it.
A regression test closes the same sink twice and verifies that the mocked
OpenTelemetry span records neither an exception nor an error description.
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
CONTRIBUTINGGuide.fix:which represents bug fixes, and correlates to a SemVer patch.feat:represents a new feature, and correlates to a SemVer minor.feat!:, orfix!:,refactor!:, etc., which represent a breaking change (indicated by the!) and will result in a SemVer major.bash scripts/format.shfrom the repository root to format)Fixes #1133 🦕