infra: set protobuf version to 3.9.2 and downgrade gRPC #5561

yatbear · 2022-02-08T22:32:33Z

tb-nightly 2.9.0a20220208 broke Keras Github CI, possibly due to the change #5528. We are depending on gRPC v1.42.0 which is currently on Protobuf 3.18.1 (commit).

We are hoping to be able to explicitly define our own com_google_protobuf dep at v3.9.2 and overwrite the gRPC one, to match with TF's protobuf version. Since gRPC release after 1.30.0 require protobuf version 3.12.0+ (grpc/grpc#23324), gRPC version is downgraded to commit b54a5b338637f92bfcf4b0bc05e0f57a5fd8fadd, and rules_apple is added as a result.

nfelt

Thanks for doing this!!

nfelt · 2022-02-09T00:07:26Z

WORKSPACE

@@ -97,6 +97,16 @@ load("@io_bazel_rules_sass//:defs.bzl", "sass_repositories")

 sass_repositories()

+http_archive(


Let's add a note here about updating this, for future reference:

We should ensure we always bump the requirements.txt protobuf dep to be >= the version here

We probably should keep this in sync with TF, even if we're no longer using the org_tensorflow workspace to build, since there are other projects (like Keras) that do, and generating protobuf code at a more recent version than the protobuf runtime supplied by TF's bazel build tooling can lead to test failures for those projects.

Thanks for the note!

nfelt · 2022-02-09T00:07:43Z

tensorboard/pip_package/requirements.txt

@@ -24,7 +24,7 @@ google-auth >= 1.6.3, < 3
 google-auth-oauthlib >= 0.4.1, < 0.5
 markdown >= 2.6.8
 numpy >= 1.12.0
-protobuf >= 3.6.0
+protobuf >= 3.12.0


Might as well just make this 3.9.2 - no reason to be more strict than necessary.

nfelt

LGTM :)

`rules_apple` needs to be added manually as a result.

nfelt

Thanks for continuing to chip away at this!

nfelt · 2022-02-09T22:31:31Z

WORKSPACE

    urls = [
-        "http://mirror.tensorflow.org/github.com/grpc/grpc/archive/v1.42.0.tar.gz",
-        "https://github.com/grpc/grpc/archive/v1.42.0.tar.gz",  # 2021-11-17
+        # Same as TF: https://github.com/tensorflow/tensorflow/blob/master/tensorflow/workspace2.bzl#L492


Maybe worth a note that we can't upgrade GRPC past 1.30.0 without also bumping protobuf?

(It's not clear that this commit that TF uses is any particular version, but it looks like it's roughly 1.27.0)

…5793) This PR allows TensorBoard to build and run with Python 3.10. There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: #5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the `patches` property of the `http_archive` rule. For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error: ``` Error in fail: Python Configuration Error: Problem getting python include path for /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python. <string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives <string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead Is the Python binary path set up right? (See ./configure or /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python.) Is distutils installed? Are Python headers installed? Try installing python-dev or python3-dev on Debian-based systems. Try python-devel or python3-devel on Redhat-based systems. ``` For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup: ``` File "/usr/local/google/home/bdubois/.cache/bazel/_bazel_bdubois/079646a57be11faea0b2bfefccb2a81a/execroot/org_tensorflow_tensorboard/bazel-out/k8-fastbuild/bin/tensorboard/dev.runfiles/com_google_protobuf/python/google/protobuf/descriptor.py", line 47, in <module> from google.protobuf.pyext import _message AttributeError: module 'collections' has no attribute 'MutableSequence' ``` The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.

Fixes #5708 and #5703. This updates our protobuf dependency to 3.19.6 in an attempt to address #5708, and provide a cleaner solution to #5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in #5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from #5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from #5561 that we only needed for gRPC

) * define our own com_google_protobuf and make protobuf minimum requirement to be 3.9.2 * downgrade gRPC to be compatible with protobuf 3.9.2 - `rules_apple` needs to be added manually as a result.

…ensorflow#5793) This PR allows TensorBoard to build and run with Python 3.10. There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: tensorflow#5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the `patches` property of the `http_archive` rule. For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error: ``` Error in fail: Python Configuration Error: Problem getting python include path for /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python. <string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives <string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead Is the Python binary path set up right? (See ./configure or /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python.) Is distutils installed? Are Python headers installed? Try installing python-dev or python3-dev on Debian-based systems. Try python-devel or python3-devel on Redhat-based systems. ``` For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup: ``` File "/usr/local/google/home/bdubois/.cache/bazel/_bazel_bdubois/079646a57be11faea0b2bfefccb2a81a/execroot/org_tensorflow_tensorboard/bazel-out/k8-fastbuild/bin/tensorboard/dev.runfiles/com_google_protobuf/python/google/protobuf/descriptor.py", line 47, in <module> from google.protobuf.pyext import _message AttributeError: module 'collections' has no attribute 'MutableSequence' ``` The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.

Fixes tensorflow#5708 and tensorflow#5703. This updates our protobuf dependency to 3.19.6 in an attempt to address tensorflow#5708, and provide a cleaner solution to tensorflow#5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in tensorflow#5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from tensorflow#5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from tensorflow#5561 that we only needed for gRPC

) * define our own com_google_protobuf and make protobuf minimum requirement to be 3.9.2 * downgrade gRPC to be compatible with protobuf 3.9.2 - `rules_apple` needs to be added manually as a result.

…ensorflow#5793) This PR allows TensorBoard to build and run with Python 3.10. There are incompatibilities in the protobuf and grpc dependencies, which are quite old but we are unable to upgrade (see: tensorflow#5561 , Googlers: http://b/219030239). We patch the necessary fixes from those projects into our older versions using the `patches` property of the `http_archive` rule. For grpc we patch grpc/grpc@dbe73c9 to avoid the following compile-time error: ``` Error in fail: Python Configuration Error: Problem getting python include path for /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python. <string>:1: DeprecationWarning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives <string>:1: DeprecationWarning: The distutils.sysconfig module is deprecated, use sysconfig instead Is the Python binary path set up right? (See ./configure or /usr/local/google/home/bdubois/virtualenv/tensorboard/bin/python.) Is distutils installed? Are Python headers installed? Try installing python-dev or python3-dev on Debian-based systems. Try python-devel or python3-devel on Redhat-based systems. ``` For protobuf we patch protocolbuffers/protobuf@9d61ead to avoid the following runtime error at startup: ``` File "/usr/local/google/home/bdubois/.cache/bazel/_bazel_bdubois/079646a57be11faea0b2bfefccb2a81a/execroot/org_tensorflow_tensorboard/bazel-out/k8-fastbuild/bin/tensorboard/dev.runfiles/com_google_protobuf/python/google/protobuf/descriptor.py", line 47, in <module> from google.protobuf.pyext import _message AttributeError: module 'collections' has no attribute 'MutableSequence' ``` The changes patched into message.cc are not the latest. I considered also patching protocolbuffers/protobuf@624d29d to remove the references to PY_MAJOR_VERSION but that change is wide in scope.

Fixes tensorflow#5708 and tensorflow#5703. This updates our protobuf dependency to 3.19.6 in an attempt to address tensorflow#5708, and provide a cleaner solution to tensorflow#5703. The choice of 3.19.6 is meant to satisfy two competing constraints: - Current Python protobuf runtimes (the 4.x series) only support generated code from protoc versions 3.19.0+, as discussed in https://protobuf.dev/news/2022-05-06/. As a result, prior to this change, TensorBoard's pip package had to force its pip package dependency to `protobuf < 4` to avoid the errors seen in tensorflow#5703. This PR lifts that restriction. - Current TensorFlow is still stuck on protobuf 3.x, the same as we have been, and as a result pins its pip package dependency using `protobuf < 3.20` (this could presumably be relaxed to `< 4` but that would require new TF releases). As a result, we must support at least one protobuf runtime version that also works with TF's constraints. Our previous attempt at this upgrade (to ~3.18 or so) caused test failures for Keras (which depends on TB, via TF, for the summary API code), apparently due to a protobuf runtime that was too old for our generated code. At the time, this was puzzling because they were pip-installing a protobuf runtime version that should have been recent enough - but I suspect now that this was a red herring, and bazel test was actually getting the protobuf runtime from the protobuf build dependency, not from the installed Python packages. If we see this failure mode again, we'll have to get Keras to update the protobuf Python runtime available in bazel tests. Lastly, this upgrade lets us clean up some additional issues we had to work around: - We can also upgrade gRPC now, to 1.48.2. I selected this version since it appears to be the most recent version prior to gRPC adopting protobuf 4.x (see grpc/grpc@41ec08c) - We can revert the backported fixes to protobuf and grpc from tensorflow#5793, since the upgraded dependencies don't require patching - We can back out rules_apple reintroduction from tensorflow#5561 that we only needed for gRPC

yatbear added 2 commits February 8, 2022 22:12

upgrade protobuf to 3.12.0

d84464c

define our own com_google_protobuf

e0b7892

yatbear added the dependencies Pull requests that update a dependency file label Feb 8, 2022

yatbear requested a review from nfelt February 8, 2022 23:35

move com_google_protobuf above com_github_grpc_grpc

3fba8b4

nfelt approved these changes Feb 9, 2022

View reviewed changes

yatbear added 2 commits February 9, 2022 00:12

fix sha256 for com_google_protobuf

a0d5c27

make protobuf minimum requirement to be 3.9.2

96c2e57

nfelt reviewed Feb 9, 2022

View reviewed changes

yatbear added 2 commits February 9, 2022 20:44

downgrade gRPC to be compatible with protobuf 3.9.2

7bc474d

`rules_apple` needs to be added manually as a result.

update protos

ad2d3c6

yatbear changed the title ~~infra: add com_google_protobuf dep at v3.9.2~~ infra: add com_google_protobuf dep at v3.9.2 and downgrade gRPC Feb 9, 2022

yatbear requested a review from nfelt February 9, 2022 21:11

yatbear changed the title ~~infra: add com_google_protobuf dep at v3.9.2 and downgrade gRPC~~ infra: set protobuf version to 3.9.2 and downgrade gRPC Feb 9, 2022

nfelt approved these changes Feb 9, 2022

View reviewed changes

add a note

009c655

yatbear merged commit f7c5484 into tensorflow:master Feb 10, 2022

yatbear deleted the we-broke-keras branch February 10, 2022 00:27

nfelt mentioned this pull request May 17, 2022

tracking issue for upgrading protobuf to 3.19.0+ (and gRPC as well) #5708

Closed

bmd3k mentioned this pull request Jul 6, 2022

Patch protobuf and grpc dependencies to be Python 3.10 compatible. #5793

Merged

nfelt mentioned this pull request Jan 14, 2023

build: update protobuf to 3.19.6 and grpc to 1.48.2 #6147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

infra: set protobuf version to 3.9.2 and downgrade gRPC #5561

infra: set protobuf version to 3.9.2 and downgrade gRPC #5561

yatbear commented Feb 8, 2022 •

edited

Loading

nfelt left a comment

nfelt Feb 9, 2022

yatbear Feb 9, 2022

nfelt Feb 9, 2022

yatbear Feb 9, 2022

nfelt left a comment

nfelt left a comment

nfelt Feb 9, 2022

yatbear Feb 9, 2022

		@@ -97,6 +97,16 @@ load("@io_bazel_rules_sass//:defs.bzl", "sass_repositories")

		sass_repositories()

		http_archive(

infra: set protobuf version to 3.9.2 and downgrade gRPC #5561

infra: set protobuf version to 3.9.2 and downgrade gRPC #5561

Conversation

yatbear commented Feb 8, 2022 • edited Loading

nfelt left a comment

Choose a reason for hiding this comment

nfelt Feb 9, 2022

Choose a reason for hiding this comment

yatbear Feb 9, 2022

Choose a reason for hiding this comment

nfelt Feb 9, 2022

Choose a reason for hiding this comment

yatbear Feb 9, 2022

Choose a reason for hiding this comment

nfelt left a comment

Choose a reason for hiding this comment

nfelt left a comment

Choose a reason for hiding this comment

nfelt Feb 9, 2022

Choose a reason for hiding this comment

yatbear Feb 9, 2022

Choose a reason for hiding this comment

yatbear commented Feb 8, 2022 •

edited

Loading