From 2fb126634d147a0c6a460d5699160519f219bdeb Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Tue, 15 Dec 2020 22:34:55 -0500 Subject: [PATCH] PR feedback: naming an comments --- tools/run_tests/xds_test_driver/.gitignore | 1 - .../xds_test_driver/framework/rpc/__init__.py | 9 +++++---- .../xds_test_driver/framework/rpc/grpc_channelz.py | 14 +++++++++----- .../xds_test_driver/framework/rpc/grpc_testing.py | 7 ++++++- .../xds_test_driver/framework/xds_k8s_testcase.py | 2 +- 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/tools/run_tests/xds_test_driver/.gitignore b/tools/run_tests/xds_test_driver/.gitignore index 4de58d1dea3..cc784450cc1 100644 --- a/tools/run_tests/xds_test_driver/.gitignore +++ b/tools/run_tests/xds_test_driver/.gitignore @@ -2,4 +2,3 @@ config/local-dev.cfg src/proto venv/ out/ - diff --git a/tools/run_tests/xds_test_driver/framework/rpc/__init__.py b/tools/run_tests/xds_test_driver/framework/rpc/__init__.py index 857bc64b2c2..e3935816db1 100644 --- a/tools/run_tests/xds_test_driver/framework/rpc/__init__.py +++ b/tools/run_tests/xds_test_driver/framework/rpc/__init__.py @@ -35,10 +35,11 @@ class GrpcClientHelper: def __init__(self, channel: grpc.Channel, stub_class: ClassVar): self.channel = channel self.stub = stub_class(channel) - # For better logging - self.service_name = re.sub('Stub$', '', self.stub.__class__.__name__) + # This is purely cosmetic to make RPC logs look like method calls. + self.log_service_name = re.sub('Stub$', '', + self.stub.__class__.__name__) - def call_unary_when_channel_ready( + def call_unary_with_deadline( self, *, rpc: str, @@ -59,7 +60,7 @@ class GrpcClientHelper: return rpc_callable(req, **call_kwargs) def _log_debug(self, rpc, req, call_kwargs): - logger.debug('RPC %s.%s(request=%s(%r), %s)', self.service_name, rpc, + logger.debug('RPC %s.%s(request=%s(%r), %s)', self.log_service_name, rpc, req.__class__.__name__, json_format.MessageToDict(req), ', '.join({f'{k}={v}' for k, v in call_kwargs.items()})) diff --git a/tools/run_tests/xds_test_driver/framework/rpc/grpc_channelz.py b/tools/run_tests/xds_test_driver/framework/rpc/grpc_channelz.py index fc9b7c0998c..1dfe72ad103 100644 --- a/tools/run_tests/xds_test_driver/framework/rpc/grpc_channelz.py +++ b/tools/run_tests/xds_test_driver/framework/rpc/grpc_channelz.py @@ -11,6 +11,10 @@ # 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 contains helpers for gRPC services defined in +https://github.com/grpc/grpc-proto/blob/master/grpc/channelz/v1/channelz.proto +""" import ipaddress import logging from typing import Optional, Iterator @@ -118,7 +122,7 @@ class ChannelzServiceClient(framework.rpc.GrpcClientHelper): # From proto: To request subsequent pages, the client generates this # value by adding 1 to the highest seen result ID. start += 1 - response = self.call_unary_when_channel_ready( + response = self.call_unary_with_deadline( rpc='GetTopChannels', req=_GetTopChannelsRequest(start_channel_id=start)) for channel in response.channel: @@ -133,7 +137,7 @@ class ChannelzServiceClient(framework.rpc.GrpcClientHelper): # From proto: To request subsequent pages, the client generates this # value by adding 1 to the highest seen result ID. start += 1 - response = self.call_unary_when_channel_ready( + response = self.call_unary_with_deadline( rpc='GetServers', req=_GetServersRequest(start_server_id=start)) for server in response.server: start = max(start, server.ref.server_id) @@ -147,7 +151,7 @@ class ChannelzServiceClient(framework.rpc.GrpcClientHelper): # From proto: To request subsequent pages, the client generates this # value by adding 1 to the highest seen result ID. start += 1 - response = self.call_unary_when_channel_ready( + response = self.call_unary_with_deadline( rpc='GetServerSockets', req=_GetServerSocketsRequest(server_id=server_id, start_socket_id=start)) @@ -159,13 +163,13 @@ class ChannelzServiceClient(framework.rpc.GrpcClientHelper): def get_subchannel(self, subchannel_id) -> Subchannel: """Return a single Subchannel, otherwise raises RpcError.""" - response: _GetSubchannelResponse = self.call_unary_when_channel_ready( + response: _GetSubchannelResponse = self.call_unary_with_deadline( rpc='GetSubchannel', req=_GetSubchannelRequest(subchannel_id=subchannel_id)) return response.subchannel def get_socket(self, socket_id) -> Socket: """Return a single Socket, otherwise raises RpcError.""" - response: _GetSocketResponse = self.call_unary_when_channel_ready( + response: _GetSocketResponse = self.call_unary_with_deadline( rpc='GetSocket', req=_GetSocketRequest(socket_id=socket_id)) return response.socket diff --git a/tools/run_tests/xds_test_driver/framework/rpc/grpc_testing.py b/tools/run_tests/xds_test_driver/framework/rpc/grpc_testing.py index 95c954f17c3..3242b5f9209 100644 --- a/tools/run_tests/xds_test_driver/framework/rpc/grpc_testing.py +++ b/tools/run_tests/xds_test_driver/framework/rpc/grpc_testing.py @@ -11,6 +11,11 @@ # 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 contains helpers for gRPC services defined in +https://github.com/grpc/grpc/blob/master/src/proto/grpc/testing/test.proto +https://github.com/grpc/grpc/blob/master/src/proto/grpc/testing/test.proto +""" from typing import Optional import grpc @@ -40,7 +45,7 @@ class LoadBalancerStatsServiceClient(framework.rpc.GrpcClientHelper): if timeout_sec is None: timeout_sec = self.STATS_PARTIAL_RESULTS_TIMEOUT_SEC - return self.call_unary_when_channel_ready( + return self.call_unary_with_deadline( rpc='GetClientStats', wait_for_ready_sec=timeout_sec, req=_LoadBalancerStatsRequest(num_rpcs=num_rpcs, diff --git a/tools/run_tests/xds_test_driver/framework/xds_k8s_testcase.py b/tools/run_tests/xds_test_driver/framework/xds_k8s_testcase.py index 9ca14e845f8..e64b87f4169 100644 --- a/tools/run_tests/xds_test_driver/framework/xds_k8s_testcase.py +++ b/tools/run_tests/xds_test_driver/framework/xds_k8s_testcase.py @@ -110,7 +110,7 @@ class XdsKubernetesTestCase(absltest.TestCase): def setupServerBackends(self): # Load Backends neg_name, neg_zones = self.server_runner.k8s_namespace.get_service_neg( - self.server_runner.service_name, self.server_port) + self.server_runner.log_service_name, self.server_port) # Add backends to the Backend Service self.td.backend_service_add_neg_backends(neg_name, neg_zones)