refactor: address review comments

- Switch pyright config to use 'exclude' instead of 'ignore' for types.

- Update REST transport to explicitly construct resource URLs, accommodating bare ID inputs.

- Update tests to use bare IDs instead of resource names, aligning with 'Short ID' convention.

- Ensure parts.py uses correct typing.

Signed-off-by: Luca Muscariello <muscariello@ieee.org>

Author: muscariello

pyrightconfig.json

@@ -5,11 +5,10 @@
         "src"
     ],
     "exclude": [
-        "**/__pycache__"
-    ],
-    "ignore": [
+        "**/__pycache__",
         "src/a2a/types"
     ],
+    "ignore": [],
     "reportMissingImports": false,
     "reportMissingModuleSource": false
 }

src/a2a/client/transports/rest.py

@@ -259,7 +259,7 @@ async def cancel_task(
             context,
         )
         response_data = await self._send_post_request(
-            f'/v1/{request.id}:cancel', payload, modified_kwargs
+            f'/v1/tasks/{request.id}:cancel', payload, modified_kwargs
         )
         response: Task = ParseDict(response_data, Task())
         return response
@@ -281,7 +281,7 @@ async def set_task_callback(
             payload, modified_kwargs, context
         )
         response_data = await self._send_post_request(
-            f'/v1/{request.task_id}/pushNotificationConfigs',
+            f'/v1/tasks/{request.task_id}/pushNotificationConfigs',
             payload,
             modified_kwargs,
         )
@@ -313,7 +313,7 @@ async def get_task_callback(
         if 'task_id' in params:
             del params['task_id']
         response_data = await self._send_get_request(
-            f'/v1/{request.task_id}/pushNotificationConfigs/{request.id}',
+            f'/v1/tasks/{request.task_id}/pushNotificationConfigs/{request.id}',
             params,
             modified_kwargs,
         )

tests/client/transports/test_grpc_client.py

@@ -306,13 +306,13 @@ async def test_get_task(
 ) -> None:
     """Test retrieving a task."""
     mock_grpc_stub.GetTask.return_value = sample_task
-    params = GetTaskRequest(id=f'tasks/{sample_task.id}')
+    params = GetTaskRequest(id=f'{sample_task.id}')
 
     response = await grpc_transport.get_task(params)
 
     mock_grpc_stub.GetTask.assert_awaited_once_with(
         a2a_pb2.GetTaskRequest(
-            id=f'tasks/{sample_task.id}', history_length=None
+            id=f'{sample_task.id}', history_length=None
         ),
         metadata=[
             (
@@ -332,14 +332,14 @@ async def test_get_task_with_history(
     mock_grpc_stub.GetTask.return_value = sample_task
     history_len = 10
     params = GetTaskRequest(
-        id=f'tasks/{sample_task.id}', history_length=history_len
+        id=f'{sample_task.id}', history_length=history_len
     )
 
     await grpc_transport.get_task(params)
 
     mock_grpc_stub.GetTask.assert_awaited_once_with(
         a2a_pb2.GetTaskRequest(
-            id=f'tasks/{sample_task.id}', history_length=history_len
+            id=f'{sample_task.id}', history_length=history_len
         ),
         metadata=[
             (
@@ -364,11 +364,11 @@ async def test_cancel_task(
     extensions = [
         'https://example.com/test-ext/v3',
     ]
-    request = a2a_pb2.CancelTaskRequest(id=f'tasks/{sample_task.id}')
+    request = a2a_pb2.CancelTaskRequest(id=f'{sample_task.id}')
     response = await grpc_transport.cancel_task(request, extensions=extensions)
 
     mock_grpc_stub.CancelTask.assert_awaited_once_with(
-        a2a_pb2.CancelTaskRequest(id=f'tasks/{sample_task.id}'),
+        a2a_pb2.CancelTaskRequest(id=f'{sample_task.id}'),
         metadata=[(HTTP_EXTENSION_HEADER, 'https://example.com/test-ext/v3')],
     )
     assert response.status.state == TaskState.TASK_STATE_CANCELED
@@ -387,7 +387,7 @@ async def test_set_task_callback_with_valid_task(
 
     # Create the request object expected by the transport
     request = CreateTaskPushNotificationConfigRequest(
-        task_id='tasks/task-1',
+        task_id='task-1',
         config_id=sample_task_push_notification_config.push_notification_config.id,
         config=sample_task_push_notification_config.push_notification_config,
     )
@@ -415,22 +415,22 @@ async def test_set_task_callback_with_invalid_task(
     # Return a config with an invalid name format
     mock_grpc_stub.CreateTaskPushNotificationConfig.return_value = (
         a2a_pb2.TaskPushNotificationConfig(
-            task_id='invalid-path-to-tasks/task-1',
+            task_id='invalid-path-to-task-1',
             id='config-1',
             push_notification_config=sample_push_notification_config,
         )
     )
 
     request = CreateTaskPushNotificationConfigRequest(
-        task_id='tasks/task-1',
+        task_id='task-1',
         config_id='config-1',
         config=sample_push_notification_config,
     )
 
     # Note: The transport doesn't validate the response name format
     # It just returns the response from the stub
     response = await grpc_transport.set_task_callback(request)
-    assert response.task_id == 'invalid-path-to-tasks/task-1'
+    assert response.task_id == 'invalid-path-to-task-1'
 
 
 @pytest.mark.asyncio
@@ -447,14 +447,14 @@ async def test_get_task_callback_with_valid_task(
 
     response = await grpc_transport.get_task_callback(
         GetTaskPushNotificationConfigRequest(
-            task_id='tasks/task-1',
+            task_id='task-1',
             id=config_id,
         )
     )
 
     mock_grpc_stub.GetTaskPushNotificationConfig.assert_awaited_once_with(
         a2a_pb2.GetTaskPushNotificationConfigRequest(
-            task_id='tasks/task-1',
+            task_id='task-1',
             id=config_id,
         ),
         metadata=[
@@ -476,20 +476,20 @@ async def test_get_task_callback_with_invalid_task(
     """Test retrieving a task push notification config with an invalid task name."""
     mock_grpc_stub.GetTaskPushNotificationConfig.return_value = (
         a2a_pb2.TaskPushNotificationConfig(
-            task_id='invalid-path-to-tasks/task-1',
+            task_id='invalid-path-to-task-1',
             id='config-1',
             push_notification_config=sample_push_notification_config,
         )
     )
 
     response = await grpc_transport.get_task_callback(
         GetTaskPushNotificationConfigRequest(
-            task_id='tasks/task-1',
+            task_id='task-1',
             id='config-1',
         )
     )
     # The transport doesn't validate the response name format
-    assert response.task_id == 'invalid-path-to-tasks/task-1'
+    assert response.task_id == 'invalid-path-to-task-1'
 
 
 @pytest.mark.parametrize(

tests/client/transports/test_jsonrpc_client.py

@@ -276,7 +276,7 @@ async def test_get_task_success(self, transport, mock_httpx_client):
         mock_httpx_client.post.return_value = mock_response
 
         # Proto uses 'name' field for task identifier in request
-        request = GetTaskRequest(id=f'tasks/{task_id}')
+        request = GetTaskRequest(id=f'{task_id}')
         response = await transport.get_task(request)
 
         assert isinstance(response, Task)
@@ -303,7 +303,7 @@ async def test_get_task_with_history(self, transport, mock_httpx_client):
         mock_response.raise_for_status = MagicMock()
         mock_httpx_client.post.return_value = mock_response
 
-        request = GetTaskRequest(id=f'tasks/{task_id}', history_length=10)
+        request = GetTaskRequest(id=f'{task_id}', history_length=10)
         response = await transport.get_task(request)
 
         assert isinstance(response, Task)
@@ -332,7 +332,7 @@ async def test_cancel_task_success(self, transport, mock_httpx_client):
         mock_response.raise_for_status = MagicMock()
         mock_httpx_client.post.return_value = mock_response
 
-        request = CancelTaskRequest(id=f'tasks/{task_id}')
+        request = CancelTaskRequest(id=f'{task_id}')
         response = await transport.cancel_task(request)
 
         assert isinstance(response, Task)
@@ -356,15 +356,15 @@ async def test_get_task_callback_success(
             'jsonrpc': '2.0',
             'id': '1',
             'result': {
-                'task_id': f'tasks/{task_id}',
+                'task_id': f'{task_id}',
                 'id': 'config-1',
             },
         }
         mock_response.raise_for_status = MagicMock()
         mock_httpx_client.post.return_value = mock_response
 
         request = GetTaskPushNotificationConfigRequest(
-            task_id=f'tasks/{task_id}',
+            task_id=f'{task_id}',
             id='config-1',
         )
         response = await transport.get_task_callback(request)

tests/e2e/push_notifications/notifications_app.py

@@ -57,7 +57,7 @@ async def add_notification(request: Request):
             'status': 'received',
         }
 
-    @app.get('/tasks/{task_id}/notifications')
+    @app.get('/{task_id}/notifications')
     async def list_notifications_by_task(
         task_id: Annotated[
             str, Path(title='The ID of the task to list the notifications for.')

tests/e2e/push_notifications/test_default_push_notification_support.py

@@ -138,7 +138,7 @@ async def test_notification_triggering_with_in_message_config_e2e(
     # Verify a single notification was sent.
     notifications = await wait_for_n_notifications(
         http_client,
-        f'{notifications_server}/tasks/{task.id}/notifications',
+        f'{notifications_server}/{task.id}/notifications',
         n=1,
     )
     assert notifications[0].token == token
@@ -182,7 +182,7 @@ async def test_notification_triggering_after_config_change_e2e(
 
     # Verify that no notification has been sent yet.
     response = await http_client.get(
-        f'{notifications_server}/tasks/{task.id}/notifications'
+        f'{notifications_server}/{task.id}/notifications'
     )
     assert response.status_code == 200
     assert len(response.json().get('notifications', [])) == 0
@@ -191,7 +191,7 @@ async def test_notification_triggering_after_config_change_e2e(
     token = uuid.uuid4().hex
     await a2a_client.set_task_callback(
         CreateTaskPushNotificationConfigRequest(
-            task_id=f'tasks/{task.id}',
+            task_id=f'{task.id}',
             config_id='after-config-change',
             config=PushNotificationConfig(
                 id='after-config-change',
@@ -218,7 +218,7 @@ async def test_notification_triggering_after_config_change_e2e(
     # Verify that the push notification was sent.
     notifications = await wait_for_n_notifications(
         http_client,
-        f'{notifications_server}/tasks/{task.id}/notifications',
+        f'{notifications_server}/{task.id}/notifications',
         n=1,
     )
     # Notification.task is a dict from proto serialization

tests/integration/test_client_server_integration.py

@@ -440,7 +440,7 @@ def channel_factory(address: str) -> Channel:
     transport = GrpcTransport(channel=channel, agent_card=agent_card)
 
     # Use GetTaskRequest with name (AIP resource format)
-    params = GetTaskRequest(id=f'tasks/{GET_TASK_RESPONSE.id}')
+    params = GetTaskRequest(id=f'{GET_TASK_RESPONSE.id}')
     result = await transport.get_task(request=params)
 
     assert result.id == GET_TASK_RESPONSE.id
@@ -467,7 +467,7 @@ async def test_http_transport_cancel_task(
     handler = transport_setup.handler
 
     # Use CancelTaskRequest with name (AIP resource format)
-    params = CancelTaskRequest(id=f'tasks/{CANCEL_TASK_RESPONSE.id}')
+    params = CancelTaskRequest(id=f'{CANCEL_TASK_RESPONSE.id}')
     result = await transport.cancel_task(request=params)
 
     assert result.id == CANCEL_TASK_RESPONSE.id
@@ -491,7 +491,7 @@ def channel_factory(address: str) -> Channel:
     transport = GrpcTransport(channel=channel, agent_card=agent_card)
 
     # Use CancelTaskRequest with name (AIP resource format)
-    params = CancelTaskRequest(id=f'tasks/{CANCEL_TASK_RESPONSE.id}')
+    params = CancelTaskRequest(id=f'{CANCEL_TASK_RESPONSE.id}')
     result = await transport.cancel_task(request=params)
 
     assert result.id == CANCEL_TASK_RESPONSE.id
@@ -519,7 +519,7 @@ async def test_http_transport_set_task_callback(
 
     # Create CreateTaskPushNotificationConfigRequest with required fields
     params = CreateTaskPushNotificationConfigRequest(
-        task_id='tasks/task-callback-123',
+        task_id='task-callback-123',
         config_id='pnc-abc',
         config=CALLBACK_CONFIG.push_notification_config,
     )
@@ -556,7 +556,7 @@ def channel_factory(address: str) -> Channel:
 
     # Create CreateTaskPushNotificationConfigRequest with required fields
     params = CreateTaskPushNotificationConfigRequest(
-        task_id='tasks/task-callback-123',
+        task_id='task-callback-123',
         config_id='pnc-abc',
         config=CALLBACK_CONFIG.push_notification_config,
     )
@@ -596,7 +596,7 @@ async def test_http_transport_get_task_callback(
 
     # Use GetTaskPushNotificationConfigRequest with name field (resource name)
     params = GetTaskPushNotificationConfigRequest(
-        task_id=f'tasks/{CALLBACK_CONFIG.task_id}', id=CALLBACK_CONFIG.id
+        task_id=f'{CALLBACK_CONFIG.task_id}', id=CALLBACK_CONFIG.id
     )
     result = await transport.get_task_callback(request=params)
 
@@ -631,7 +631,7 @@ def channel_factory(address: str) -> Channel:
 
     # Use GetTaskPushNotificationConfigRequest with name field (resource name)
     params = GetTaskPushNotificationConfigRequest(
-        task_id=f'tasks/{CALLBACK_CONFIG.task_id}', id=CALLBACK_CONFIG.id
+        task_id=f'{CALLBACK_CONFIG.task_id}', id=CALLBACK_CONFIG.id
     )
     result = await transport.get_task_callback(request=params)