Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions sdks/python/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,9 +366,10 @@ def get_portability_package_data():
'dill>=0.3.1.1,<0.3.2',
'fastavro>=0.23.6,<2',
'fasteners>=0.3,<1.0',
# TODO(https://github.com/grpc/grpc/issues/37710): Unpin grpc
'grpcio>=1.33.1,<2,!=1.48.0,!=1.59.*,!=1.60.*,!=1.61.*,!=1.62.0,!=1.62.1,<1.66.0; python_version <= "3.12"', # pylint: disable=line-too-long
'grpcio>=1.67.0; python_version >= "3.13"',
# any version between 1.68.0 and 1.73.0 is bad
# external issue: https://github.com/grpc/grpc/issues/37710
'grpcio>=1.33.1,<1.74.0,!=1.48.0,!=1.59.*,!=1.60.*,!=1.61.*,!=1.62.0,!=1.62.1,!=1.68.*,!=1.69.*,!=1.70.*,!=1.71.*,!=1.72.*,!=1.73.0; python_version <= "3.12"', # pylint: disable=line-too-long
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need the <1.74.0 condition? Theoretically grpcio should have forward compatibility, and blocking this could lead to dependency conflicts. It would also be nice to allow the same versions which we allow for 3.13.

I'd also be fine waiting a few releases to see if grpcio remains stable for us though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably do no need <1.74.0 but given the wide range has caused a lot of issues for us, I would suggest we use the tighter bound with Python <= 3.12.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - I'm fine with this for now, eventually I'd like to get rid of it once we've built some confidence in this again

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would use the same upper bound for all python versions. I think it should be fine to keep the first line and remove the python_version modifier.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should check if Go tests still break with this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, we should check if Go tests still break with this.

Launched https://github.com/apache/beam/actions/runs/15907748034

'grpcio>=1.73.1; python_version >= "3.13"',
'hdfs>=2.1.0,<3.0.0',
'httplib2>=0.8,<0.23.0',
'jsonschema>=4.0.0,<5.0.0',
Expand Down
6 changes: 6 additions & 0 deletions sdks/python/tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ deps =
pylint==2.17.5
isort==4.2.15
flake8==4.0.1
# https://github.com/grpc/grpc/issues/37660: ignore pip check
commands_pre =
python --version
pip --version
# pip check
bash {toxinidir}/scripts/run_tox_cleanup.sh
commands =
pylint --version
time {toxinidir}/scripts/run_pylint.sh
Expand Down
Loading