From 087b74f739a269e436822b17ac38e62cf3b5d74e Mon Sep 17 00:00:00 2001 From: Vignesh Babu Date: Mon, 24 Jul 2023 12:02:02 -0700 Subject: [PATCH] Revert "Revert "[interop] Add absl dependency to interop server"" #33828 (#33830) Reverts https://github.com/grpc/grpc/pull/33676 The serve method needs to be called with args. The previous attempt did not change the signature in commands.py which lead to errors such as https://screenshot.googleplex.com/6wZVER9ZETMGAmA. The fix is in https://github.com/grpc/grpc/pull/33830/commits/4e211d02914fa9886ea1283442a9a6f6a54a61bd --- requirements.bazel.txt | 1 + src/python/grpcio_tests/commands.py | 5 +++-- src/python/grpcio_tests/setup.py | 1 + src/python/grpcio_tests/tests/interop/BUILD.bazel | 1 + src/python/grpcio_tests/tests/interop/server.py | 15 +++++++-------- .../grpcio_tests/tests_aio/interop/server.py | 8 ++++---- tools/run_tests/helper_scripts/build_python.sh | 2 +- 7 files changed, 18 insertions(+), 15 deletions(-) diff --git a/requirements.bazel.txt b/requirements.bazel.txt index 0a26df5ac27..b42c8f63bd7 100644 --- a/requirements.bazel.txt +++ b/requirements.bazel.txt @@ -15,3 +15,4 @@ setuptools==44.1.1 xds-protos==0.0.11 opencensus==0.10.0 opencensus-ext-stackdriver==0.8.0 +absl-py==1.4.0 diff --git a/src/python/grpcio_tests/commands.py b/src/python/grpcio_tests/commands.py index 22f458b8c9b..9fb3abb4dad 100644 --- a/src/python/grpcio_tests/commands.py +++ b/src/python/grpcio_tests/commands.py @@ -330,12 +330,13 @@ class RunInterop(test.test): from tests_aio.interop import server sys.argv[1:] = self.args.split() - asyncio.get_event_loop().run_until_complete(server.serve()) + args = server.parse_interop_server_arguments(sys.argv) + asyncio.get_event_loop().run_until_complete(server.serve(args)) else: from tests.interop import server sys.argv[1:] = self.args.split() - server.serve() + server.serve(server.parse_interop_server_arguments(sys.argv)) def run_client(self): # We import here to ensure that our setuptools parent has had a chance to diff --git a/src/python/grpcio_tests/setup.py b/src/python/grpcio_tests/setup.py index 8409dcc9519..0cef4c2755e 100644 --- a/src/python/grpcio_tests/setup.py +++ b/src/python/grpcio_tests/setup.py @@ -47,6 +47,7 @@ INSTALL_REQUIRES = ( "protobuf>=4.21.6rc1,!=4.22.0.*", "google-auth>=1.17.2", "requests>=2.14.2", + "absl-py>=1.4.0", ) COMMAND_CLASS = { diff --git a/src/python/grpcio_tests/tests/interop/BUILD.bazel b/src/python/grpcio_tests/tests/interop/BUILD.bazel index aa7e15544c4..2d164847423 100644 --- a/src/python/grpcio_tests/tests/interop/BUILD.bazel +++ b/src/python/grpcio_tests/tests/interop/BUILD.bazel @@ -94,6 +94,7 @@ py_library( "//src/proto/grpc/testing:py_test_proto", "//src/python/grpcio/grpc:grpcio", "//src/python/grpcio_tests/tests/unit:test_common", + requirement("absl-py"), ], ) diff --git a/src/python/grpcio_tests/tests/interop/server.py b/src/python/grpcio_tests/tests/interop/server.py index d51671ef8a9..cc705b026da 100644 --- a/src/python/grpcio_tests/tests/interop/server.py +++ b/src/python/grpcio_tests/tests/interop/server.py @@ -13,10 +13,11 @@ # limitations under the License. """The Python implementation of the GRPC interoperability test server.""" -import argparse from concurrent import futures import logging +from absl import app +from absl.flags import argparse_flags import grpc from src.proto.grpc.testing import test_pb2_grpc @@ -28,8 +29,8 @@ logging.basicConfig() _LOGGER = logging.getLogger(__name__) -def parse_interop_server_arguments(): - parser = argparse.ArgumentParser() +def parse_interop_server_arguments(argv): + parser = argparse_flags.ArgumentParser() parser.add_argument( "--port", type=int, required=True, help="the port on which to serve" ) @@ -45,7 +46,7 @@ def parse_interop_server_arguments(): type=resources.parse_bool, help="require an ALTS connection", ) - return parser.parse_args() + return parser.parse_args(argv[1:]) def get_server_credentials(use_tls): @@ -57,9 +58,7 @@ def get_server_credentials(use_tls): return grpc.alts_server_credentials() -def serve(): - args = parse_interop_server_arguments() - +def serve(args): server = test_common.test_server() test_pb2_grpc.add_TestServiceServicer_to_server( service.TestService(), server @@ -77,4 +76,4 @@ def serve(): if __name__ == "__main__": - serve() + app.run(serve, flags_parser=parse_interop_server_arguments) diff --git a/src/python/grpcio_tests/tests_aio/interop/server.py b/src/python/grpcio_tests/tests_aio/interop/server.py index 7786a4206a6..3247bed26a8 100644 --- a/src/python/grpcio_tests/tests_aio/interop/server.py +++ b/src/python/grpcio_tests/tests_aio/interop/server.py @@ -16,6 +16,7 @@ import argparse import asyncio import logging +import sys import grpc @@ -27,9 +28,7 @@ _LOGGER = logging.getLogger(__name__) _LOGGER.setLevel(logging.DEBUG) -async def serve(): - args = interop_server_lib.parse_interop_server_arguments() - +async def serve(args): if args.use_tls or args.use_alts: credentials = interop_server_lib.get_server_credentials(args.use_tls) address, server = await _test_server.start_test_server( @@ -47,4 +46,5 @@ async def serve(): if __name__ == "__main__": - asyncio.get_event_loop().run_until_complete(serve()) + args = interop_server_lib.parse_interop_server_arguments(sys.argv) + asyncio.get_event_loop().run_until_complete(serve(args)) diff --git a/tools/run_tests/helper_scripts/build_python.sh b/tools/run_tests/helper_scripts/build_python.sh index 5057b642c9c..b2a9e91c7cf 100755 --- a/tools/run_tests/helper_scripts/build_python.sh +++ b/tools/run_tests/helper_scripts/build_python.sh @@ -209,7 +209,7 @@ pip_install_dir "$ROOT/src/python/grpcio_testing" # Build/install tests pip_install coverage==4.4 oauth2client==4.1.0 \ google-auth>=1.17.2 requests==2.14.2 \ - googleapis-common-protos>=1.5.5 rsa==4.0 + googleapis-common-protos>=1.5.5 rsa==4.0 absl-py==1.4.0 $VENV_PYTHON "$ROOT/src/python/grpcio_tests/setup.py" preprocess $VENV_PYTHON "$ROOT/src/python/grpcio_tests/setup.py" build_package_protos pip_install_dir "$ROOT/src/python/grpcio_tests"