Skip to content

fix(utils): protobuf-compatible repeated field detection#1141

Open
piyushbag wants to merge 1 commit into
a2aproject:mainfrom
piyushbag:fix/1011-proto-is-repeated
Open

fix(utils): protobuf-compatible repeated field detection#1141
piyushbag wants to merge 1 commit into
a2aproject:mainfrom
piyushbag:fix/1011-proto-is-repeated

Conversation

@piyushbag

Copy link
Copy Markdown

Summary

  • Add _is_field_repeated() in proto_utils that prefers field.is_repeated and falls back to field.label == FieldDescriptor.LABEL_REPEATED when the newer API is unavailable.
  • Replace the three repeated-field checks in REST param parsing and proto required-field validation with the helper.
  • Add unit tests for scalar/repeated fields and for the label fallback path.

Problem

proto_utils used the deprecated FieldDescriptor.label API. Protobuf 7 removes label, which breaks validation on newer runtimes even though the SDK dependency range can resolve protobuf 7.x in some environments.

Test plan

  • uv run pytest tests/utils/test_proto_utils.py -q
  • Verified _is_field_repeated against protobuf 6.x locally (is_repeated + label both present)

Fixes #1011

Add _is_field_repeated helper that prefers field.is_repeated and falls
back to field.label on older protobuf runtimes. Fixes crashes when newer
protobuf builds remove label while the SDK still relied on it.

Fixes a2aproject#1011
@piyushbag
piyushbag requested a review from a team as a code owner July 21, 2026 17:38
@github-actions

Copy link
Copy Markdown

🧪 Code Coverage (vs main)

⬇️ Download Full Report

Base PR Delta
src/a2a/utils/proto_utils.py 90.87% 91.08% 🟢 +0.21%
Total 92.97% 92.98% ⚪️ 0.00%

Generated by coverage-comment.yml

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

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 a helper function _is_field_repeated to safely check if a protobuf field is repeated across different protobuf versions, replacing deprecated field.label checks. It also adds unit tests for this helper. The review feedback points out that monkeypatching FieldDescriptor in test_fallback_when_is_repeated_missing will fail in environments using the fast C++ implementation of Protobuf because C-extension types are read-only. A code suggestion is provided to use a simple mock object instead.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +255 to +266
def test_fallback_when_is_repeated_missing(self, monkeypatch):
field = AgentSkill.DESCRIPTOR.fields_by_name['tags']

def raise_attribute_error(_self):
raise AttributeError('is_repeated')

monkeypatch.setattr(
type(field),
'is_repeated',
property(raise_attribute_error),
)
assert proto_utils._is_field_repeated(field) is True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Modifying the attributes or properties of FieldDescriptor (or its type) using monkeypatch will fail with a TypeError in environments where the fast C++ implementation of Protobuf is used (which is the default in many environments, as FieldDescriptor is a C-extension type and its attributes/type are read-only).

Instead of monkeypatching the built-in FieldDescriptor class, you can use a simple mock object or a custom class that lacks the is_repeated attribute to safely test the fallback path.

Suggested change
def test_fallback_when_is_repeated_missing(self, monkeypatch):
field = AgentSkill.DESCRIPTOR.fields_by_name['tags']
def raise_attribute_error(_self):
raise AttributeError('is_repeated')
monkeypatch.setattr(
type(field),
'is_repeated',
property(raise_attribute_error),
)
assert proto_utils._is_field_repeated(field) is True
def test_fallback_when_is_repeated_missing(self):
class MockField:
label = proto_utils.FieldDescriptor.LABEL_REPEATED
field = MockField()
assert proto_utils._is_field_repeated(field) is True
References
  1. When checking for repeated fields in Protobuf, access descriptor constants via the FieldDescriptor class (e.g., FieldDescriptor.LABEL_REPEATED) rather than directly from the field instance (e.g., field.is_repeated).

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.

[Task]: Replace deprecated FieldDescriptor.label with is_repeated in proto_utils

1 participant