Skip to content

Migrate field label to is_repeated, enums removed in protobuf v34.0#33

Merged
pmai merged 5 commits into
mainfrom
fix/migrate-pb-field-labels
May 7, 2026
Merged

Migrate field label to is_repeated, enums removed in protobuf v34.0#33
pmai merged 5 commits into
mainfrom
fix/migrate-pb-field-labels

Conversation

@thomassedlmayer
Copy link
Copy Markdown
Contributor

See

When using protobuf versions based on v34.0 (Python package version number >7.34) I get this error in the result file:

<Checker status="error" checkerId="osirules_osi" description="Evaluates messages in the trace file against the OSI Rules of the given OSI version to guarantee they are in conformance with the standard OSI rules." summary="Checker validating OSI Rules compliance of messages in a trace file Error: 'google._upb._message.FieldDescriptor' object has no attribute 'label'.">

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@thomassedlmayer thomassedlmayer requested a review from pmai April 24, 2026 12:59
@thomassedlmayer thomassedlmayer changed the title Migrate field labels to is_repeated, enums removed in protobuf v34.0 Migrate field label to is_repeated, enums removed in protobuf v34.0 Apr 24, 2026
@jdsika jdsika added bug Something isn't working ReadyForMerge An issue that has been reviewed by CCB and can be merged ReadyForCCBReview CCB will review it and change the status to ReadyForMerge if everything is ok and removed ReadyForMerge An issue that has been reviewed by CCB and can be merged labels Apr 24, 2026
@jdsika
Copy link
Copy Markdown
Contributor

jdsika commented Apr 24, 2026

Does it then still work with old protobuf versions?

@thomassedlmayer
Copy link
Copy Markdown
Contributor Author

They seem to have introduced is_required and is_repeated in v31.0 and removed label in v34.0. So it works for all protobuf versions >=6.31.0 if I'm not mistaken. Maybe we should add protobuf>=6.31.0 to the dependencies either here or in osi-python? osi-python currently uses protobuf>=6.30.2.

https://github.com/protocolbuffers/protobuf/blob/v31.0/python/google/protobuf/descriptor.py#L732

https://github.com/protocolbuffers/protobuf/blob/v30.2/python/google/protobuf/descriptor.py -> Didn't find any is_repeated

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@pmai
Copy link
Copy Markdown
Contributor

pmai commented Apr 24, 2026

The changeover only happened a year ago in 2025, in this commit: protocolbuffers/protobuf@5bd7d02 and is officially only part of v32 and newer, AFAICT.

So if we don't want to be as unnecessarily disruptive as our google friends, I would rather suggest that forcing everyone who uses qc-osi-trace (which might be in a much larger project with lots of different protobuf users with different version requirements), we might want to do more than just switch over.

Either we catch/disable the deprecation warning for now, or we provide different code for both versions (doing this in the inner loop might be a bit expensive, however, so might not want to do that trivially).

We could also as an interim measure pin protobuf (we do not receive any benefits from their constant churn, imho), until this becomes clearer.

Probably best approach - though uglier and really should not be necessary if google didn't yolo all the time - is to create own wrapper predicates based on detected version.

Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai
Copy link
Copy Markdown
Contributor

pmai commented Apr 24, 2026

Maybe like this, please give it a check whether this works as intended.

@thomassedlmayer
Copy link
Copy Markdown
Contributor Author

It works for the most recent protobuf (7.34.1) and the minimally required protobuf by osi-python (6.30.2). Older versions we probably don't need to check anyways.

Should I revert the protobuf dependency again then?

Comment thread pyproject.toml Outdated
pmai added 2 commits May 7, 2026 11:00
Signed-off-by: Pierre R. Mai <pmai@pmsf.de>
@pmai
Copy link
Copy Markdown
Contributor

pmai commented May 7, 2026

CCB 2026-05-07: Merge with aligned baseline.

@pmai pmai added ReadyForMerge An issue that has been reviewed by CCB and can be merged and removed ReadyForCCBReview CCB will review it and change the status to ReadyForMerge if everything is ok labels May 7, 2026
@pmai pmai merged commit 347db91 into main May 7, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ReadyForMerge An issue that has been reviewed by CCB and can be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants