Repository navigation
fix(compat): get a v0.3 push config by task id alone - #1380
mykytanetipa merged 3 commits into
Conversation
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/compat/v0_3/request_handler.py | 96.97% | 97.40% | 🟢 +0.43% |
| Total | 93.42% | 93.42% | 🟢 +0.01% |
Generated by coverage-comment.yml
mykytanetipa
left a comment
There was a problem hiding this comment.
This only fixes configs that were created without an id. v0.3 clients can also set their own config id, and then get by task id alone still fails:
set: {"taskId": "task-1", "pushNotificationConfig": {"id": "cfg-1", "url": "https://..."}}
get: {"id": "task-1"}
The get looks up config "task-1", finds nothing, and the v1.0 handler raises TaskNotFoundError even though the task exists. v0.3.26 returned the task's first config here.
When no id is given, list the task's configs instead. Prefer the one stored under the task id, otherwise take the first:
if not v10_req.id:
resp = await self.request_handler.on_list_task_push_notification_configs(
ListTaskPushNotificationConfigsRequest(task_id=v10_req.task_id),
context,
)
configs = list(resp.configs)
chosen = next(
(c for c in configs if c.id == v10_req.task_id),
configs[0] if configs else None,
)
if chosen is None:
raise TaskNotFoundError
return conversions.to_compat_task_push_notification_config(chosen)Please also add a test that goes through DefaultRequestHandlerV2 with InMemoryPushNotificationConfigStore, covering set with and without an id. The current test mocks the core handler, so it doesn't catch this.
|
Thanks, you're right. It now lists the task's configs when no id is given and prefers the one stored under the task id, otherwise the first. Swapped the mocked test for one through DefaultRequestHandlerV2 and InMemoryPushNotificationConfigStore that covers set with and without an id. |
Problem
A v0.3
pushNotificationConfig/getwith only a task id failed validation, because the v1.0 request had no config id.Fix
When no config id is given, list the task's configs and return the one stored under the task id, otherwise the first. This matches v0.3.
Tests
test_on_get_task_push_notification_config_by_task_idruns throughDefaultRequestHandlerV2andInMemoryPushNotificationConfigStore. It covers a set with and without an id and fails on main. 2329 passed.Fixes #1379