Skip to content
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

Add support for Protobuf 5 #3958

Open
2 tasks
aabmass opened this issue Jun 6, 2024 · 7 comments · May be fixed by #3931
Open
2 tasks

Add support for Protobuf 5 #3958

aabmass opened this issue Jun 6, 2024 · 7 comments · May be fixed by #3931
Labels
priority:p2 Issues that are less important than priority:p1 proto

Comments

@aabmass
Copy link
Member

aabmass commented Jun 6, 2024

Protobuf 5 was released but it currently conflicts with opentelemetry-proto package

dependencies = [
"protobuf>=3.19, < 5.0",
]

This is by design since protobuf generated code is supposed to match the runtime protobuf library version. When 4.x was released we had a lot of issues and I opened protocolbuffers/protobuf#11123 to get some clarification. We know have some docs https://protobuf.dev/support/cross-version-runtime-guarantee/ which make it clear New Gencode + Old Runtime = Never Allowed. I.e. regenerating code with grpcio-tools 5 will break compatibility with protobuf 4.

This puts us in a tricky spot of choosing which major version to support. The good news is

Starting with the 2025Q1 release, the protobuf project will adopt a rolling compatibility window for major versions..

That doesn't help right now, but hopefully will for the next major version. Because they are planning to fix things, I'm hesitant on making our own solution like keeping two copies of generated code and dynamically choosing between them at import time.


Related

@aabmass aabmass added the proto label Jun 6, 2024
@aabmass aabmass linked a pull request Jun 6, 2024 that will close this issue
6 tasks
@aabmass
Copy link
Member Author

aabmass commented Jun 6, 2024

The simplest solution is to only support a single protobuf major version. Once the ecosystem has majority moved over to Protobuf 5, we can switch over to just supporting it. I don't have a good signal on when that is.

It would helpful if people can share what version of Protobuf you are using and if this is a critical blocker for other version upgrades.

@aabmass aabmass added the priority:p2 Issues that are less important than priority:p1 label Jun 6, 2024
@aabmass aabmass linked a pull request Jun 6, 2024 that will close this issue
6 tasks
@AlexCAPS
Copy link

The new version of the authzed library already uses protobuf 5. And we are planning an update, but a dependency conflict is preventing us.

Hope for a speedy resolution

@xrmx
Copy link
Contributor

xrmx commented Jul 1, 2024

This may be interesting:
googleapis/python-api-common-protos@v1.63.1...v1.63.2

@ehiggs
Copy link
Contributor

ehiggs commented Jul 23, 2024

We know have some docs https://protobuf.dev/support/cross-version-runtime-guarantee/ which make it clear New Gencode + Old Runtime = Never Allowed. I.e. regenerating code with grpcio-tools 5 will break compatibility with protobuf 4.

Right, but can't the runtime be updated even if the generated files are old? e.g. the google cloud tools like pubsub allow anything from >=3.20.2 up to <6.0.0dev. So in this situation, if you continue to generate the files with grpcio-tools 4 and allow more modern runtimes, it should work shouldn't it?

https://github.com/googleapis/python-pubsub/blob/main/setup.py#L45

Upon investigation, it turns out that they are using proto-plus to define their messages like class Foo(proto.Message):....

@aabmass
Copy link
Member Author

aabmass commented Jul 23, 2024

Right, but can't the runtime be updated even if the generated files are old? e.g. the google cloud tools like pubsub allow anything from >=3.20.2 up to <6.0.0dev. So in this situation, if you continue to generate the files with grpcio-tools 4 and allow more modern runtimes, it should work shouldn't it?

My understanding is that, until Q1 2025 when they launch rolling compatibility, it is not guaranteed to work:

Protobuf cross-version usages outside the guarantees are error-prone and not supported. Version skews can lead to flakes and undefined behaviors that are hard to diagnose, even if it can often seem to work as long as nothing has changed in a source-incompatible way.

It is only guaranteed to work across the same minor version:

Within a single major runtime version, generated code from an older version of protoc will run on a newer runtime.

@chrismiller
Copy link

We are using protobuf 5, and just started using OpenTelemetry. Our current workaround is to have our own version of opentelemetry-python that contains protobuf 5 generated files. Not ideal, but without that OpenTelemetry would be a non-starter for us for the time being.

Yesterday though I encountered an interesting approach to this problem that might be a good solution for OpenTelemetry too. wandb generates pb2 files for all protobuf versions, then dynamically imports the appropriate one based on the protobuf runtime version, as shown in the link below:

https://github.com/wandb/wandb/blob/7c8ead46482d3beafaeb8ba9f579e056b3812a68/wandb/proto/wandb_base_pb2.py

@aabmass
Copy link
Member Author

aabmass commented Sep 20, 2024

Thanks for the data point. It's been a few months now, I think we should just upgrade to protobuf 5. Let's try to get it in for the next release.

Yesterday though I encountered an interesting approach to this problem that might be a good solution for OpenTelemetry too. wandb generates pb2 files for all protobuf versions, then dynamically imports the appropriate one based on the protobuf runtime version, as shown in the link below:

We have considered that as well, but since protobuf says they'll have rolling support in the next 6 months, I think we could just wait. Hopefully the next major version will be covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:p2 Issues that are less important than priority:p1 proto
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants