fix(server): generate complete OpenAPI schema using compiled protobuf specifications#849
Conversation
…efactor import statements.
…e_a2a_json_from_fast_api
Summary of ChangesHello, 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 refactors the OpenAPI schema generation logic within the A2A server applications. The core change involves centralizing the Protobuf-based OpenAPI schema loading into the REST server, where it is compatible and provides accurate documentation for A2A v1.0 Protobuf types. Concurrently, the JSON-RPC server's application setup has been streamlined by removing this incompatible custom OpenAPI handling, ensuring each server correctly manages its API documentation. Highlights
Changelog
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.
Code Review
This pull request correctly refactors the OpenAPI schema generation by moving the logic that loads the a2a.json schema from the JSON-RPC application to the REST application. This aligns with the fact that the schema is only compatible with the REST transport. The new implementation in the REST application also correctly prepends the rpc_url to the schema paths. The changes are logical and well-executed. I have one minor suggestion to improve exception handling for better robustness and easier debugging.
| except Exception: # noqa: BLE001 | ||
| logger.warning( | ||
| "Could not load 'a2a.json' from 'a2a.types'. Falling back to auto-generation." | ||
| ) |
There was a problem hiding this comment.
Catching a broad Exception is generally discouraged as it can hide unexpected errors. It's better to catch specific exceptions that you expect to handle, such as ImportError, OSError, and json.JSONDecodeError. This will make the code more robust and easier to debug. Additionally, logging the specific exception provides more context for troubleshooting.
| except Exception: # noqa: BLE001 | |
| logger.warning( | |
| "Could not load 'a2a.json' from 'a2a.types'. Falling back to auto-generation." | |
| ) | |
| except (ImportError, OSError, json.JSONDecodeError) as e: | |
| logger.warning( | |
| "Could not load 'a2a.json' from 'a2a.types'. Falling back to auto-generation: %s", | |
| e, | |
| ) |
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/apps/rest/fastapi_app.py | 85.42% | 58.23% | 🔴 -27.19% |
| Total | 90.22% | 89.92% | 🔴 -0.30% |
Generated by coverage-comment.yml
Description
This PR addresses OpenAPI serialization in the REST and JSON-RPC FastAPI bindings, ensuring that the interactive Swagger documentation parses and displays A2A v1.0 Protobuf inputs/outputs for the REST server, removing it from the JSON-RPC. Indeed the
a2a.jsongenerate file from thea2a.protois compatible with the REST transport, but not with the JSON-RPC.