Skip to content

fix: remove unused import and request for FastAPI in pyproject#934

Open
guglielmo-san wants to merge 9 commits into1.0-devfrom
guglielmoc/remove_unused_imports
Open

fix: remove unused import and request for FastAPI in pyproject#934
guglielmo-san wants to merge 9 commits into1.0-devfrom
guglielmoc/remove_unused_imports

Conversation

@guglielmo-san
Copy link
Copy Markdown
Member

@guglielmo-san guglielmo-san commented Apr 3, 2026

Description

This PR removes unused imports and dependencies across the project.

@guglielmo-san guglielmo-san changed the base branch from main to 1.0-dev April 3, 2026 15:02
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a 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 introduces significant updates to the A2A SDK, primarily focusing on migrating from Pydantic-based models to Protobuf-based models for the core data layer, adding support for database migrations, and implementing backward compatibility for the v0.3 protocol. My review identified critical issues in the GrpcHandler regarding incorrect status code access and the use of a non-existent trailing_metadata() method on the gRPC context. Additionally, I noted that the SQLAlchemy models in models.py require a custom TypeDecorator to handle Protobuf serialization, as the current JSON column type is insufficient.

I am having trouble creating individual review comments. Click here to see my feedback.

src/a2a/server/request_handlers/grpc_handler.py (397)

critical

The expression code.value[0] will raise a TypeError because grpc.StatusCode values are integers, which are not subscriptable. It should be changed to code.value to correctly retrieve the integer status code.

            status_code = code.value

src/a2a/server/models.py (54-58)

high

The status, artifacts, and history fields are now using Protobuf message types (from a2a.types.a2a_pb2), but the SQLAlchemy column type is set to JSON. Protobuf messages are not JSON-serializable by default using standard libraries like json.dumps. You should implement a custom TypeDecorator (e.g., ProtobufType) that handles serialization using google.protobuf.json_format.MessageToDict and ParseDict. Note that for A2A payloads, preserving_proto_field_name=True is not strictly required as lower camel case is permitted. Additionally, ensure the status field is non-nullable.

References
  1. The status field in TaskMixin must be non-nullable.
  2. The A2A protocol specification allows lower camel case for push notification JSON payloads, so preserving_proto_field_name=True is not strictly required when using MessageToDict.

src/a2a/server/request_handlers/grpc_handler.py (424-427)

high

The grpc.aio.ServicerContext object does not have a trailing_metadata() method. Attempting to call it will result in an AttributeError. If you need to merge metadata, you should track it manually within your handler or context builder.

@guglielmo-san guglielmo-san marked this pull request as ready for review April 3, 2026 15:20
@guglielmo-san guglielmo-san requested a review from a team as a code owner April 3, 2026 15:20
@guglielmo-san guglielmo-san requested a review from ishymko April 3, 2026 15:20
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 3, 2026

🧪 Code Coverage (vs 1.0-dev)

⬇️ Download Full Report

Base PR Delta
src/a2a/server/routes/jsonrpc_dispatcher.py 85.39% 85.98% 🟢 +0.59%
src/a2a/server/routes/rest_routes.py 73.53% 83.33% 🟢 +9.80%
Total 91.88% 91.95% 🟢 +0.07%

Generated by coverage-comment.yml

@guglielmo-san guglielmo-san enabled auto-merge (squash) April 3, 2026 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant