vmagamedov / grpclib

Pure-Python gRPC implementation for asyncio
http://grpclib.readthedocs.io
BSD 3-Clause "New" or "Revised" License
936 stars 92 forks source link

Compatibility with protobuf 4.21.0 #159

Open AntoniaAdler opened 2 years ago

AntoniaAdler commented 2 years ago

With protobuf update from version 3.20.1 to 4.21.0, the following error is thrown

Traceback (most recent call last):
  ...
    from grpclib.health.service import Health
  File "<repo>/lib/python3.9/site-packages/grpclib/health/service.py", line 10, in <module>
    from .v1.health_pb2 import HealthCheckRequest, HealthCheckResponse
  File "<repo>/lib/python3.9/site-packages/grpclib/health/v1/health_pb2.py", line 34, in <module>
    _descriptor.EnumValueDescriptor(
  File "<repo>/lib/python3.9/site-packages/google/protobuf/descriptor.py", line 755, in __new__
    _message.Message._CheckCalledFromGeneratedFile()
TypeError: Descriptors cannot not be created directly.
If this call came from a _pb2.py file, your generated code is out of date and must be regenerated with protoc >= 3.19.0.
If you cannot immediately regenerate your protos, some other possible workarounds are:
 1. Downgrade the protobuf package to 3.20.x or lower.
 2. Set PROTOCOL_BUFFERS_PYTHON_IMPLEMENTATION=python (but this will use pure-Python parsing and will be much slower).

More information: https://developers.google.com/protocol-buffers/docs/news/2022-05-06#python-updates

Setting the protobuf version to 3.20.1 fixes the problem.

vmagamedov commented 2 years ago

Released grpclib==0.4.3rc3 with updated *_pb2.py files

upcFrost commented 2 years ago

shouldn't protobuf be listed in the package dependencies? For example, without the protobuf package installed it's impossible to use the Healthcheck as it imports google.protobuf.

vmagamedov commented 2 years ago

@upcFrost gRPC states that protobuf is not the only way to serialise requests and replies, so in grpclib it is optional 🤷🏻‍♂️ Health checks are also optional as some other features, grpclib just wants to have some batteries to be included.

BTW, Google's grpcio also doesn't have protobuf as a dependency, only as an optional dependency.

upcFrost commented 2 years ago

Google's grpcio also doesn't have protobuf as a dependency, only as an optional dependency

Why not make it an optional dependency as well, i.e. grpcio[protobuf]? It will allow the flexibility of specifying which protobuf version are supported. For example, Google's grpcio-tools specifically say that they work with protobuf [required: >=3.12.0,<4.0dev].

Python is currently trying to move towards proper dependency resolution, with pip 22+ and tools like poetry, so imho that'd be a nice change to do, and also a pretty small one. Something like this in setup.cfg should do:

diff --git a/setup.cfg b/setup.cfg
index 3dffb2b..62cd44a 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -29,6 +29,10 @@ python_requires = >=3.7
 install_requires=
     h2<5,>=3.1.0
     multidict
+
+[options.extras_require]
+protobuf =
+    protobuf<4.21.0

 [options.entry_points]
 console_scripts =
vmagamedov commented 2 years ago

I think that Google's protobuf and grpcio projects have some problems with versioning. Two times they broke backward compatibility in protobuf.

Right now grpcio-tools requires protobuf<4.0dev,>=3.12.0, so even if grpcio-tools can work with protobuf>4 it will require a new release just to fix this.

Also right now grpcio-health-checking requires protobuf>=3.12.0 and your project will fail to import stubs from this package if you have say protobuf==3.12.2.

So I added protobuf as an optional dependency with only minimum version specified.