dianfu commented on code in PR #28059:
URL: https://github.com/apache/flink/pull/28059#discussion_r3763903487


##########
flink-python/README.md:
##########
@@ -39,7 +39,7 @@ python pyflink/gen_protos.py
 ```
 
 PyFlink depends on the following libraries to execute the above script:
-1. grpcio-tools (>=1.29.0,<=1.71.0)
+1. grpcio-tools (>=1.29.0,<2)

Review Comment:
   1.29.0 -> 1.33.1



##########
flink-python/setup.py:
##########
@@ -319,10 +319,10 @@ def extracted_output_files(base_dir, file_path, 
output_directory):
         'pyflink.bin': ['*']}
 
     install_requires = ['py4j==0.10.9.7', 'python-dateutil>=2.8.0,<3',
-                        'apache-beam>=2.54.0,<=2.61.0',
+                        'apache-beam>=2.54.0,<3.0.0',

Review Comment:
   We'd better limit the upper bound of apache-beam. There is no backward 
compatibility guarantee‌. 



##########
flink-python/setup.py:
##########
@@ -319,10 +319,10 @@ def extracted_output_files(base_dir, file_path, 
output_directory):
         'pyflink.bin': ['*']}
 
     install_requires = ['py4j==0.10.9.7', 'python-dateutil>=2.8.0,<3',
-                        'apache-beam>=2.54.0,<=2.61.0',
+                        'apache-beam>=2.54.0,<3.0.0',

Review Comment:
   The protobuf>=6.31.1 requirement makes Beam 2.54–2.68 impossible to install: 
Beam 2.54 requires protobuf<4.26 and Beam 2.68 still requires protobuf<6. The 
effective minimum is Beam 2.69, so update this bound and the corresponding 
documentation, or relax the protobuf constraint



##########
flink-python/setup.py:
##########
@@ -319,10 +319,10 @@ def extracted_output_files(base_dir, file_path, 
output_directory):
         'pyflink.bin': ['*']}
 
     install_requires = ['py4j==0.10.9.7', 'python-dateutil>=2.8.0,<3',
-                        'apache-beam>=2.54.0,<=2.61.0',
+                        'apache-beam>=2.54.0,<3.0.0',
                         'cloudpickle>=2.2.0', 'avro>=1.12.0',
                         'pytz>=2018.3', 'fastavro>=1.1.0,!=1.8.0', 
'requests>=2.26.0',
-                        'protobuf>=3.19.0',
+                        'protobuf>=6.31.1,<7.0.0.dev0',

Review Comment:
   It accepts protobuf 6.31.1, but flink_fn_execution_pb2.py was regenerated 
with 6.33.5 and validates that exact minimum during import.



##########
flink-python/pyproject.toml:
##########
@@ -43,11 +43,11 @@ dev = [
   "pytz>=2018.3",
   "numpy>=1.22.4,<2.3.0",
   "fastavro>=1.1.0,!=1.8.0",
-  "grpcio>=1.29.0,<=1.71.0",
-  "grpcio-tools>=1.29.0,<=1.71.0",
+  "grpcio>=1.33.1,<2",
+  "grpcio-tools>=1.33.1,<2",

Review Comment:
   Could we pin grpcio-tools to 1.80.0 here, and use the same exact version in 
gen_protos.py and the README, instead of accepting a version range?
   
   grpcio-tools is a code generator rather than an ordinary runtime dependency: 
it bundles a particular protoc version, and changing it can change the 
checked-in _pb2.py/_pb2.pyi files as well as their Protobuf runtime-version 
guard. 
   
   1.80.0 is the best fit for the compatibility contract currently declared by 
this PR. It is the latest grpcio-tools release that still supports Python 3.9, 
and it bundles libprotoc 31.1, which generates code requiring Protobuf 
6.31.1—exactly matching the lower bound in setup.py. In contrast, 1.81.1 
requires Python 3.10+ and generates code requiring Protobuf 6.33.5, while 
1.82.1 and later require Protobuf 7 and therefore conflict with the current 
protobuf<7 constraint. Please pin grpcio-tools==1.80.0 and regenerate the 
checked-in protobuf files with that version.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to