Adopt reviewer's suggestions

* Correct the StatusCode
* Format code
* Use @staticmethod
* Fix typo

3 files changed, 65 insertions, 37 deletions

diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/channelz.pyx.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/channelz.pyx.pxi
index 572a473d3e..113f7976dd 100644
--- a/src/python/grpcio/grpc/_cython/_cygrpc/channelz.pyx.pxi
+++ b/src/python/grpcio/grpc/_cython/_cygrpc/channelz.pyx.pxi
@@ -14,43 +14,56 @@
 
 
 def channelz_get_top_channels(start_channel_id):
-    cdef char *c_returned_str = grpc_channelz_get_top_channels(start_channel_id)
+    cdef char *c_returned_str = grpc_channelz_get_top_channels(
+        start_channel_id,
+    )
     if c_returned_str == NULL:
-        raise ValueError('Failed to get top channels, please ensure your start_channel_id==%s is valid' % start_channel_id)
+        raise ValueError('Failed to get top channels, please ensure your' \
+                         ' start_channel_id==%s is valid' % start_channel_id)
     return c_returned_str
     
 def channelz_get_servers(start_server_id):
     cdef char *c_returned_str = grpc_channelz_get_servers(start_server_id)
     if c_returned_str == NULL:
-        raise ValueError('Failed to get servers, please ensure your start_server_id==%s is valid' % start_server_id)
+        raise ValueError('Failed to get servers, please ensure your' \
+                         ' start_server_id==%s is valid' % start_server_id)
     return c_returned_str
     
 def channelz_get_server(server_id):
     cdef char *c_returned_str = grpc_channelz_get_server(server_id)
     if c_returned_str == NULL:
-        raise ValueError('Failed to get the server, please ensure your server_id==%s is valid' % server_id)
+        raise ValueError('Failed to get the server, please ensure your' \
+                         ' server_id==%s is valid' % server_id)
     return c_returned_str
     
 def channelz_get_server_sockets(server_id, start_socket_id):
-    cdef char *c_returned_str = grpc_channelz_get_server_sockets(server_id, start_socket_id)
+    cdef char *c_returned_str = grpc_channelz_get_server_sockets(
+        server_id,
+        start_socket_id,
+    )
     if c_returned_str == NULL:
-        raise ValueError('Failed to get server sockets, please ensure your server_id==%s and start_socket_id==%s is valid' % (server_id, start_socket_id))
+        raise ValueError('Failed to get server sockets, please ensure your' \
+                         ' server_id==%s and start_socket_id==%s is valid' %
+                         (server_id, start_socket_id))
     return c_returned_str
     
 def channelz_get_channel(channel_id):
     cdef char *c_returned_str = grpc_channelz_get_channel(channel_id)
     if c_returned_str == NULL:
-        raise ValueError('Failed to get the channel, please ensure your channel_id==%s is valid' % (channel_id))
+        raise ValueError('Failed to get the channel, please ensure your' \
+                         ' channel_id==%s is valid' % (channel_id))
     return c_returned_str
     
 def channelz_get_subchannel(subchannel_id):
     cdef char *c_returned_str = grpc_channelz_get_subchannel(subchannel_id)
     if c_returned_str == NULL:
-        raise ValueError('Failed to get the subchannel, please ensure your subchannel_id==%s is valid' % (subchannel_id))
+        raise ValueError('Failed to get the subchannel, please ensure your' \
+                         ' subchannel_id==%s is valid' % (subchannel_id))
     return c_returned_str
     
 def channelz_get_socket(socket_id):
     cdef char *c_returned_str = grpc_channelz_get_socket(socket_id)
     if c_returned_str == NULL:
-        raise ValueError('Failed to get the socket, please ensure your socket_id==%s is valid' % (socket_id))
+        raise ValueError('Failed to get the socket, please ensure your' \
+                         ' socket_id==%s is valid' % (socket_id))
     return c_returned_str
diff --git a/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py b/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py
index bc08562b7b..1ae5e8c140 100644
--- a/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py
+++ b/src/python/grpcio_channelz/grpc_channelz/v1/channelz.py
@@ -25,41 +25,44 @@ from google.protobuf import json_format
 class ChannelzServicer(_channelz_pb2_grpc.ChannelzServicer):
     """Servicer handling RPCs for service statuses."""
 
-    # pylint: disable=no-self-use
-    def GetTopChannels(self, request, context):
+    @staticmethod
+    def GetTopChannels(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_top_channels(request.start_channel_id),
                 _channelz_pb2.GetTopChannelsResponse(),
             )
-        except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+        except (ValueError, json_format.ParseError) as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetServers(self, request, context):
+    @staticmethod
+    def GetServers(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_servers(request.start_server_id),
                 _channelz_pb2.GetServersResponse(),
             )
-        except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+        except (ValueError, json_format.ParseError) as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetServer(self, request, context):
+    @staticmethod
+    def GetServer(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_server(request.server_id),
                 _channelz_pb2.GetServerResponse(),
             )
         except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+            context.set_code(grpc.StatusCode.NOT_FOUND)
+            context.set_details(str(e))
+        except json_format.ParseError as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetServerSockets(self, request, context):
+    @staticmethod
+    def GetServerSockets(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_server_sockets(request.server_id,
@@ -67,40 +70,52 @@ class ChannelzServicer(_channelz_pb2_grpc.ChannelzServicer):
                 _channelz_pb2.GetServerSocketsResponse(),
             )
         except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+            context.set_code(grpc.StatusCode.NOT_FOUND)
+            context.set_details(str(e))
+        except json_format.ParseError as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetChannel(self, request, context):
+    @staticmethod
+    def GetChannel(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_channel(request.channel_id),
                 _channelz_pb2.GetChannelResponse(),
             )
         except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+            context.set_code(grpc.StatusCode.NOT_FOUND)
+            context.set_details(str(e))
+        except json_format.ParseError as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetSubchannel(self, request, context):
+    @staticmethod
+    def GetSubchannel(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_subchannel(request.subchannel_id),
                 _channelz_pb2.GetSubchannelResponse(),
             )
         except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+            context.set_code(grpc.StatusCode.NOT_FOUND)
+            context.set_details(str(e))
+        except json_format.ParseError as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
-    # pylint: disable=no-self-use
-    def GetSocket(self, request, context):
+    @staticmethod
+    def GetSocket(request, context):
         try:
             return json_format.Parse(
                 cygrpc.channelz_get_socket(request.socket_id),
                 _channelz_pb2.GetSocketResponse(),
             )
         except ValueError as e:
-            context.set_code(grpc.StatusCode.INVALID_ARGUMENT)
+            context.set_code(grpc.StatusCode.NOT_FOUND)
+            context.set_details(str(e))
+        except json_format.ParseError as e:
+            context.set_code(grpc.StatusCode.INTERNAL)
             context.set_details(str(e))
 
 
diff --git a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py
index 103609441e..2549ed76be 100644
--- a/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py
+++ b/src/python/grpcio_tests/tests/channelz/_channelz_servicer_test.py
@@ -428,7 +428,7 @@ class ChannelzServicerTest(unittest.TestCase):
             self._channelz_stub.GetServer(
                 channelz_pb2.GetServerRequest(server_id=10000))
         except BaseException as e:
-            self.assertIn('StatusCode.INVALID_ARGUMENT', str(e))
+            self.assertIn('StatusCode.NOT_FOUND', str(e))
         else:
             self.fail('Invalid query not detected')
 
@@ -437,7 +437,7 @@ class ChannelzServicerTest(unittest.TestCase):
             self._channelz_stub.GetChannel(
                 channelz_pb2.GetChannelRequest(channel_id=10000))
         except BaseException as e:
-            self.assertIn('StatusCode.INVALID_ARGUMENT', str(e))
+            self.assertIn('StatusCode.NOT_FOUND', str(e))
         else:
             self.fail('Invalid query not detected')
 
@@ -446,7 +446,7 @@ class ChannelzServicerTest(unittest.TestCase):
             self._channelz_stub.GetSubchannel(
                 channelz_pb2.GetSubchannelRequest(subchannel_id=10000))
         except BaseException as e:
-            self.assertIn('StatusCode.INVALID_ARGUMENT', str(e))
+            self.assertIn('StatusCode.NOT_FOUND', str(e))
         else:
             self.fail('Invalid query not detected')
 
@@ -455,7 +455,7 @@ class ChannelzServicerTest(unittest.TestCase):
             self._channelz_stub.GetSocket(
                 channelz_pb2.GetSocketRequest(socket_id=10000))
         except BaseException as e:
-            self.assertIn('StatusCode.INVALID_ARGUMENT', str(e))
+            self.assertIn('StatusCode.NOT_FOUND', str(e))
         else:
             self.fail('Invalid query not detected')
 
@@ -467,7 +467,7 @@ class ChannelzServicerTest(unittest.TestCase):
                     start_socket_id=0,
                 ))
         except BaseException as e:
-            self.assertIn('StatusCode.INVALID_ARGUMENT', str(e))
+            self.assertIn('StatusCode.NOT_FOUND', str(e))
         else:
             self.fail('Invalid query not detected')