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

feature(grpc): protobuf backward compatibility layer #3039

Closed
aarnphm opened this issue Sep 27, 2022 · 2 comments · Fixed by #3333
Closed

feature(grpc): protobuf backward compatibility layer #3039

aarnphm opened this issue Sep 27, 2022 · 2 comments · Fixed by #3333
Labels
good-first-issue Good for newcomers

Comments

@aarnphm
Copy link
Contributor

aarnphm commented Sep 27, 2022

Feature request

protobuf > 4 introduces a lot of breaking changes, therefore to ease the transition, I propose to have two sets of generated service stubs.

We lazy load all of our stubs imports via import_generated_stubs. I believe we can do one more step here:

If protobuf >4, it should be able to use the newly generated stubs structure. Otherwise, it will, by default, use the stubs generated with protobuf<4

pb, services = import_generated_stubs()

One drawback is that users will still run into issues with protobuf error if some of the library other than bentoml still uses older generated stubs.

Motivation

No response

Other

No response

@aarnphm aarnphm added the good-first-issue Good for newcomers label Sep 27, 2022
@Srivathsan-V
Copy link

Hi! Iam a newcomer to open source and would like to contribute on this issue. Could you please give me some insights into it?

@aarnphm
Copy link
Contributor Author

aarnphm commented Oct 19, 2022

Hi @Srivathsan-V, So recently protobuf>4 is a major release, which breaks all of the generated code for cases where library uses protobuf<4 (protobuf>=3.19).

Currently, BentoML generates proto stubs for our gRPC components using protobuf==3.19.6. This is fine since the majority of the library out there that uses protobuf will still be on protobuf 3 for the time being.

With that being said, if users are using protobuf >4, they wouldn't able to use gRPC features from BentoML.

So the idea here is to generate two sets of protofiles, one using protobuf==3.19.4, and the other using protobuf>4.
BentoML can provide a compact layer that check for current protobuf version

from bentoml._internal.utils.pkg import get_pkg_version

if get_pkg_version("protobuf")[0] < 4:
    # import generated stubs for protobuf==3.19.6 here
else:
    # import generated stubs from protobuf>4 here

This is where we are currently handle the generated stubs. So the check would be from here.

Another good thing to note is that the generated stubs are currently under bentoml/grpc/v1alpha1/service_pb2.py
If we introduce this compact layer, we might have to change our folder structure. But I'm open for suggestion on this as well.

If anything isn't clear, please let me know.

@aarnphm aarnphm linked a pull request Dec 8, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants