Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/a2a/server/apps/jsonrpc/fastapi_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,38 +66,41 @@
(SSE).
"""

def __init__( # noqa: PLR0913
self,
agent_card: AgentCard,
http_handler: RequestHandler,
extended_agent_card: AgentCard | None = None,
context_builder: CallContextBuilder | None = None,
card_modifier: Callable[[AgentCard], AgentCard] | None = None,
extended_card_modifier: Callable[
[AgentCard, ServerCallContext], AgentCard
]
| None = None,
max_content_length: int | None = 10 * 1024 * 1024, # 10MB
stream_send_timeout: float | None = None,
) -> None:
"""Initializes the A2AFastAPIApplication.

Args:
agent_card: The AgentCard describing the agent's capabilities.
http_handler: The handler instance responsible for processing A2A
requests via http.
extended_agent_card: An optional, distinct AgentCard to be served
at the authenticated extended card endpoint.
context_builder: The CallContextBuilder used to construct the
ServerCallContext passed to the http_handler. If None, no
ServerCallContext is passed.
card_modifier: An optional callback to dynamically modify the public
agent card before it is served.
extended_card_modifier: An optional callback to dynamically modify
the extended agent card before it is served. It receives the
call context.
max_content_length: The maximum allowed content length for incoming
requests. Defaults to 10MB. Set to None for unbounded maximum.
stream_send_timeout: The timeout in seconds for sending events in default timeout. Set to a larger value or None to disable for
long-running agents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

The phrase 'in default timeout' is unclear. The docstring should also clarify that None disables the timeout, which is a change in the default behavior. For clarity and consistency, could you update it?

            stream_send_timeout: The timeout in seconds for sending events in
              streaming responses. Defaults to `None`, which disables the timeout.
              Set a float value to specify a timeout.

"""

Check notice on line 103 in src/a2a/server/apps/jsonrpc/fastapi_app.py

View workflow job for this annotation

GitHub Actions / Lint Code Base

Copy/pasted code

see src/a2a/server/apps/rest/rest_adapter.py (55-87)
if not _package_fastapi_installed:
raise ImportError(
'The `fastapi` package is required to use the `A2AFastAPIApplication`.'
Expand All @@ -112,6 +115,7 @@
card_modifier=card_modifier,
extended_card_modifier=extended_card_modifier,
max_content_length=max_content_length,
stream_send_timeout=stream_send_timeout,
)

def add_routes_to_app(
Expand Down
14 changes: 13 additions & 1 deletion src/a2a/server/apps/jsonrpc/jsonrpc_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -172,38 +172,43 @@
for model in A2ARequestModel.__args__
}

def __init__( # noqa: PLR0913
self,
agent_card: AgentCard,
http_handler: RequestHandler,
extended_agent_card: AgentCard | None = None,
context_builder: CallContextBuilder | None = None,
card_modifier: Callable[[AgentCard], AgentCard] | None = None,
extended_card_modifier: Callable[
[AgentCard, ServerCallContext], AgentCard
]
| None = None,
max_content_length: int | None = 10 * 1024 * 1024, # 10MB
stream_send_timeout: float | None = None,
) -> None:
"""Initializes the JSONRPCApplication.

Args:
agent_card: The AgentCard describing the agent's capabilities.
http_handler: The handler instance responsible for processing A2A
requests via http.
extended_agent_card: An optional, distinct AgentCard to be served
at the authenticated extended card endpoint.
context_builder: The CallContextBuilder used to construct the
ServerCallContext passed to the http_handler. If None, no
ServerCallContext is passed.
card_modifier: An optional callback to dynamically modify the public
agent card before it is served.
extended_card_modifier: An optional callback to dynamically modify
the extended agent card before it is served. It receives the
call context.
max_content_length: The maximum allowed content length for incoming
requests. Defaults to 10MB. Set to None for unbounded maximum.
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to None, which uses Starlette's
default timeout. Set to a larger value or None to disable for
long-running agents.
Comment on lines 544 to 556
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

This docstring is contradictory. It first says Defaults to None, which uses Starlette's default timeout, and then says None to disable. The latter is correct: None disables the timeout. This is also a change in the default behavior (from a 5s timeout to no timeout), which is important to document. Please clarify the docstring.

Suggested change
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to None, which uses Starlette's
default timeout. Set to a larger value or None to disable for
long-running agents.
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to `None`, which disables the timeout.
This changes the default behavior from using Starlette's 5-second
default. Set a float value to specify a timeout.

"""

Check notice on line 211 in src/a2a/server/apps/jsonrpc/jsonrpc_app.py

View workflow job for this annotation

GitHub Actions / Lint Code Base

Copy/pasted code

see src/a2a/server/apps/rest/rest_adapter.py (55-87)
if not _package_starlette_installed:
raise ImportError(
'Packages `starlette` and `sse-starlette` are required to use the'
Expand All @@ -222,6 +227,7 @@
)
self._context_builder = context_builder or DefaultCallContextBuilder()
self._max_content_length = max_content_length
self.stream_send_timeout = stream_send_timeout

def _generate_error_response(
self, request_id: str | int | None, error: JSONRPCError | A2AError
Expand Down Expand Up @@ -540,8 +546,14 @@
async for item in stream:
yield {'data': item.root.model_dump_json(exclude_none=True)}

send_timeout = context.state.get(
'stream_send_timeout', self.stream_send_timeout
)

return EventSourceResponse(
event_generator(handler_result), headers=headers
event_generator(handler_result),
headers=headers,
send_timeout=send_timeout,
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The added tests for fastapi_app and starlette_app verify that the stream_send_timeout parameter is stored correctly on the application instance. However, they don't test that this value is actually used when creating an EventSourceResponse in this method.

To ensure the core logic of this fix is working as intended, I recommend adding a test that mocks EventSourceResponse and asserts it's called with the correct send_timeout value, covering cases where it's set on the app, None, and overridden via the context.state. This would provide more confidence in the change.

if isinstance(handler_result, JSONRPCErrorResponse):
return JSONResponse(
Expand Down
6 changes: 6 additions & 0 deletions src/a2a/server/apps/jsonrpc/starlette_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,18 +48,19 @@
(SSE).
"""

def __init__( # noqa: PLR0913
self,
agent_card: AgentCard,
http_handler: RequestHandler,
extended_agent_card: AgentCard | None = None,
context_builder: CallContextBuilder | None = None,
card_modifier: Callable[[AgentCard], AgentCard] | None = None,
extended_card_modifier: Callable[
[AgentCard, ServerCallContext], AgentCard
]
| None = None,
max_content_length: int | None = 10 * 1024 * 1024, # 10MB

Check notice on line 62 in src/a2a/server/apps/jsonrpc/starlette_app.py

View workflow job for this annotation

GitHub Actions / Lint Code Base

Copy/pasted code

see src/a2a/server/apps/rest/rest_adapter.py (55-66)
stream_send_timeout: float | None = None,
) -> None:
"""Initializes the A2AStarletteApplication.

Expand All @@ -79,6 +80,10 @@
call context.
max_content_length: The maximum allowed content length for incoming
requests. Defaults to 10MB. Set to None for unbounded maximum.
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to None, which uses Starlette's
default timeout. Set to a larger value or None to disable for
long-running agents.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

This docstring is contradictory. It first says Defaults to None, which uses Starlette's default timeout, and then says None to disable. The latter is correct: None disables the timeout. This is also a change in the default behavior (from a 5s timeout to no timeout), which is important to document. Please clarify the docstring.

Suggested change
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to None, which uses Starlette's
default timeout. Set to a larger value or None to disable for
long-running agents.
stream_send_timeout: The timeout in seconds for sending events in
streaming responses. Defaults to `None`, which disables the timeout.
This changes the default behavior from using Starlette's 5-second
default. Set a float value to specify a timeout.

"""
if not _package_starlette_installed:
raise ImportError(
Expand All @@ -94,6 +99,7 @@
card_modifier=card_modifier,
extended_card_modifier=extended_card_modifier,
max_content_length=max_content_length,
stream_send_timeout=stream_send_timeout,
)

def routes(
Expand Down
17 changes: 17 additions & 0 deletions tests/server/apps/jsonrpc/test_fastapi_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,23 @@ def test_create_a2a_fastapi_app_with_missing_deps_raises_importerror(
):
_app = A2AFastAPIApplication(**mock_app_params)

def test_stream_send_timeout_parameter(self, mock_app_params: dict):
try:
app_default = A2AFastAPIApplication(**mock_app_params)
assert app_default.stream_send_timeout is None

app_custom = A2AFastAPIApplication(
**mock_app_params, stream_send_timeout=30.0
)
assert app_custom.stream_send_timeout == 30.0

app_none = A2AFastAPIApplication(
**mock_app_params, stream_send_timeout=None
)
assert app_none.stream_send_timeout is None
except ImportError:
pytest.skip('FastAPI dependencies not available')


if __name__ == '__main__':
pytest.main([__file__])
17 changes: 17 additions & 0 deletions tests/server/apps/jsonrpc/test_starlette_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,23 @@ def test_create_a2a_starlette_app_with_missing_deps_raises_importerror(
):
_app = A2AStarletteApplication(**mock_app_params)

def test_stream_send_timeout_parameter(self, mock_app_params: dict):
try:
app_default = A2AStarletteApplication(**mock_app_params)
assert app_default.stream_send_timeout is None

app_custom = A2AStarletteApplication(
**mock_app_params, stream_send_timeout=30.0
)
assert app_custom.stream_send_timeout == 30.0

app_none = A2AStarletteApplication(
**mock_app_params, stream_send_timeout=None
)
assert app_none.stream_send_timeout is None
except ImportError:
pytest.skip('Starlette dependencies not available')


if __name__ == '__main__':
pytest.main([__file__])
Loading