-
Notifications
You must be signed in to change notification settings - Fork 325
Update to specification from 60f83c3faac4770b231f038406c9e02282887a25 #608
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?
Update to specification from 60f83c3faac4770b231f038406c9e02282887a25 #608
Conversation
Summary of ChangesHello @a2a-bot, 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 brings the A2A protocol specification up to date, incorporating a range of structural and functional enhancements. The changes primarily focus on refining existing data models, introducing new capabilities for task management, and improving the overall clarity and consistency of the API definitions across protobuf, gRPC, and Python type hints. These updates aim to provide a more robust and developer-friendly interface for interacting with the A2A service. 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
|
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.
Code Review
This pull request updates the generated Python code for the A2A protocol specification, reflecting changes from a new version. The changes include updates to protobuf messages, gRPC services, and Pydantic type models. My review focuses on the generated gRPC service file, where I've noticed that many docstrings have become less descriptive. I've left several low-severity comments suggesting that the source .proto file comments be enhanced to restore the helpful documentation in the generated code, providing examples of more descriptive docstrings. All original comments were kept as they do not contradict any provided rules.
| """A2AService defines the operations of the A2A protocol. | ||
| """ |
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.
The docstring for A2AServiceStub has been shortened, which results in a loss of valuable information about the service structure and its resources. The previous version provided a helpful overview. While this is a generated file, it would be beneficial for developers if the descriptiveness could be restored. This might require updating the comments in the source .proto file.
For reference, here is a more descriptive docstring that reflects the latest service definition:
"""A2AService defines the operations of the A2A protocol.
The service is structured around several resources:
- **Message**: Sent via `SendMessage` and `SendStreamingMessage`. Not a standard resource.
- **Task**: Represents an operation. Can be retrieved with `GetTask`, listed with `ListTasks`, canceled with `CancelTask`, and subscribed to with `SubscribeToTask`.
- **TaskPushNotificationConfig**: A sub-resource of Task for managing push notifications. Supports `Set`, `Get`, `List`, and `Delete` operations.
- **AgentCard**: The `GetExtendedAgentCard` method retrieves detailed agent information for authenticated clients.
"""| """A2AService defines the operations of the A2A protocol. | ||
| """ |
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.
Similar to A2AServiceStub, the docstring for A2AServiceServicer has lost its descriptive summary. To improve usability, consider enhancing the comments in the source .proto file so that a more detailed docstring is generated.
A good docstring could look like this:
"""A2AService defines the operations of the A2A protocol.
The service is structured around several resources:
- **Message**: Sent via `SendMessage` and `SendStreamingMessage`. Not a standard resource.
- **Task**: Represents an operation. Can be retrieved with `GetTask`, listed with `ListTasks`, canceled with `CancelTask`, and subscribed to with `SubscribeToTask`.
- **TaskPushNotificationConfig**: A sub-resource of Task for managing push notifications. Supports `Set`, `Get`, `List`, and `Delete` operations.
- **AgentCard**: The `GetExtendedAgentCard` method retrieves detailed agent information for authenticated clients.
"""| """Send a message to the agent. | ||
| """ |
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.
The docstring for SendMessage has been shortened. The previous version provided more context about its behavior. A more descriptive docstring would be helpful. This could be improved in the source .proto file.
Example of a more descriptive docstring:
"""Sends a message to the agent.
This can be a blocking or non-blocking call depending on the `SendMessageConfiguration`.
It can return either a `Task` object for asynchronous operations or a direct `Message` response.
"""| """SendStreamingMessage is a streaming version of SendMessage. | ||
| """ |
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.
The docstring for SendStreamingMessage is now very brief. The previous version was more informative. Consider updating the source .proto file to generate a more helpful docstring.
For example:
"""SendStreamingMessage is a streaming version of SendMessage.
It initiates a task and returns a stream of events, including status updates,
artifact updates, and final messages, until the task reaches a terminal state.
"""| """Cancel a task. | ||
| """ |
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.
The CancelTask docstring has been shortened. The previous version included useful information about the expected behavior after cancellation. It would be beneficial to restore this detail, likely by updating the source .proto file.
For example:
"""Cancels a task. If supported, no more task updates should be expected for this task.
"""| """A2AService defines the operations of the A2A protocol. | ||
| """ |
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.
The docstring for the experimental A2AService class has been shortened, losing a helpful summary of the service. Consider improving the source .proto file comments to generate a more descriptive docstring here as well.
A good docstring could look like this:
"""A2AService defines the operations of the A2A protocol.
The service is structured around several resources:
- **Message**: Sent via `SendMessage` and `SendStreamingMessage`. Not a standard resource.
- **Task**: Represents an operation. Can be retrieved with `GetTask`, listed with `ListTasks`, canceled with `CancelTask`, and subscribed to with `SubscribeToTask`.
- **TaskPushNotificationConfig**: A sub-resource of Task for managing push notifications. Supports `Set`, `Get`, `List`, and `Delete` operations.
- **AgentCard**: The `GetExtendedAgentCard` method retrieves detailed agent information for authenticated clients.
"""
Commit: a2aproject/A2A@60f83c3