damccorm commented on code in PR #38090:
URL: https://github.com/apache/beam/pull/38090#discussion_r3053132801


##########
sdks/python/setup.py:
##########
@@ -167,23 +162,39 @@ def cythonize(*args, **kwargs):
 
 milvus_dependency = ['pymilvus>=2.5.10,<3.0.0']
 
-ml_base = [
+# google-adk / OpenTelemetry require protobuf>=5; tensorflow-transform in
+# ml_test is pinned to versions that require protobuf<5 on Python 3.10. Those

Review Comment:
   Doesn't this mean that adk tests won't run as part of the ml test suite 
though? Is there a suite where we've confirmed these do run?



##########
sdks/python/setup.py:
##########
@@ -185,7 +185,7 @@ def cythonize(*args, **kwargs):
 ]
 
 ml_adk_dependency = [
-    'google-adk==1.17.0',
+    'google-adk==1.28.1',
     'opentelemetry-api==1.37.0',
     'opentelemetry-sdk==1.37.0',
     'opentelemetry-exporter-otlp-proto-http==1.37.0',

Review Comment:
   Do you know where the opentelemetry requirement was coming from that 
installed 1.40.x? I would've expected that to be handled by pip



##########
sdks/python/setup.py:
##########
@@ -397,7 +408,10 @@ def get_portability_package_data():
           'packaging>=22.0',
           'pillow>=12.1.1,<13',
           'pymongo>=3.8.0,<5.0.0',
-          'proto-plus>=1.7.1,<2',
+          # proto-plus<1.24 requires protobuf<5; opentelemetry-proto
+          # (google-adk) needs protobuf>=5. 1.26.1 allows protobuf<7
+          # (matches Beam's cap).
+          'proto-plus>=1.26.1,<2',

Review Comment:
   If this is the case, we should just put this constraint in the adk portion 
of the extra and should keep the broader constraint here. This updates the core 
constraint which will be enforced on all users of beam



##########
sdks/python/setup.py:
##########
@@ -546,21 +560,22 @@ def get_portability_package_data():
               'datatable',
               # tensorflow-transform requires dill, but doesn't set dill as a
               # hard requirement in setup.py.
-              'dill',  # match tft extra.
+              'dill',
+              # match tft extra.
               'tensorflow_transform>=1.14.0,<1.15.0',
               # TFT->TFX-BSL require pandas 1.x, which is not compatible
               # with numpy 2.x

Review Comment:
   Why don't we need this anymore?



-- 
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