build: update protobuf to 3.19.6 and grpc to 1.48.2 #6147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
cc @vam-google FYI |
This was referenced Feb 8, 2023
nfelt
added a commit
that referenced
this pull request
Feb 8, 2023
We inadvertently resolved #6115 via #6147, so we no longer need to pin to a maximum Bazel version of 5.4.0. That said, I think it's still a good idea to retain a maximum compatible version, since Bazel major version upgrades have reliably resulted in our build breaking in the past. That way, when Bazel 7.0.0 arrives, we'll get a clean error message, and if we then try to remove the pin, we'll either be pleasantly surprised that it's fine, or we'll be expecting a build failure and know the cause (rather than encountering a build failure during a routine build, and then wasting time debugging before realizing Bazel was silently upgraded). Putting this in place now also ensures this protection extends to older commits, rather than doing it reactively, which then relies on people syncing past that commit before trying to build. Tested via `USE_BAZEL_VERSION=6.0.0 bazel run tensorboard` (w/ bazel set to bazelisk) and confirmed it builds successfully and runs apparently normally.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
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
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
…flow#6183) We inadvertently resolved tensorflow#6115 via tensorflow#6147, so we no longer need to pin to a maximum Bazel version of 5.4.0. That said, I think it's still a good idea to retain a maximum compatible version, since Bazel major version upgrades have reliably resulted in our build breaking in the past. That way, when Bazel 7.0.0 arrives, we'll get a clean error message, and if we then try to remove the pin, we'll either be pleasantly surprised that it's fine, or we'll be expecting a build failure and know the cause (rather than encountering a build failure during a routine build, and then wasting time debugging before realizing Bazel was silently upgraded). Putting this in place now also ensures this protection extends to older commits, rather than doing it reactively, which then relies on people syncing past that commit before trying to build. Tested via `USE_BAZEL_VERSION=6.0.0 bazel run tensorboard` (w/ bazel set to bazelisk) and confirmed it builds successfully and runs apparently normally.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
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
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
…flow#6183) We inadvertently resolved tensorflow#6115 via tensorflow#6147, so we no longer need to pin to a maximum Bazel version of 5.4.0. That said, I think it's still a good idea to retain a maximum compatible version, since Bazel major version upgrades have reliably resulted in our build breaking in the past. That way, when Bazel 7.0.0 arrives, we'll get a clean error message, and if we then try to remove the pin, we'll either be pleasantly surprised that it's fine, or we'll be expecting a build failure and know the cause (rather than encountering a build failure during a routine build, and then wasting time debugging before realizing Bazel was silently upgraded). Putting this in place now also ensures this protection extends to older commits, rather than doing it reactively, which then relies on people syncing past that commit before trying to build. Tested via `USE_BAZEL_VERSION=6.0.0 bazel run tensorboard` (w/ bazel set to bazelisk) and confirmed it builds successfully and runs apparently normally.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 < 4to avoid the errors seen in TensorBoard's generated pb2.py files are incompatible with protobuf 4.21.0 #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< 4but 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: