Conversation
# Description Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly: - [x] Follow the [`CONTRIBUTING` Guide](https://github.com/a2aproject/a2a-python/blob/main/CONTRIBUTING.md). - [x] Make your Pull Request title in the <https://www.conventionalcommits.org/> specification. - Important Prefixes for [release-please](https://github.com/googleapis/release-please): - `fix:` which represents bug fixes, and correlates to a [SemVer](https://semver.org/) patch. - `feat:` represents a new feature, and correlates to a SemVer minor. - `feat!:`, or `fix!:`, `refactor!:`, etc., which represent a breaking change (indicated by the `!`) and will result in a SemVer major. - [x] Ensure the tests and linter pass (Run `bash scripts/format.sh` from the repository root to format) - [x] Appropriate docs were updated (if necessary) Follow-up to #682, as suggested by @ishymko in the [review](#682 (review)). This extends the async context manager pattern to `BaseClient`, which wraps `ClientTransport` and also exposes a `close()` method. Fixes #674 🦕 ## Problem `BaseClient` delegates resource cleanup to its underlying `ClientTransport` via `close()`, but doesn't implement `__aenter__`/`__aexit__`. This means clients cannot be used with `async with`, leading to the same resource leak risk that #682 solved for transports: ```python client = BaseClient(card=card, config=config, transport=transport, consumers=[], middleware=[]) result = await client.send_message(msg) # if this raises, close() is never called await client.close() ``` ## Fix Added `__aenter__` and `__aexit__` methods to `BaseClient` in `src/a2a/client/base_client.py`: `__aenter__` returns `self` `__aexit__ `awaits `close()` This enables the standard async context manager pattern: ```python async with BaseClient(card=card, config=config, transport=transport, consumers=[], middleware=[]) as client: async for event in client.send_message(msg): ... # close() called automatically, even on exceptions ``` This is a non-breaking, additive change. Calling `close()` manually or via `try/finally` continues to work exactly as before. ## Test Tests were added to `tests/client/test_base_client.py`, following the same approach as the `ClientTransport` tests from #682. Release-As: 0.3.23
…PC clients (#690) This PR standardizes timeout error handling across the JSON-RPC and REST clients. Previously, only the JSON-RPC client (in non-streaming mode) handled `ReadTimeout` exceptions, while streaming calls and the REST client catch them incorrectly. Updating both `JsonRpcTransport` and `RestTransport` to catch the base httpx.TimeoutException, all timeout types (Read, Connect, Write, Pool) are consistently caught and wrapped in A2AClientTimeoutError. This ensures consistent behavior for API consumers regardless of the transport (REST vs JSON-RPC) or mode (Streaming vs Non-Streaming) being used, preventing generic errors when network timeouts occur.
Summary of ChangesHello @ishymko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the client's robustness and ease of use by introducing asynchronous context management for the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 merges changes from main into 1.0-dev. The main changes include implementing the async context manager protocol for BaseClient to ensure resources are properly closed, and unifying timeout handling across JsonRpcTransport and RestTransport to catch httpx.TimeoutException. Corresponding tests have been added for this new functionality.
The changes are well-implemented and improve the client's robustness and usability. I have a minor suggestion to replace the hardcoded timeout error message string with a constant to improve maintainability. Overall, great work.
| raise A2AClientJSONRPCError(response.root) | ||
| yield response.root.result | ||
| except httpx.TimeoutException as e: | ||
| raise A2AClientTimeoutError('Client Request timed out') from e |
There was a problem hiding this comment.
The error message 'Client Request timed out' is hardcoded here and in other places within this file, as well as in src/a2a/client/transports/rest.py. To improve maintainability and avoid magic strings, consider defining this as a module-level constant in each file. For example:
_CLIENT_REQUEST_TIMEOUT_MESSAGE = 'Client Request timed out'This constant can then be used whenever A2AClientTimeoutError is raised in the respective transport files.
3 participants
No description provided.