feat: Add support for custom task store in to_a2a function #3639
Conversation
This update introduces an optional `task_store` parameter to the `to_a2a` function, allowing users to provide a custom task store instance. If not specified, the function defaults to using `InMemoryTaskStore`. Additionally, unit tests have been added to verify the functionality with a custom task store.
Summary of ChangesHello @secprog, 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 introduces a significant enhancement to the Highlights
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
|
adk-bot
added
the
core
|
Response from ADK Triaging Agent Hello @secprog, thank you for creating this PR! To help reviewers better understand the context and purpose of your contribution, could you please associate a GitHub issue with this PR? If there is no existing issue, you can create one. In addition, could you please add a This information will help reviewers to review your PR more efficiently. Thanks! |
![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 introduces a task_store parameter to the to_a2a function, allowing for custom task store implementations, with InMemoryTaskStore as the default. The changes are well-implemented and include corresponding unit tests. My main feedback is to improve the type hinting for the new task_store parameter by replacing Any with a more specific type, such as InMemoryTaskStore or a Protocol, to enhance type safety and code clarity.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Removed unused import of 'Any' from typing.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature by allowing a custom task_store to be passed to the to_a2a function, defaulting to InMemoryTaskStore if none is provided. The implementation is clean and follows the description. The addition of a unit test ensures the new functionality is working as expected. I have one suggestion regarding the type hint for the new parameter to improve API clarity and maintainability.
Fix type hint for task_store parameter.
|
/gemini review |
|
/gemini summary |
There was a problem hiding this comment.
Code Review
This pull request introduces an optional task_store parameter to the to_a2a function, allowing for more flexible task management by enabling the use of custom task stores. The implementation correctly defaults to InMemoryTaskStore when no custom store is provided. The accompanying unit test is well-written and effectively validates the new functionality by ensuring a custom task store is utilized when passed and that the default store is not instantiated in this case. The changes are clear, well-scoped, and improve the utility's extensibility.
Summary of ChangesThis pull request significantly enhances the Highlights
Changelog
Activity
|
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully adds support for a custom task_store to the to_a2a function, with a sensible default to InMemoryTaskStore. The implementation and documentation changes are clear. A new unit test is added to cover this functionality. However, the new test is missing an import for TaskStore, which will cause it to fail. Please address this issue.
Refactor the test to replace the mock of InMemoryTaskStore with TaskStore, ensuring consistency with the updated type hint for task_store. This change enhances the clarity and accuracy of the unit tests.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for a custom task store in the to_a2a function, which is a great enhancement for flexibility. The implementation is straightforward and correct. I've added a comment to improve the clarity of the docstring example.
However, I found a critical issue in the new unit test. The order of mock arguments is incorrect due to how @patch decorators work, which makes the test invalid. I've provided a suggestion to fix this. Please ensure the test passes with the corrected argument order.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for a custom task_store in the to_a2a function, defaulting to InMemoryTaskStore if none is provided. The change is well-documented and includes a new unit test to verify the functionality. My review has identified a critical issue in the new unit test where the mocked arguments are in the wrong order, which will cause the test to fail. I've also provided a medium-severity suggestion to improve the clarity of the code example in the docstring to avoid misleading users about the implementation of a custom TaskStore.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request effectively adds support for a custom task_store to the to_a2a function, defaulting to InMemoryTaskStore when one isn't provided. The changes are clear, the documentation is updated with a helpful example, and a new unit test correctly validates the new functionality. I have one minor suggestion to improve the conciseness of the new test.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request successfully adds support for a custom task_store in the to_a2a function, increasing its flexibility. The implementation is straightforward, and the default behavior of using InMemoryTaskStore is preserved. The new functionality is well-documented with updates to the docstring and a clear example.
The accompanying unit test correctly verifies that a custom task store is used when provided and that the default is not instantiated in that case. I've left one suggestion in the test file to refactor some duplicated test setup code using a pytest fixture, which would improve the overall maintainability of the test suite.
Overall, this is a solid contribution that enhances the utility of the to_a2a function.
|
/gemini summary |
Summary of ChangesThis pull request introduces a significant enhancement to the Highlights
Changelog
Activity
|
|
Hi @secprog Thank you for your contribution! We appreciate you taking the time to submit this pull request. |
ryanaiagent
added
request clarification
… modules Removed unnecessary blank lines and organized imports for better readability in `experiment.py` and `run_experiment.py`. Also, updated import statements in `agent_to_a2a.py` and its corresponding test file to maintain consistency and clarity.
@ryanaiagent Done :) |
3 participants
This update introduces an optional
task_storeparameter to theto_a2afunction, allowing users to provide a custom task store instance. If not specified, the function defaults to usingInMemoryTaskStore. Additionally, unit tests have been added to verify the functionality with a custom task store.