-
Notifications
You must be signed in to change notification settings - Fork 24
Add streaming support
#43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
3e60fa1
8a5dbfe
5903cff
d16abca
23907cf
dec5581
33ee9c5
1709673
2db81b8
ab97e62
e5e2be8
447153f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| .cache | ||
| __pycache__ | ||
| .coverage* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| from .applications import FastA2A | ||
| from .broker import Broker | ||
| from .schema import Skill | ||
| from .schema import Skill, StreamEvent | ||
| from .storage import Storage | ||
| from .worker import Worker | ||
|
|
||
| __all__ = ['FastA2A', 'Skill', 'Storage', 'Broker', 'Worker'] | ||
| __all__ = ['FastA2A', 'Skill', 'Storage', 'Broker', 'Worker', 'StreamEvent'] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,12 @@ | |
| from typing import Annotated, Any, Generic, Literal, TypeVar | ||
|
|
||
| import anyio | ||
| from anyio.streams.memory import MemoryObjectSendStream | ||
| from opentelemetry.trace import Span, get_current_span, get_tracer | ||
| from pydantic import Discriminator | ||
| from typing_extensions import Self, TypedDict | ||
|
|
||
| from .schema import TaskIdParams, TaskSendParams | ||
| from .schema import StreamEvent, TaskIdParams, TaskSendParams | ||
|
|
||
| tracer = get_tracer(__name__) | ||
|
|
||
|
|
@@ -37,6 +38,26 @@ async def cancel_task(self, params: TaskIdParams) -> None: | |
| """Cancel a task.""" | ||
| raise NotImplementedError('send_cancel_task is not implemented yet.') | ||
|
|
||
| @abstractmethod | ||
| async def send_stream_event(self, task_id: str, event: StreamEvent) -> None: | ||
| """Send a streaming event from worker to subscribers. | ||
|
|
||
| This is used by workers to publish status updates, messages, and artifacts | ||
| during task execution. Events are forwarded to all active subscribers of | ||
| the given task_id. | ||
| """ | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| def subscribe_to_stream(self, task_id: str) -> AsyncIterator[StreamEvent]: | ||
| """Subscribe to streaming events for a specific task. | ||
|
|
||
| Returns an async iterator that yields events published by workers for the | ||
| given task_id. The iterator completes when a TaskStatusUpdateEvent with | ||
| final=True is received or the subscription is cancelled. | ||
| """ | ||
| ... | ||
|
|
||
| @abstractmethod | ||
| async def __aenter__(self) -> Self: ... | ||
|
|
||
|
|
@@ -73,6 +94,10 @@ class _TaskOperation(TypedDict, Generic[OperationT, ParamsT]): | |
| class InMemoryBroker(Broker): | ||
| """A broker that schedules tasks in memory.""" | ||
|
|
||
| def __init__(self) -> None: | ||
| self._event_subscribers: dict[str, list[MemoryObjectSendStream[StreamEvent]]] = {} | ||
| self._subscriber_lock: anyio.Lock | None = None | ||
|
|
||
| async def __aenter__(self): | ||
| self.aexit_stack = AsyncExitStack() | ||
| await self.aexit_stack.__aenter__() | ||
|
|
@@ -81,6 +106,8 @@ async def __aenter__(self): | |
| await self.aexit_stack.enter_async_context(self._read_stream) | ||
| await self.aexit_stack.enter_async_context(self._write_stream) | ||
|
|
||
| self._subscriber_lock = anyio.Lock() | ||
|
|
||
| return self | ||
|
|
||
| async def __aexit__(self, exc_type: Any, exc_value: Any, traceback: Any): | ||
|
|
@@ -96,3 +123,65 @@ async def receive_task_operations(self) -> AsyncIterator[TaskOperation]: | |
| """Receive task operations from the broker.""" | ||
| async for task_operation in self._read_stream: | ||
| yield task_operation | ||
|
|
||
| async def send_stream_event(self, task_id: str, event: StreamEvent) -> None: | ||
| """Send a streaming event from worker to subscribers.""" | ||
| assert self._subscriber_lock is not None, 'Broker not initialized' | ||
|
|
||
| async with self._subscriber_lock: | ||
| subscribers = self._event_subscribers.get(task_id, []) | ||
| if not subscribers: | ||
| return | ||
|
|
||
| # Send event to all subscribers, removing closed streams | ||
| active_subscribers: list[MemoryObjectSendStream[StreamEvent]] = [] | ||
| for stream in subscribers: | ||
| try: | ||
| await stream.send(event) | ||
| active_subscribers.append(stream) | ||
| except (anyio.ClosedResourceError, anyio.BrokenResourceError): | ||
| # Subscriber disconnected, remove from list | ||
| pass | ||
|
|
||
| # Update subscriber list with only active ones | ||
| if active_subscribers: | ||
| self._event_subscribers[task_id] = active_subscribers | ||
| elif task_id in self._event_subscribers: | ||
| # No active subscribers left, clean up | ||
| del self._event_subscribers[task_id] | ||
|
Comment on lines
+131
to
+151
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Potential lock contention in send_stream_event while holding lock during await In Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| async def subscribe_to_stream(self, task_id: str) -> AsyncIterator[StreamEvent]: | ||
| """Subscribe to streaming events for a specific task.""" | ||
| assert self._subscriber_lock is not None, 'Broker not initialized' | ||
|
|
||
| # Create a new stream for this subscriber | ||
| send_stream, receive_stream = anyio.create_memory_object_stream[StreamEvent](max_buffer_size=100) | ||
|
|
||
| # Register the subscriber | ||
| async with self._subscriber_lock: | ||
| if task_id not in self._event_subscribers: | ||
| self._event_subscribers[task_id] = [] | ||
| self._event_subscribers[task_id].append(send_stream) | ||
|
|
||
| try: | ||
| async with receive_stream: | ||
| async for event in receive_stream: | ||
| yield event | ||
|
|
||
| # Check if this is a final status update | ||
| if isinstance(event, dict) and event.get('kind') == 'status-update' and event.get('final', False): | ||
| break | ||
| finally: | ||
| # Clean up subscription on exit | ||
| async with self._subscriber_lock: | ||
| if task_id in self._event_subscribers: | ||
| try: | ||
| self._event_subscribers[task_id].remove(send_stream) | ||
| if not self._event_subscribers[task_id]: | ||
| del self._event_subscribers[task_id] | ||
| except ValueError: | ||
| # Already removed | ||
| pass | ||
|
|
||
| # Close the send stream | ||
| await send_stream.aclose() | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 InMemoryStorage.load_task mutates the stored task's history in place Pre-existing issue: Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,9 @@ | |
|
|
||
| from __future__ import annotations as _annotations | ||
|
|
||
| import asyncio | ||
| import uuid | ||
| from collections.abc import AsyncGenerator | ||
| from contextlib import AsyncExitStack | ||
| from dataclasses import dataclass, field | ||
| from typing import Any | ||
|
|
@@ -78,8 +80,8 @@ | |
| SendMessageResponse, | ||
| SetTaskPushNotificationRequest, | ||
| SetTaskPushNotificationResponse, | ||
| StreamEvent, | ||
| StreamMessageRequest, | ||
| StreamMessageResponse, | ||
| TaskNotFoundError, | ||
| TaskSendParams, | ||
| ) | ||
|
|
@@ -156,9 +158,44 @@ async def cancel_task(self, request: CancelTaskRequest) -> CancelTaskResponse: | |
| ) | ||
| return CancelTaskResponse(jsonrpc='2.0', id=request['id'], result=task) | ||
|
|
||
| async def stream_message(self, request: StreamMessageRequest) -> StreamMessageResponse: | ||
| """Stream messages using Server-Sent Events.""" | ||
| raise NotImplementedError('message/stream method is not implemented yet.') | ||
| async def stream_message(self, request: StreamMessageRequest) -> AsyncGenerator[StreamEvent, None]: | ||
| """Handle a streaming message request. | ||
| This method: | ||
| 1. Creates and submits a new task | ||
| 2. Yields the initial task object | ||
| 3. Subscribes to the broker's event stream | ||
| 4. Starts task execution asynchronously | ||
| 5. Streams all events until completion | ||
| """ | ||
| # Extract parameters | ||
| params = request['params'] | ||
| message = params['message'] | ||
| context_id = message.get('context_id', str(uuid.uuid4())) | ||
|
|
||
| # Create and submit the task | ||
| task = await self.storage.submit_task(context_id, message) | ||
|
|
||
| # Yield the initial task | ||
| yield task | ||
|
|
||
| # Prepare broker params | ||
| broker_params: TaskSendParams = {'id': task['id'], 'context_id': context_id, 'message': message} | ||
| config = params.get('configuration', {}) | ||
| history_length = config.get('history_length') | ||
| if history_length is not None: | ||
| broker_params['history_length'] = history_length | ||
|
|
||
| metadata = params.get('metadata') | ||
| if metadata is not None: | ||
| broker_params['metadata'] = metadata | ||
|
Comment on lines
+189
to
+191
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Inconsistent metadata forwarding between send_message and stream_message The Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| # Start task execution in background | ||
| asyncio.create_task(self.broker.run_task(broker_params)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Fire-and-forget At line 194, Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| # Stream events from broker | ||
| async for event in self.broker.subscribe_to_stream(task['id']): | ||
| yield event | ||
|
Comment on lines
+194
to
+198
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Race condition: task execution starts before stream subscription is registered In Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+194
to
+198
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 Subscription registration vs task execution ordering relies on subtle asyncio scheduling guarantees In Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
| async def set_task_push_notification( | ||
| self, request: SetTaskPushNotificationRequest | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -10,11 +10,12 @@ | |||||
| from opentelemetry.trace import get_tracer, use_span | ||||||
| from typing_extensions import assert_never | ||||||
|
|
||||||
| from .schema import TaskArtifactUpdateEvent, TaskStatusUpdateEvent | ||||||
| from .storage import ContextT, Storage | ||||||
|
|
||||||
| if TYPE_CHECKING: | ||||||
| from .broker import Broker, TaskOperation | ||||||
| from .schema import Artifact, Message, TaskIdParams, TaskSendParams | ||||||
| from .schema import Artifact, Message, TaskIdParams, TaskSendParams, TaskState | ||||||
|
|
||||||
| tracer = get_tracer(__name__) | ||||||
|
|
||||||
|
|
@@ -56,6 +57,66 @@ async def _handle_task_operation(self, task_operation: TaskOperation) -> None: | |||||
| except Exception: | ||||||
| await self.storage.update_task(task_operation['params']['id'], state='failed') | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Exception handler in _handle_task_operation bypasses streaming event publishing When a task raises an exception,
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||
|
|
||||||
| async def update_task( | ||||||
| self, | ||||||
| task_id: str, | ||||||
| state: TaskState, | ||||||
| new_artifacts: list[Artifact] | None = None, | ||||||
| new_messages: list[Message] | None = None, | ||||||
| ) -> None: | ||||||
| """Update a task's state in storage and publish streaming events to the broker. | ||||||
| This is the primary method workers should use to update task state. It handles | ||||||
| both persisting the update and notifying any stream subscribers. | ||||||
| """ | ||||||
| task = await self.storage.update_task(task_id, state, new_artifacts, new_messages) | ||||||
|
|
||||||
| final = state in ('completed', 'failed', 'canceled') | ||||||
|
|
||||||
| # For non-final updates, publish status first | ||||||
| if not final: | ||||||
| await self.broker.send_stream_event( | ||||||
| task_id, | ||||||
| TaskStatusUpdateEvent( | ||||||
| kind='status-update', | ||||||
| task_id=task_id, | ||||||
| context_id=task['context_id'], | ||||||
| status=task['status'], | ||||||
| final=False, | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| # Publish message events before final status so subscribers receive them | ||||||
| if new_messages: | ||||||
| for message in new_messages: | ||||||
| await self.broker.send_stream_event(task_id, message) | ||||||
|
|
||||||
| # Publish artifact events | ||||||
| if new_artifacts: | ||||||
| for artifact in new_artifacts: | ||||||
| await self.broker.send_stream_event( | ||||||
| task_id, | ||||||
| TaskArtifactUpdateEvent( | ||||||
| kind='artifact-update', | ||||||
| task_id=task_id, | ||||||
| context_id=task['context_id'], | ||||||
| artifact=artifact, | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| # For final updates, publish status last (after messages and artifacts) | ||||||
| if final: | ||||||
| await self.broker.send_stream_event( | ||||||
| task_id, | ||||||
| TaskStatusUpdateEvent( | ||||||
| kind='status-update', | ||||||
| task_id=task_id, | ||||||
| context_id=task['context_id'], | ||||||
| status=task['status'], | ||||||
| final=True, | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| @abstractmethod | ||||||
| async def run_task(self, params: TaskSendParams) -> None: ... | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚩 Default streaming=True may break backward compatibility for non-streaming setups
The
FastA2A.__init__now defaultsstreaming=True(fasta2a/applications.py:49), which means the agent card will advertise streaming capability. If a user upgrades without wrapping their storage inStreamingStorageWrapper, the agent card will claim streaming support butmessage/streamrequests will fail because no streaming events are published. This is a potential compatibility concern — previously streaming was hardcoded toFalse(fasta2a/applications.py:103). The default beingTrueassumes all users are streaming-ready.Was this helpful? React with 👍 or 👎 to provide feedback.