From 17f905b8025bfc249d161596e8ad39e80cf2d945 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 7 Feb 2025 09:17:08 -0800 Subject: [PATCH 1/5] [Http2PingRatePolicy] Fix flaky test (#38687) Fix flakes of the kind - ``` [ RUN ] PingRatePolicy.ClientThrottledUntilDataSent test/core/transport/chttp2/ping_rate_policy_test.cc:93: Failure Expected equality of these values: std::get(result).wait Which is: 59999ms Duration::Minutes(1) Which is: 60000ms ``` https://btx.cloud.google.com/invocations/ecb28c3d-c5cd-4337-864f-cc241fe1f243/targets/%2F%2Ftest%2Fcore%2Ftransport%2Fchttp2:ping_rate_policy_test;config=d1d796853829b38520a1cb7c8e7476c3b09606cb23d8d1c16b5e6044aa588efb/log Closes #38687 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/38687 from yashykt:FixFlakyPingPolicy 6849a62e6b6dd183182e0a83332dde12fc900af4 PiperOrigin-RevId: 724368757 --- .../transport/chttp2/ping_rate_policy_test.cc | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/core/transport/chttp2/ping_rate_policy_test.cc b/test/core/transport/chttp2/ping_rate_policy_test.cc index 8bae8680fd1a4..ca30eb3114f4c 100644 --- a/test/core/transport/chttp2/ping_rate_policy_test.cc +++ b/test/core/transport/chttp2/ping_rate_policy_test.cc @@ -17,12 +17,16 @@ #include #include +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "src/core/lib/experiments/experiments.h" +#include "test/core/test_util/test_config.h" namespace grpc_core { namespace { +using ::testing::PrintToString; + Chttp2PingRatePolicy::RequestSendPingResult SendGranted() { return Chttp2PingRatePolicy::SendGranted{}; } @@ -64,6 +68,12 @@ TEST(PingRatePolicy, ClientBlockedUntilDataSent) { EXPECT_EQ(policy.RequestSendPing(Duration::Zero(), 0), TooManyRecentPings()); } +MATCHER_P2(IsWithinRange, lo, hi, + absl::StrCat(negation ? "isn't" : "is", " between ", + PrintToString(lo), " and ", PrintToString(hi))) { + return lo <= arg && arg <= hi; +} + TEST(PingRatePolicy, ClientThrottledUntilDataSent) { if (!IsMaxPingsWoDataThrottleEnabled()) { GTEST_SKIP() @@ -77,8 +87,8 @@ TEST(PingRatePolicy, ClientThrottledUntilDataSent) { // Second ping is throttled since no data has been sent. auto result = policy.RequestSendPing(Duration::Zero(), 0); EXPECT_TRUE(std::holds_alternative(result)); - EXPECT_EQ(std::get(result).wait, - Duration::Minutes(1)); + EXPECT_THAT(std::get(result).wait, + IsWithinRange(Duration::Seconds(59), Duration::Minutes(1))); policy.ResetPingsBeforeDataRequired(); // After resetting pings before data required (data sent), we can send pings // without being throttled. @@ -89,8 +99,8 @@ TEST(PingRatePolicy, ClientThrottledUntilDataSent) { // After reaching limit, we are throttled again. result = policy.RequestSendPing(Duration::Zero(), 0); EXPECT_TRUE(std::holds_alternative(result)); - EXPECT_EQ(std::get(result).wait, - Duration::Minutes(1)); + EXPECT_THAT(std::get(result).wait, + IsWithinRange(Duration::Seconds(59), Duration::Minutes(1))); } TEST(PingRatePolicy, RateThrottlingWorks) { From 1f97f7540151294ec5704c604811f173dd68988d Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Fri, 7 Feb 2025 09:17:20 -0800 Subject: [PATCH 2/5] [CI] Used VS2022 for Windows/build_artifact_protoc (#38697) gRPC stopped supporting VS2019 so all artifacts need to be built with VS2022 on Windows. Closes #38697 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/38697 from veblush:win-vs170 bed7a42f9ff0ee95249e8a60ce97cad089c22c89 PiperOrigin-RevId: 724368810 --- tools/run_tests/artifacts/build_artifact_protoc.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/artifacts/build_artifact_protoc.bat b/tools/run_tests/artifacts/build_artifact_protoc.bat index 2ab49dd406053..8ba1b28427eca 100644 --- a/tools/run_tests/artifacts/build_artifact_protoc.bat +++ b/tools/run_tests/artifacts/build_artifact_protoc.bat @@ -29,7 +29,7 @@ if "%GRPC_PROTOC_BUILD_COMPILER_JOBS%"=="" ( @rem set cl.exe build environment to build with VS2019 tooling @rem this is required for Ninja build to work -call "%VS160COMNTOOLS%..\..\VC\Auxiliary\Build\vcvarsall.bat" %ARCHITECTURE% +call "%VS170COMNTOOLS%..\..\VC\Auxiliary\Build\vcvarsall.bat" %ARCHITECTURE% @rem restore command echo echo on From 68b5dc5125dc65bfe2b6e6f888f7fc3aaed16d60 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Fri, 7 Feb 2025 12:07:19 -0800 Subject: [PATCH 3/5] [Deps] Removed vendored python deps from Bazel builds (#38692) With the Bazel build transitioning to BzlMod, all vendored Bazel Python targets must be removed. These targets are not available in the BCR and need be managed via a pip `requirements.bazel.txt` file. gRPC already uses this approach, so it is going to be extended to include those target. (e.g. see [test utils](https://github.com/grpc/grpc/blob/e06ad82c3fdde9ec6598d314998d88e848f4c577/test/cpp/naming/utils/BUILD#L27-L45) to understand how those targets are used) Additionally, the generation of the `requirements.bazel.lock` file has been improved. Because this file is a lock file, including all transitive dependencies, manual maintenance is not managable. Its new source file, `requirements.bazel.txt` is created to list only the direct dependencies used by gRPC, along with instructions for generating the full lock file. This source file omits specific version requirements to use the latest available versions, but version constraints can be added as needed. Closes #38692 PiperOrigin-RevId: 724427578 --- .github/labeler.yml | 1 + MODULE.bazel | 2 +- WORKSPACE | 2 +- bazel/grpc_deps.bzl | 60 --------- requirements.bazel.lock | 116 ++++++++++++++++++ requirements.bazel.txt | 82 ++++++------- templates/MODULE.bazel.template | 2 +- test/cpp/naming/utils/BUILD | 12 +- third_party/BUILD | 12 -- third_party/constantly.BUILD | 7 -- third_party/incremental.BUILD | 10 -- third_party/twisted.BUILD | 15 --- third_party/yaml.BUILD | 10 -- third_party/zope_interface.BUILD | 13 -- tools/distrib/python/docgen.py | 2 +- .../run_tests/sanity/check_bazel_workspace.py | 17 --- 16 files changed, 167 insertions(+), 196 deletions(-) create mode 100644 requirements.bazel.lock delete mode 100644 third_party/constantly.BUILD delete mode 100644 third_party/incremental.BUILD delete mode 100644 third_party/twisted.BUILD delete mode 100644 third_party/yaml.BUILD delete mode 100644 third_party/zope_interface.BUILD diff --git a/.github/labeler.yml b/.github/labeler.yml index b0070de633771..15fa2d9e07d9f 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -46,6 +46,7 @@ lang/python: - any-glob-to-any-file: - bazel/python_rules.bzl - examples/python/** + - requirements.bazel.lock - requirements.bazel.txt - src/compiler/python* - all-globs-to-any-file: diff --git a/MODULE.bazel b/MODULE.bazel index d3c3a8342b0be..945dc6b217302 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -85,7 +85,7 @@ pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip") pip.parse( hub_name = "grpc_python_dependencies", python_version = python_version, - requirements_lock = "//:requirements.bazel.txt", + requirements_lock = "//:requirements.bazel.lock", ) for python_version in PYTHON_VERSIONS ] diff --git a/WORKSPACE b/WORKSPACE index 691f983ed728d..3d02cba5e877f 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -46,7 +46,7 @@ load("@rules_python//python:pip.bzl", "pip_parse") pip_parse( name = "grpc_python_dependencies", - requirements_lock = "@com_github_grpc_grpc//:requirements.bazel.txt", + requirements_lock = "@com_github_grpc_grpc//:requirements.bazel.lock", ) load("@grpc_python_dependencies//:requirements.bzl", "install_deps") diff --git a/bazel/grpc_deps.bzl b/bazel/grpc_deps.bzl index 211f72690bc61..0cde8ed64ae47 100644 --- a/bazel/grpc_deps.bzl +++ b/bazel/grpc_deps.bzl @@ -368,66 +368,6 @@ def grpc_test_only_deps(): Loads dependencies that are only needed to run grpc library's tests. """ - if "com_github_twisted_twisted" not in native.existing_rules(): - http_archive( - name = "com_github_twisted_twisted", - sha256 = "ca17699d0d62eafc5c28daf2c7d0a18e62ae77b4137300b6c7d7868b39b06139", - strip_prefix = "twisted-twisted-17.5.0", - urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/twisted/twisted/archive/twisted-17.5.0.zip", - "https://github.com/twisted/twisted/archive/twisted-17.5.0.zip", - ], - build_file = "@com_github_grpc_grpc//third_party:twisted.BUILD", - ) - - if "com_github_yaml_pyyaml" not in native.existing_rules(): - http_archive( - name = "com_github_yaml_pyyaml", - sha256 = "e34d97db6d846f5e2ad51417fd646e7ce6a3a70726ccea2a857e0580a7155f39", - strip_prefix = "pyyaml-6.0.1", - urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/yaml/pyyaml/archive/6.0.1.zip", - "https://github.com/yaml/pyyaml/archive/6.0.1.zip", - ], - build_file = "@com_github_grpc_grpc//third_party:yaml.BUILD", - ) - - if "com_github_twisted_incremental" not in native.existing_rules(): - http_archive( - name = "com_github_twisted_incremental", - sha256 = "f0ca93359ee70243ff7fbf2d904a6291810bd88cb80ed4aca6fa77f318a41a36", - strip_prefix = "incremental-incremental-17.5.0", - urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/twisted/incremental/archive/incremental-17.5.0.zip", - "https://github.com/twisted/incremental/archive/incremental-17.5.0.zip", - ], - build_file = "@com_github_grpc_grpc//third_party:incremental.BUILD", - ) - - if "com_github_zopefoundation_zope_interface" not in native.existing_rules(): - http_archive( - name = "com_github_zopefoundation_zope_interface", - sha256 = "e9579fc6149294339897be3aa9ecd8a29217c0b013fe6f44fcdae00e3204198a", - strip_prefix = "zope.interface-4.4.3", - urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/zopefoundation/zope.interface/archive/4.4.3.zip", - "https://github.com/zopefoundation/zope.interface/archive/4.4.3.zip", - ], - build_file = "@com_github_grpc_grpc//third_party:zope_interface.BUILD", - ) - - if "com_github_twisted_constantly" not in native.existing_rules(): - http_archive( - name = "com_github_twisted_constantly", - sha256 = "2702cd322161a579d2c0dbf94af4e57712eedc7bd7bbbdc554a230544f7d346c", - strip_prefix = "constantly-15.1.0", - urls = [ - "https://storage.googleapis.com/grpc-bazel-mirror/github.com/twisted/constantly/archive/15.1.0.zip", - "https://github.com/twisted/constantly/archive/15.1.0.zip", - ], - build_file = "@com_github_grpc_grpc//third_party:constantly.BUILD", - ) - if "com_google_libprotobuf_mutator" not in native.existing_rules(): http_archive( name = "com_google_libprotobuf_mutator", diff --git a/requirements.bazel.lock b/requirements.bazel.lock new file mode 100644 index 0000000000000..94be90bc6cd6d --- /dev/null +++ b/requirements.bazel.lock @@ -0,0 +1,116 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# pip-compile --allow-unsafe requirements.bazel.in +# +absl-py==2.1.0 + # via -r requirements.bazel.in +attrs==25.1.0 + # via twisted +automat==24.8.1 + # via twisted +cachetools==5.5.1 + # via google-auth +certifi==2025.1.31 + # via + # -r requirements.bazel.in + # requests +chardet==5.2.0 + # via -r requirements.bazel.in +charset-normalizer==3.4.1 + # via requests +constantly==23.10.4 + # via twisted +deprecated==1.2.18 + # via + # opentelemetry-api + # opentelemetry-semantic-conventions +gevent==24.2.1 + # via -r requirements.bazel.in +google-auth==2.38.0 + # via -r requirements.bazel.in +googleapis-common-protos==1.66.0 + # via -r requirements.bazel.in +greenlet==3.1.1 + # via gevent +hyperlink==21.0.0 + # via twisted +idna==3.10 + # via + # -r requirements.bazel.in + # hyperlink + # requests +importlib-metadata==8.5.0 + # via opentelemetry-api +incremental==24.7.2 + # via twisted +opentelemetry-api==1.30.0 + # via + # -r requirements.bazel.in + # opentelemetry-exporter-prometheus + # opentelemetry-resourcedetector-gcp + # opentelemetry-sdk + # opentelemetry-semantic-conventions +opentelemetry-exporter-prometheus==0.51b0 + # via -r requirements.bazel.in +opentelemetry-resourcedetector-gcp==1.9.0a0 + # via -r requirements.bazel.in +opentelemetry-sdk==1.30.0 + # via + # -r requirements.bazel.in + # opentelemetry-exporter-prometheus + # opentelemetry-resourcedetector-gcp +opentelemetry-semantic-conventions==0.51b0 + # via opentelemetry-sdk +prometheus-client==0.21.1 + # via opentelemetry-exporter-prometheus +protobuf==5.29.3 + # via + # -r requirements.bazel.in + # googleapis-common-protos +pyasn1==0.6.1 + # via + # pyasn1-modules + # rsa +pyasn1-modules==0.4.1 + # via google-auth +pyyaml==6.0.2 + # via -r requirements.bazel.in +requests==2.32.3 + # via + # -r requirements.bazel.in + # opentelemetry-resourcedetector-gcp +rsa==4.9 + # via google-auth +tomli==2.2.1 + # via incremental +twisted==24.11.0 + # via -r requirements.bazel.in +typing-extensions==4.12.2 + # via + # automat + # opentelemetry-resourcedetector-gcp + # opentelemetry-sdk + # twisted +urllib3==2.2.3 + # via + # -r requirements.bazel.in + # requests +wrapt==1.17.2 + # via deprecated +zipp==3.20.2 + # via importlib-metadata +zope-event==5.0 + # via gevent +zope-interface==7.2 + # via + # gevent + # twisted + +# The following packages are considered to be unsafe in a requirements file: +setuptools==75.3.0 + # via + # incremental + # zope-event + # zope-interface diff --git a/requirements.bazel.txt b/requirements.bazel.txt index 373e74322c175..98e5a70171960 100644 --- a/requirements.bazel.txt +++ b/requirements.bazel.txt @@ -1,41 +1,41 @@ -# GRPC Python setup requirements -# The requirements listed below are solely for Bazel tests and should not be used elsewhere. -absl-py==1.4.0 -cachetools==5.3.2 -certifi==2023.7.22 -chardet==3.0.4 -charset-normalizer==3.3.2 -coverage==4.5.4 -cython==3.0.0 -Deprecated==1.2.14 -gevent==22.08.0 -google-api-core==2.24.0 -google-auth==2.23.4 -google-cloud-monitoring==2.26.0 -google-cloud-trace==1.15.0 -googleapis-common-protos==1.63.1 -greenlet==1.1.3.post0 -idna==2.7 -importlib-metadata==6.11.0 -oauth2client==4.1.0 -opencensus-context==0.1.3 -opentelemetry-api==1.25.0 -opentelemetry-exporter-prometheus==0.46b0 -opentelemetry-resourcedetector-gcp==1.6.0a0 -opentelemetry-sdk==1.25.0 -opentelemetry-semantic-conventions==0.46b0 -prometheus_client==0.20.0 -proto-plus==1.25.0 -protobuf>=5.27.1,<6.0dev -pyasn1-modules==0.3.0 -pyasn1==0.5.0 -requests==2.25.1 -rsa==4.9 -setuptools==44.1.1 -typing-extensions==4.9.0 -urllib3==1.26.18 -wheel==0.38.1 -wrapt==1.16.0 -zipp==3.17.0 -zope.event==4.5.0 -zope.interface==6.1 +# Copyright 2025 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# This file provides the source for generating requirements.bazel.lock. +# Instructions for generation follow: +# Use the oldest supported version of Python +# +# $ docker run -it --rm -v $(pwd):/grpc python:3.8 /bin/bash +# # cd grpc +# # pip install pip-tools +# # pip-compile --allow-unsafe requirements.bazel.txt > requirements.bazel.lock +# # exit +# $ sudo chown $USER requirements.bazel.lock + +absl-py +certifi +chardet +gevent +google-auth +googleapis-common-protos +idna +opentelemetry-api +opentelemetry-exporter-prometheus +opentelemetry-resourcedetector-gcp +opentelemetry-sdk +protobuf +pyyaml # for DNS test +requests +twisted # for DNS test +urllib3 diff --git a/templates/MODULE.bazel.template b/templates/MODULE.bazel.template index 57ab319e5b517..04e943c0afbe2 100644 --- a/templates/MODULE.bazel.template +++ b/templates/MODULE.bazel.template @@ -87,7 +87,7 @@ pip.parse( hub_name = "grpc_python_dependencies", python_version = python_version, - requirements_lock = "//:requirements.bazel.txt", + requirements_lock = "//:requirements.bazel.lock", ) for python_version in PYTHON_VERSIONS ] diff --git a/test/cpp/naming/utils/BUILD b/test/cpp/naming/utils/BUILD index 37f497ffe2ada..95d1686fcf639 100644 --- a/test/cpp/naming/utils/BUILD +++ b/test/cpp/naming/utils/BUILD @@ -28,21 +28,19 @@ grpc_py_binary( name = "dns_server", testonly = True, srcs = ["dns_server.py"], - external_deps = [ - "twisted", - "yaml", - ], python_version = "PY3", + deps = [ + requirement("twisted"), + requirement("pyyaml"), + ], ) grpc_py_binary( name = "dns_resolver", testonly = True, srcs = ["dns_resolver.py"], - external_deps = [ - "twisted", - ], python_version = "PY3", + deps = [requirement("twisted")], ) grpc_py_binary( diff --git a/third_party/BUILD b/third_party/BUILD index 9d0c08258746a..f13f4c875e2a8 100644 --- a/third_party/BUILD +++ b/third_party/BUILD @@ -208,15 +208,3 @@ alias( actual = "@com_google_googleapis//google/logging/v2:logging_cc_proto", tags = ["manual"], ) - -alias( - name = "twisted", - actual = "@com_github_twisted_twisted//:twisted", - tags = ["manual"], -) - -alias( - name = "yaml", - actual = "@com_github_yaml_pyyaml//:yaml", - tags = ["manual"], -) diff --git a/third_party/constantly.BUILD b/third_party/constantly.BUILD deleted file mode 100644 index f1f93cb79fc23..0000000000000 --- a/third_party/constantly.BUILD +++ /dev/null @@ -1,7 +0,0 @@ -py_library( - name = "constantly", - srcs = glob(["constantly/*.py"]), - visibility = [ - "//visibility:public", - ], -) diff --git a/third_party/incremental.BUILD b/third_party/incremental.BUILD deleted file mode 100644 index f2a06b49dc406..0000000000000 --- a/third_party/incremental.BUILD +++ /dev/null @@ -1,10 +0,0 @@ -py_library( - name = "incremental", - srcs = glob(["src/incremental/*.py"]), - imports = [ - "src", - ], - visibility = [ - "//visibility:public", - ], -) diff --git a/third_party/twisted.BUILD b/third_party/twisted.BUILD deleted file mode 100644 index 5005c5a3f8c2d..0000000000000 --- a/third_party/twisted.BUILD +++ /dev/null @@ -1,15 +0,0 @@ -py_library( - name = "twisted", - srcs = glob(["src/twisted/**/*.py"]), - imports = [ - "src", - ], - visibility = [ - "//visibility:public", - ], - deps = [ - "@com_github_twisted_incremental//:incremental", - "@com_github_twisted_constantly//:constantly", - "@com_github_zopefoundation_zope_interface//:zope_interface", - ], -) diff --git a/third_party/yaml.BUILD b/third_party/yaml.BUILD deleted file mode 100644 index f88854c5719b8..0000000000000 --- a/third_party/yaml.BUILD +++ /dev/null @@ -1,10 +0,0 @@ -py_library( - name = "yaml", - srcs = glob(["lib/yaml/*.py"]), - imports = [ - "lib", - ], - visibility = [ - "//visibility:public", - ], -) diff --git a/third_party/zope_interface.BUILD b/third_party/zope_interface.BUILD deleted file mode 100644 index b7b8d1e6cf130..0000000000000 --- a/third_party/zope_interface.BUILD +++ /dev/null @@ -1,13 +0,0 @@ -py_library( - name = "zope_interface", - srcs = glob([ - "src/zope/interface/*.py", - "src/zope/interface/common/*.py", - ]), - imports = [ - "src", - ], - visibility = [ - "//visibility:public", - ], -) diff --git a/tools/distrib/python/docgen.py b/tools/distrib/python/docgen.py index 00d00940e73bc..e7c02416332c0 100755 --- a/tools/distrib/python/docgen.py +++ b/tools/distrib/python/docgen.py @@ -38,7 +38,7 @@ PROJECT_ROOT = os.path.abspath(os.path.join(SCRIPT_DIR, "..", "..", "..")) SETUP_PATH = os.path.join(PROJECT_ROOT, "setup.py") -REQUIREMENTS_PATH = os.path.join(PROJECT_ROOT, "requirements.bazel.txt") +REQUIREMENTS_PATH = os.path.join(PROJECT_ROOT, "requirements.bazel.lock") DOC_PATH = os.path.join(PROJECT_ROOT, "doc/build") if "VIRTUAL_ENV" in os.environ: diff --git a/tools/run_tests/sanity/check_bazel_workspace.py b/tools/run_tests/sanity/check_bazel_workspace.py index 122e13031919e..373bc4f81ef32 100755 --- a/tools/run_tests/sanity/check_bazel_workspace.py +++ b/tools/run_tests/sanity/check_bazel_workspace.py @@ -38,13 +38,6 @@ _BAZEL_SKYLIB_DEP_NAME = "bazel_skylib" _BAZEL_TOOLCHAINS_DEP_NAME = "bazel_toolchains" _BAZEL_COMPDB_DEP_NAME = "bazel_compdb" -_TWISTED_TWISTED_DEP_NAME = "com_github_twisted_twisted" -_YAML_PYYAML_DEP_NAME = "com_github_yaml_pyyaml" -_TWISTED_INCREMENTAL_DEP_NAME = "com_github_twisted_incremental" -_ZOPEFOUNDATION_ZOPE_INTERFACE_DEP_NAME = ( - "com_github_zopefoundation_zope_interface" -) -_TWISTED_CONSTANTLY_DEP_NAME = "com_github_twisted_constantly" _GRPC_DEP_NAMES = [ "platforms", @@ -63,11 +56,6 @@ _BAZEL_SKYLIB_DEP_NAME, _BAZEL_TOOLCHAINS_DEP_NAME, _BAZEL_COMPDB_DEP_NAME, - _TWISTED_TWISTED_DEP_NAME, - _YAML_PYYAML_DEP_NAME, - _TWISTED_INCREMENTAL_DEP_NAME, - _ZOPEFOUNDATION_ZOPE_INTERFACE_DEP_NAME, - _TWISTED_CONSTANTLY_DEP_NAME, "bazel_features", "rules_proto", "io_bazel_rules_go", @@ -94,11 +82,6 @@ _BAZEL_SKYLIB_DEP_NAME, _BAZEL_TOOLCHAINS_DEP_NAME, _BAZEL_COMPDB_DEP_NAME, - _TWISTED_TWISTED_DEP_NAME, - _YAML_PYYAML_DEP_NAME, - _TWISTED_INCREMENTAL_DEP_NAME, - _ZOPEFOUNDATION_ZOPE_INTERFACE_DEP_NAME, - _TWISTED_CONSTANTLY_DEP_NAME, "bazel_features", "rules_proto", "io_bazel_rules_go", From f73fad26263334013f71669c1659831115540a21 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Fri, 7 Feb 2025 13:05:06 -0800 Subject: [PATCH 4/5] [xDS] use DownCast<> where appropriate (#38700) Also remove some unnecessary includes of port_platform.h, which have not been necessary since #36234. Closes #38700 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/38700 from markdroth:xds_down_cast 002ffd4fbe045825cc1c046d63ead0bd0a4b1747 PiperOrigin-RevId: 724446450 --- src/core/BUILD | 2 ++ .../xds/grpc/file_watcher_certificate_provider_factory.cc | 4 ++-- src/core/xds/grpc/xds_bootstrap_grpc.cc | 4 ++-- src/core/xds/grpc/xds_client_grpc.cc | 4 ++-- src/core/xds/grpc/xds_cluster_parser.cc | 5 +++-- src/core/xds/grpc/xds_common_types_parser.cc | 6 +++--- src/core/xds/grpc/xds_http_rbac_filter.cc | 4 ++-- src/core/xds/grpc/xds_listener_parser.cc | 4 ++-- src/core/xds/grpc/xds_listener_parser.h | 3 ++- src/core/xds/grpc/xds_route_config_parser.cc | 6 +++--- src/core/xds/grpc/xds_route_config_parser.h | 4 ++-- src/core/xds/grpc/xds_server_grpc.cc | 3 ++- src/core/xds/grpc/xds_transport_grpc.cc | 6 +++--- src/core/xds/xds_client/xds_resource_type_impl.h | 6 +++--- 14 files changed, 33 insertions(+), 28 deletions(-) diff --git a/src/core/BUILD b/src/core/BUILD index 6539855751d29..f559238682a23 100644 --- a/src/core/BUILD +++ b/src/core/BUILD @@ -5441,6 +5441,7 @@ grpc_cc_library( tags = ["nofixdeps"], deps = [ "certificate_provider_factory", + "down_cast", "grpc_tls_credentials", "json", "json_args", @@ -5580,6 +5581,7 @@ grpc_cc_library( tags = ["nofixdeps"], deps = [ "channel_creds_registry", + "down_cast", "json", "json_args", "json_object_loader", diff --git a/src/core/xds/grpc/file_watcher_certificate_provider_factory.cc b/src/core/xds/grpc/file_watcher_certificate_provider_factory.cc index f668d3f381226..8b08403c1b470 100644 --- a/src/core/xds/grpc/file_watcher_certificate_provider_factory.cc +++ b/src/core/xds/grpc/file_watcher_certificate_provider_factory.cc @@ -18,7 +18,6 @@ #include "src/core/xds/grpc/file_watcher_certificate_provider_factory.h" -#include #include #include @@ -31,6 +30,7 @@ #include "absl/strings/str_join.h" #include "src/core/config/core_configuration.h" #include "src/core/lib/security/credentials/tls/grpc_tls_certificate_provider.h" +#include "src/core/util/down_cast.h" namespace grpc_core { @@ -119,7 +119,7 @@ FileWatcherCertificateProviderFactory::CreateCertificateProvider( return nullptr; } auto* file_watcher_config = - static_cast(config.get()); + DownCast(config.get()); return MakeRefCounted( file_watcher_config->private_key_file(), file_watcher_config->identity_cert_file(), diff --git a/src/core/xds/grpc/xds_bootstrap_grpc.cc b/src/core/xds/grpc/xds_bootstrap_grpc.cc index 9334dbbe4edd6..a4e4986cba2fa 100644 --- a/src/core/xds/grpc/xds_bootstrap_grpc.cc +++ b/src/core/xds/grpc/xds_bootstrap_grpc.cc @@ -17,7 +17,6 @@ #include "src/core/xds/grpc/xds_bootstrap_grpc.h" #include -#include #include #include @@ -32,6 +31,7 @@ #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/string_view.h" +#include "src/core/util/down_cast.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_object_loader.h" #include "src/core/util/json/json_reader.h" @@ -200,7 +200,7 @@ std::string GrpcXdsBootstrap::ToString() const { std::vector server_jsons; for (const XdsServer* server : authority.servers()) { server_jsons.emplace_back( - JsonDump(static_cast(server)->ToJson())); + JsonDump(DownCast(server)->ToJson())); } if (!server_jsons.empty()) { parts.push_back(absl::StrFormat(" servers=[\n%s\n],\n", diff --git a/src/core/xds/grpc/xds_client_grpc.cc b/src/core/xds/grpc/xds_client_grpc.cc index 5171a6d454c13..3b92fe87dcabf 100644 --- a/src/core/xds/grpc/xds_client_grpc.cc +++ b/src/core/xds/grpc/xds_client_grpc.cc @@ -20,7 +20,6 @@ #include #include #include -#include #include #include @@ -48,6 +47,7 @@ #include "src/core/lib/transport/error_utils.h" #include "src/core/telemetry/metrics.h" #include "src/core/util/debug_location.h" +#include "src/core/util/down_cast.h" #include "src/core/util/env.h" #include "src/core/util/load_file.h" #include "src/core/util/orphanable.h" @@ -321,7 +321,7 @@ GrpcXdsClient::GrpcXdsClient( .value_or(Duration::Seconds(15)))), key_(key), certificate_provider_store_(MakeOrphanable( - static_cast(this->bootstrap()) + DownCast(this->bootstrap()) .certificate_providers())), stats_plugin_group_(std::move(stats_plugin_group)), registered_metric_callback_(stats_plugin_group_.RegisterCallback( diff --git a/src/core/xds/grpc/xds_cluster_parser.cc b/src/core/xds/grpc/xds_cluster_parser.cc index 42f9f65287c4a..9466e71231b4a 100644 --- a/src/core/xds/grpc/xds_cluster_parser.cc +++ b/src/core/xds/grpc/xds_cluster_parser.cc @@ -48,6 +48,7 @@ #include "src/core/config/core_configuration.h" #include "src/core/lib/debug/trace.h" #include "src/core/load_balancing/lb_policy_registry.h" +#include "src/core/util/down_cast.h" #include "src/core/util/env.h" #include "src/core/util/host_port.h" #include "src/core/util/time.h" @@ -299,7 +300,7 @@ void ParseLbPolicyConfig(const XdsResourceType::DecodeContext& context, envoy_config_cluster_v3_Cluster_load_balancing_policy(cluster); if (load_balancing_policy != nullptr) { const auto& registry = - static_cast(context.client->bootstrap()) + DownCast(context.client->bootstrap()) .lb_policy_registry(); ValidationErrors::ScopedField field(errors, ".load_balancing_policy"); const size_t original_error_count = errors->size(); @@ -509,7 +510,7 @@ absl::StatusOr> CdsResourceParse( errors.AddError("ConfigSource is not self"); } cds_update->lrs_load_reporting_server = std::make_shared( - static_cast(context.server)); + DownCast(context.server)); } // Record LRS metric propagation. auto propagation = MakeRefCounted(); diff --git a/src/core/xds/grpc/xds_common_types_parser.cc b/src/core/xds/grpc/xds_common_types_parser.cc index 0f75f2012bc5e..eb4daca3477da 100644 --- a/src/core/xds/grpc/xds_common_types_parser.cc +++ b/src/core/xds/grpc/xds_common_types_parser.cc @@ -17,7 +17,6 @@ #include "src/core/xds/grpc/xds_common_types_parser.h" #include -#include #include #include @@ -40,6 +39,7 @@ #include "google/protobuf/struct.upbdefs.h" #include "google/protobuf/wrappers.upb.h" #include "src/core/lib/address_utils/parse_address.h" +#include "src/core/util/down_cast.h" #include "src/core/util/env.h" #include "src/core/util/json/json_reader.h" #include "src/core/util/upb_utils.h" @@ -137,7 +137,7 @@ CertificateProviderInstanceParse( envoy_extensions_transport_sockets_tls_v3_CommonTlsContext_CertificateProviderInstance_instance_name( certificate_provider_instance_proto)); const auto& bootstrap = - static_cast(context.client->bootstrap()); + DownCast(context.client->bootstrap()); if (bootstrap.certificate_providers().find(cert_provider.instance_name) == bootstrap.certificate_providers().end()) { ValidationErrors::ScopedField field(errors, ".instance_name"); @@ -162,7 +162,7 @@ CertificateProviderPluginInstanceParse( envoy_extensions_transport_sockets_tls_v3_CertificateProviderPluginInstance_instance_name( certificate_provider_plugin_instance_proto)); const auto& bootstrap = - static_cast(context.client->bootstrap()); + DownCast(context.client->bootstrap()); if (bootstrap.certificate_providers().find(cert_provider.instance_name) == bootstrap.certificate_providers().end()) { ValidationErrors::ScopedField field(errors, ".instance_name"); diff --git a/src/core/xds/grpc/xds_http_rbac_filter.cc b/src/core/xds/grpc/xds_http_rbac_filter.cc index 43e8eb53e2cca..d438620ba84f9 100644 --- a/src/core/xds/grpc/xds_http_rbac_filter.cc +++ b/src/core/xds/grpc/xds_http_rbac_filter.cc @@ -17,7 +17,6 @@ #include "src/core/xds/grpc/xds_http_rbac_filter.h" #include -#include #include #include @@ -43,6 +42,7 @@ #include "src/core/ext/filters/rbac/rbac_filter.h" #include "src/core/ext/filters/rbac/rbac_service_config_parser.h" #include "src/core/lib/channel/channel_args.h" +#include "src/core/util/down_cast.h" #include "src/core/util/env.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_writer.h" @@ -418,7 +418,7 @@ Json ParseAuditLoggerConfigsToJson( Json::Array logger_configs_json; size_t size; const auto& registry = - static_cast(context.client->bootstrap()) + DownCast(context.client->bootstrap()) .audit_logger_registry(); const envoy_config_rbac_v3_RBAC_AuditLoggingOptions_AuditLoggerConfig* const* logger_configs = diff --git a/src/core/xds/grpc/xds_listener_parser.cc b/src/core/xds/grpc/xds_listener_parser.cc index 9360f1db025ba..51afd86cf0070 100644 --- a/src/core/xds/grpc/xds_listener_parser.cc +++ b/src/core/xds/grpc/xds_listener_parser.cc @@ -16,7 +16,6 @@ #include "src/core/xds/grpc/xds_listener_parser.h" -#include #include #include @@ -48,6 +47,7 @@ #include "src/core/lib/address_utils/sockaddr_utils.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/iomgr/sockaddr.h" +#include "src/core/util/down_cast.h" #include "src/core/util/host_port.h" #include "src/core/util/match.h" #include "src/core/util/upb_utils.h" @@ -211,7 +211,7 @@ XdsListenerResource::HttpConnectionManager HttpConnectionManagerParse( { ValidationErrors::ScopedField field(errors, ".http_filters"); const auto& http_filter_registry = - static_cast(context.client->bootstrap()) + DownCast(context.client->bootstrap()) .http_filter_registry(); size_t num_filters = 0; const auto* http_filters = diff --git a/src/core/xds/grpc/xds_listener_parser.h b/src/core/xds/grpc/xds_listener_parser.h index 908da7b995bc6..ed0c88b85d6ac 100644 --- a/src/core/xds/grpc/xds_listener_parser.h +++ b/src/core/xds/grpc/xds_listener_parser.h @@ -20,6 +20,7 @@ #include "absl/strings/string_view.h" #include "envoy/config/listener/v3/listener.upbdefs.h" #include "envoy/extensions/filters/network/http_connection_manager/v3/http_connection_manager.upbdefs.h" +#include "src/core/util/down_cast.h" #include "src/core/xds/grpc/xds_bootstrap_grpc.h" #include "src/core/xds/grpc/xds_http_filter_registry.h" #include "src/core/xds/grpc/xds_listener.h" @@ -48,7 +49,7 @@ class XdsListenerResourceType final envoy_extensions_filters_network_http_connection_manager_v3_HttpConnectionManager_getmsgdef( symtab); const auto& http_filter_registry = - static_cast(xds_client->bootstrap()) + DownCast(xds_client->bootstrap()) .http_filter_registry(); http_filter_registry.PopulateSymtab(symtab); } diff --git a/src/core/xds/grpc/xds_route_config_parser.cc b/src/core/xds/grpc/xds_route_config_parser.cc index 7b14d16492cd0..b1de3decb70bf 100644 --- a/src/core/xds/grpc/xds_route_config_parser.cc +++ b/src/core/xds/grpc/xds_route_config_parser.cc @@ -17,7 +17,6 @@ #include "src/core/xds/grpc/xds_route_config_parser.h" #include -#include #include #include @@ -57,6 +56,7 @@ #include "src/core/lib/channel/status_util.h" #include "src/core/lib/debug/trace.h" #include "src/core/load_balancing/lb_policy_registry.h" +#include "src/core/util/down_cast.h" #include "src/core/util/env.h" #include "src/core/util/json/json.h" #include "src/core/util/json/json_writer.h" @@ -110,7 +110,7 @@ XdsRouteConfigResource::ClusterSpecifierPluginMap ClusterSpecifierPluginParse( XdsRouteConfigResource::ClusterSpecifierPluginMap cluster_specifier_plugin_map; const auto& cluster_specifier_plugin_registry = - static_cast(context.client->bootstrap()) + DownCast(context.client->bootstrap()) .cluster_specifier_plugin_registry(); size_t num_cluster_specifier_plugins; const envoy_config_route_v3_ClusterSpecifierPlugin* const* @@ -429,7 +429,7 @@ XdsRouteConfigResource::TypedPerFilterConfig ParseTypedPerFilterConfig( extension_to_use = &*nested_extension; } const auto& http_filter_registry = - static_cast(context.client->bootstrap()) + DownCast(context.client->bootstrap()) .http_filter_registry(); const XdsHttpFilterImpl* filter_impl = http_filter_registry.GetFilterForType(extension_to_use->type); diff --git a/src/core/xds/grpc/xds_route_config_parser.h b/src/core/xds/grpc/xds_route_config_parser.h index 98d279fd968bd..4d4f8f6891513 100644 --- a/src/core/xds/grpc/xds_route_config_parser.h +++ b/src/core/xds/grpc/xds_route_config_parser.h @@ -17,7 +17,6 @@ #ifndef GRPC_SRC_CORE_XDS_GRPC_XDS_ROUTE_CONFIG_PARSER_H #define GRPC_SRC_CORE_XDS_GRPC_XDS_ROUTE_CONFIG_PARSER_H -#include #include #include @@ -33,6 +32,7 @@ #include "envoy/config/route/v3/route.upbdefs.h" #include "re2/re2.h" #include "src/core/lib/channel/status_util.h" +#include "src/core/util/down_cast.h" #include "src/core/util/time.h" #include "src/core/util/validation_errors.h" #include "src/core/xds/grpc/xds_bootstrap_grpc.h" @@ -66,7 +66,7 @@ class XdsRouteConfigResourceType final upb_DefPool* symtab) const override { envoy_config_route_v3_RouteConfiguration_getmsgdef(symtab); const auto& cluster_specifier_plugin_registry = - static_cast(xds_client->bootstrap()) + DownCast(xds_client->bootstrap()) .cluster_specifier_plugin_registry(); cluster_specifier_plugin_registry.PopulateSymtab(symtab); } diff --git a/src/core/xds/grpc/xds_server_grpc.cc b/src/core/xds/grpc/xds_server_grpc.cc index 6b4eb9396992a..c23054659b8c5 100644 --- a/src/core/xds/grpc/xds_server_grpc.cc +++ b/src/core/xds/grpc/xds_server_grpc.cc @@ -26,6 +26,7 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "src/core/config/core_configuration.h" +#include "src/core/util/down_cast.h" #include "src/core/util/json/json_reader.h" #include "src/core/util/json/json_writer.h" @@ -69,7 +70,7 @@ bool GrpcXdsServer::TrustedXdsServer() const { } bool GrpcXdsServer::Equals(const XdsServer& other) const { - const auto& o = static_cast(other); + const auto& o = DownCast(other); return (server_uri_ == o.server_uri_ && channel_creds_config_->type() == o.channel_creds_config_->type() && channel_creds_config_->Equals(*o.channel_creds_config_) && diff --git a/src/core/xds/grpc/xds_transport_grpc.cc b/src/core/xds/grpc/xds_transport_grpc.cc index f3e0acd033ecc..508e770c8d2aa 100644 --- a/src/core/xds/grpc/xds_transport_grpc.cc +++ b/src/core/xds/grpc/xds_transport_grpc.cc @@ -24,7 +24,6 @@ #include #include #include -#include #include #include @@ -54,6 +53,7 @@ #include "src/core/lib/surface/lame_client.h" #include "src/core/lib/transport/connectivity_state.h" #include "src/core/util/debug_location.h" +#include "src/core/util/down_cast.h" #include "src/core/util/orphanable.h" #include "src/core/util/ref_counted_ptr.h" #include "src/core/util/time.h" @@ -271,8 +271,8 @@ GrpcXdsTransportFactory::GrpcXdsTransport::GrpcXdsTransport( key_(server.Key()) { GRPC_TRACE_LOG(xds_client, INFO) << "[GrpcXdsTransport " << this << "] created"; - channel_ = CreateXdsChannel(factory_->args_, - static_cast(server)); + channel_ = + CreateXdsChannel(factory_->args_, DownCast(server)); CHECK(channel_ != nullptr); if (channel_->IsLame()) { *status = absl::UnavailableError("xds client has a lame channel"); diff --git a/src/core/xds/xds_client/xds_resource_type_impl.h b/src/core/xds/xds_client/xds_resource_type_impl.h index c588f3f8f1794..bb54cbc650ee7 100644 --- a/src/core/xds/xds_client/xds_resource_type_impl.h +++ b/src/core/xds/xds_client/xds_resource_type_impl.h @@ -16,12 +16,12 @@ #ifndef GRPC_SRC_CORE_XDS_XDS_CLIENT_XDS_RESOURCE_TYPE_IMPL_H #define GRPC_SRC_CORE_XDS_XDS_CLIENT_XDS_RESOURCE_TYPE_IMPL_H -#include #include #include #include "absl/strings/string_view.h" +#include "src/core/util/down_cast.h" #include "src/core/util/ref_counted_ptr.h" #include "src/core/xds/xds_client/xds_client.h" #include "src/core/xds/xds_client/xds_resource_type.h" @@ -82,8 +82,8 @@ class XdsResourceTypeImpl : public XdsResourceType { bool ResourcesEqual(const ResourceData* r1, const ResourceData* r2) const override { - return *static_cast(r1) == - *static_cast(r2); + return *DownCast(r1) == + *DownCast(r2); } }; From 6b0f95b350f623c29ad24a4695a9aa2e8d0b1957 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Fri, 7 Feb 2025 13:36:51 -0800 Subject: [PATCH 5/5] [shutdown] Guard log with TraceFlag (#38702) Close https://github.com/grpc/grpc/issues/38490 Closes #38702 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/38702 from yashykt:ReduceLogLevel 4ac55dd6bb30860c391196200b3fb8a98ee8bf8e PiperOrigin-RevId: 724455807 --- src/core/lib/surface/init.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/lib/surface/init.cc b/src/core/lib/surface/init.cc index 9f3bd74786b69..ed468e8cf5da9 100644 --- a/src/core/lib/surface/init.cc +++ b/src/core/lib/surface/init.cc @@ -238,7 +238,8 @@ bool grpc_wait_for_shutdown_with_timeout(absl::Duration timeout) { grpc_core::MutexLock lock(g_init_mu); while (g_initializations != 0) { if (g_shutting_down_cv->WaitWithDeadline(g_init_mu, deadline)) { - LOG(ERROR) << "grpc_wait_for_shutdown_with_timeout() timed out."; + GRPC_TRACE_LOG(api, ERROR) + << "grpc_wait_for_shutdown_with_timeout() timed out."; return false; } }