Clean up Python C API use in src/python/src/grpc/_adapter/*.c. Specifically:

 - Use METH_O and METH_NOARGS where appropriate, modifying the C functions
   appropriately.  METH_O is for functions that take a single PyObject, and
   it's passed directly instead of 'args'.  METH_NOARGS is for functions
   that take no arguments, and they get called with just one argument
   ('self'.)

 - In PyArg_ParseTuple*() calls, specify the callable's name for more
   descriptive exception messages.

 - For tp_init functions (which always take keyword arguments) introduce
   keyword argument parsing (using the C local variables as keywords,
   although I don't know if they're the best names to use.) This is mostly
   as a way to show how keyword arguments are done in C.  An alternative
   method is to use _PyArg_NoKeywords(kwds) (see
   https://hg.python.org/cpython/file/70a55b2dee71/Python/getargs.c#l1820,
   but unfortunately it's not part of the official API,) or check manually
   that the dict is empty.

 - Check the return value of Python API functions that can return an error
   indicator (NULL or -1.) PyFloat_AsDouble is also one of these, but we
   don't check the return type (we would have to compare the result to 1.0
   with ==, which is not a thing you should do) so just call PyErr_Occurred
   unconditionally.

 - Change Py_BuildValue() calls with just "O" formats into PyTuple_Pack
   calls.  It requires less runtime checking.

 - Replace Py_BuildValue()/PyObject_CallObject pairs with
   PyObject_CallFunctionObjArgs (since all of them have just PyObject*
   arguments.) If the Py_BuildValue formats had included other types,
   PyObject_CallFunction() would have been easier, but no need in these
   cases.

 - Replace Py_BuildValue("f", ...) with PyFloat_FromDouble(...). Less
   runtime checking and parsing necessary, and more obvious in what it does.

 - In the PyType structs, replace "PyObject_HEAD_INIT(NULL) 0" with
   "PyVarObject_HEAD_INIT(NULL, 0)".  Anything with an ob_size struct member
   is a PyVarObject, although the distinction isn't all that import; it's
   just a more convenient macro.

 - Assign tp_new in the PyType structs directly, like all other struct
   members, rather than right before the PyType_Ready() call.

 - Remove PyErr_SetString() calls in places that already have a (meaningful)
   exception set.

 - Add a PyErr_Format() for an error return that wasn't setting an exception
   (PyObject_TypeCheck() doesn't set an exception.)

 - Remove NULL assignments to struct members in the error paths of the
   tp_init functions.  PyObject structs are always zeroed after allocation,
   guaranteed.  (If there's a way for them to already contain an object
   you'd use Py_CLEAR() to clear them, but that can't happen in these
   cases.)

 - Remove a few unnecessary parentheses.

6 files changed, 145 insertions, 124 deletions

diff --git a/src/python/src/grpc/_adapter/_c.c b/src/python/src/grpc/_adapter/_c.c
index 13eb93fe5a..55b9d0512c 100644
--- a/src/python/src/grpc/_adapter/_c.c
+++ b/src/python/src/grpc/_adapter/_c.c
@@ -40,19 +40,20 @@
 #include "grpc/_adapter/_server.h"
 #include "grpc/_adapter/_server_credentials.h"
 
-static PyObject *init(PyObject *self, PyObject *args) {
+static PyObject *init(PyObject *self) {
   grpc_init();
   Py_RETURN_NONE;
 }
 
-static PyObject *shutdown(PyObject *self, PyObject *args) {
+static PyObject *shutdown(PyObject *self) {
   grpc_shutdown();
   Py_RETURN_NONE;
 }
 
 static PyMethodDef _c_methods[] = {
-    {"init", init, METH_VARARGS, "Initialize the module's static state."},
-    {"shut_down", shutdown, METH_VARARGS,
+    {"init", (PyCFunction)init, METH_NOARGS,
+     "Initialize the module's static state."},
+    {"shut_down", (PyCFunction)shutdown, METH_NOARGS,
      "Shut down the module's static state."},
     {NULL},
 };
diff --git a/src/python/src/grpc/_adapter/_call.c b/src/python/src/grpc/_adapter/_call.c
index 7e62c1b7a3..325d3d5bbd 100644
--- a/src/python/src/grpc/_adapter/_call.c
+++ b/src/python/src/grpc/_adapter/_call.c
@@ -46,10 +46,11 @@ static int pygrpc_call_init(Call *self, PyObject *args, PyObject *kwds) {
   const char *method;
   const char *host;
   const double deadline;
+  static char *kwlist[] = {"channel", "method", "host", "deadline", NULL};
 
-  if (!PyArg_ParseTuple(args, "O!ssd", &pygrpc_ChannelType, &channel, &method,
-                        &host, &deadline)) {
-    self->c_call = NULL;
+  if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!ssd:Call", kwlist,
+                                   &pygrpc_ChannelType, &channel, &method,
+                                   &host, &deadline)) {
     return -1;
   }
 
@@ -77,7 +78,7 @@ static const PyObject *pygrpc_call_invoke(Call *self, PyObject *args) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "O!OO", &pygrpc_CompletionQueueType,
+  if (!(PyArg_ParseTuple(args, "O!OO:invoke", &pygrpc_CompletionQueueType,
                          &completion_queue, &metadata_tag, &finish_tag))) {
     return NULL;
   }
@@ -103,7 +104,7 @@ static const PyObject *pygrpc_call_write(Call *self, PyObject *args) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "s#O", &bytes, &length, &tag))) {
+  if (!(PyArg_ParseTuple(args, "s#O:write", &bytes, &length, &tag))) {
     return NULL;
   }
 
@@ -123,15 +124,10 @@ static const PyObject *pygrpc_call_write(Call *self, PyObject *args) {
   return result;
 }
 
-static const PyObject *pygrpc_call_complete(Call *self, PyObject *args) {
-  const PyObject *tag;
+static const PyObject *pygrpc_call_complete(Call *self, PyObject *tag) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "O", &tag))) {
-    return NULL;
-  }
-
   call_error = grpc_call_writes_done_old(self->c_call, (void *)tag);
 
   result = pygrpc_translate_call_error(call_error);
@@ -147,7 +143,7 @@ static const PyObject *pygrpc_call_accept(Call *self, PyObject *args) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "O!O", &pygrpc_CompletionQueueType,
+  if (!(PyArg_ParseTuple(args, "O!O:accept", &pygrpc_CompletionQueueType,
                          &completion_queue, &tag))) {
     return NULL;
   }
@@ -164,21 +160,16 @@ static const PyObject *pygrpc_call_accept(Call *self, PyObject *args) {
   return result;
 }
 
-static const PyObject *pygrpc_call_premetadata(Call *self, PyObject *args) {
+static const PyObject *pygrpc_call_premetadata(Call *self) {
   /* TODO(b/18702680): Actually support metadata. */
   return pygrpc_translate_call_error(
       grpc_call_server_end_initial_metadata_old(self->c_call, 0));
 }
 
-static const PyObject *pygrpc_call_read(Call *self, PyObject *args) {
-  const PyObject *tag;
+static const PyObject *pygrpc_call_read(Call *self, PyObject *tag) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "O", &tag))) {
-    return NULL;
-  }
-
   call_error = grpc_call_start_read_old(self->c_call, (void *)tag);
 
   result = pygrpc_translate_call_error(call_error);
@@ -198,16 +189,30 @@ static const PyObject *pygrpc_call_status(Call *self, PyObject *args) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "OO", &status, &tag))) {
+  if (!(PyArg_ParseTuple(args, "OO:status", &status, &tag))) {
     return NULL;
   }
 
   code = PyObject_GetAttrString(status, "code");
+  if (code == NULL) {
+    return NULL;
+  }
   details = PyObject_GetAttrString(status, "details");
+  if (details == NULL) {
+    Py_DECREF(code);
+    return NULL;
+  }
   c_code = PyInt_AsLong(code);
-  c_message = PyBytes_AsString(details);
   Py_DECREF(code);
+  if (c_code == -1 && PyErr_Occurred()) {
+    Py_DECREF(details);
+    return NULL;
+  }
+  c_message = PyBytes_AsString(details);
   Py_DECREF(details);
+  if (c_message == NULL) {
+    return NULL;
+  }
 
   call_error = grpc_call_start_write_status_old(self->c_call, c_code, c_message,
                                                 (void *)tag);
@@ -228,12 +233,12 @@ static PyMethodDef methods[] = {
      "Invoke this call."},
     {"write", (PyCFunction)pygrpc_call_write, METH_VARARGS,
      "Write bytes to this call."},
-    {"complete", (PyCFunction)pygrpc_call_complete, METH_VARARGS,
+    {"complete", (PyCFunction)pygrpc_call_complete, METH_O,
      "Complete writes to this call."},
     {"accept", (PyCFunction)pygrpc_call_accept, METH_VARARGS, "Accept an RPC."},
     {"premetadata", (PyCFunction)pygrpc_call_premetadata, METH_VARARGS,
      "Indicate the end of leading metadata in the response."},
-    {"read", (PyCFunction)pygrpc_call_read, METH_VARARGS,
+    {"read", (PyCFunction)pygrpc_call_read, METH_O,
      "Read bytes from this call."},
     {"status", (PyCFunction)pygrpc_call_status, METH_VARARGS,
      "Report this call's status."},
@@ -242,7 +247,7 @@ static PyMethodDef methods[] = {
     {NULL}};
 
 PyTypeObject pygrpc_CallType = {
-    PyObject_HEAD_INIT(NULL)0,       /*ob_size*/
+    PyVarObject_HEAD_INIT(NULL, 0)
     "_grpc.Call",                    /*tp_name*/
     sizeof(Call),                    /*tp_basicsize*/
     0,                               /*tp_itemsize*/
@@ -278,16 +283,16 @@ PyTypeObject pygrpc_CallType = {
     0,                               /* tp_descr_set */
     0,                               /* tp_dictoffset */
     (initproc)pygrpc_call_init,      /* tp_init */
+    0,                               /* tp_alloc */
+    PyType_GenericNew,               /* tp_new */
 };
 
 int pygrpc_add_call(PyObject *module) {
-  pygrpc_CallType.tp_new = PyType_GenericNew;
   if (PyType_Ready(&pygrpc_CallType) < 0) {
-    PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_CallType!");
     return -1;
   }
   if (PyModule_AddObject(module, "Call", (PyObject *)&pygrpc_CallType) == -1) {
-    PyErr_SetString(PyExc_ImportError, "Couldn't add Call type to module!");
+    return -1;
   }
   return 0;
 }
diff --git a/src/python/src/grpc/_adapter/_channel.c b/src/python/src/grpc/_adapter/_channel.c
index 6962722ed2..3ba943e4b2 100644
--- a/src/python/src/grpc/_adapter/_channel.c
+++ b/src/python/src/grpc/_adapter/_channel.c
@@ -38,9 +38,10 @@
 
 static int pygrpc_channel_init(Channel *self, PyObject *args, PyObject *kwds) {
   const char *hostport;
+  static char *kwlist[] = {"hostport", NULL};
 
-  if (!(PyArg_ParseTuple(args, "s", &hostport))) {
-    self->c_channel = NULL;
+  if (!(PyArg_ParseTupleAndKeywords(args, kwds, "s:Channel", kwlist,
+                                    &hostport))) {
     return -1;
   }
 
@@ -56,7 +57,7 @@ static void pygrpc_channel_dealloc(Channel *self) {
 }
 
 PyTypeObject pygrpc_ChannelType = {
-    PyObject_HEAD_INIT(NULL)0,          /*ob_size*/
+    PyVarObject_HEAD_INIT(NULL, 0)
     "_grpc.Channel",                    /*tp_name*/
     sizeof(Channel),                    /*tp_basicsize*/
     0,                                  /*tp_itemsize*/
@@ -92,17 +93,16 @@ PyTypeObject pygrpc_ChannelType = {
     0,                                  /* tp_descr_set */
     0,                                  /* tp_dictoffset */
     (initproc)pygrpc_channel_init,      /* tp_init */
+    0,                                  /* tp_alloc */
+    PyType_GenericNew,                  /* tp_new */
 };
 
 int pygrpc_add_channel(PyObject *module) {
-  pygrpc_ChannelType.tp_new = PyType_GenericNew;
   if (PyType_Ready(&pygrpc_ChannelType) < 0) {
-    PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_ChannelType!");
     return -1;
   }
   if (PyModule_AddObject(module, "Channel", (PyObject *)&pygrpc_ChannelType) ==
       -1) {
-    PyErr_SetString(PyExc_ImportError, "Couldn't add Channel type to module!");
     return -1;
   }
   return 0;
diff --git a/src/python/src/grpc/_adapter/_completion_queue.c b/src/python/src/grpc/_adapter/_completion_queue.c
index 1d593d0d14..b56ca1926e 100644
--- a/src/python/src/grpc/_adapter/_completion_queue.c
+++ b/src/python/src/grpc/_adapter/_completion_queue.c
@@ -70,7 +70,7 @@ static PyObject *metadata_event_kind;
 static PyObject *finish_event_kind;
 
 static PyObject *pygrpc_as_py_time(gpr_timespec *timespec) {
-  return Py_BuildValue("f",
+  return PyFloat_FromDouble(
                        timespec->tv_sec + ((double)timespec->tv_nsec) / 1.0E9);
 }
 
@@ -116,67 +116,82 @@ static PyObject *pygrpc_status_code(grpc_status_code c_status_code) {
 }
 
 static PyObject *pygrpc_stop_event_args(grpc_event *c_event) {
-  return Py_BuildValue("(OOOOOOO)", stop_event_kind, Py_None, Py_None, Py_None,
-                       Py_None, Py_None, Py_None);
+  return PyTuple_Pack(7, stop_event_kind, Py_None, Py_None, Py_None,
+                      Py_None, Py_None, Py_None);
 }
 
 static PyObject *pygrpc_write_event_args(grpc_event *c_event) {
   PyObject *write_accepted =
       c_event->data.write_accepted == GRPC_OP_OK ? Py_True : Py_False;
-  return Py_BuildValue("(OOOOOOO)", write_event_kind, (PyObject *)c_event->tag,
-                       write_accepted, Py_None, Py_None, Py_None, Py_None);
+  return PyTuple_Pack(7, write_event_kind, (PyObject *)c_event->tag,
+                      write_accepted, Py_None, Py_None, Py_None, Py_None);
 }
 
 static PyObject *pygrpc_complete_event_args(grpc_event *c_event) {
   PyObject *complete_accepted =
       c_event->data.finish_accepted == GRPC_OP_OK ? Py_True : Py_False;
-  return Py_BuildValue("(OOOOOOO)", complete_event_kind,
-                       (PyObject *)c_event->tag, Py_None, complete_accepted,
-                       Py_None, Py_None, Py_None);
+  return PyTuple_Pack(7, complete_event_kind, (PyObject *)c_event->tag,
+                      Py_None, complete_accepted, Py_None, Py_None, Py_None);
 }
 
 static PyObject *pygrpc_service_event_args(grpc_event *c_event) {
   if (c_event->data.server_rpc_new.method == NULL) {
-    return Py_BuildValue("(OOOOOOO)", service_event_kind, c_event->tag,
-                         Py_None, Py_None, Py_None, Py_None, Py_None);
+    return PyTuple_Pack(7, service_event_kind, c_event->tag,
+                        Py_None, Py_None, Py_None, Py_None, Py_None);
   } else {
-    PyObject *method = PyBytes_FromString(c_event->data.server_rpc_new.method);
-    PyObject *host = PyBytes_FromString(c_event->data.server_rpc_new.host);
-    PyObject *service_deadline =
+    PyObject *method = NULL;
+    PyObject *host = NULL;
+    PyObject *service_deadline = NULL;
+    Call *call = NULL;
+    PyObject *service_acceptance = NULL;
+    PyObject *event_args = NULL;
+
+    method = PyBytes_FromString(c_event->data.server_rpc_new.method);
+    if (method == NULL) {
+      goto error;
+    }
+    host = PyBytes_FromString(c_event->data.server_rpc_new.host);
+    if (host == NULL) {
+      goto error;
+    }
+    service_deadline =
         pygrpc_as_py_time(&c_event->data.server_rpc_new.deadline);
-
-    Call *call;
-    PyObject *service_acceptance_args;
-    PyObject *service_acceptance;
-    PyObject *event_args;
+    if (service_deadline == NULL) {
+      goto error;
+    }
 
     call = PyObject_New(Call, &pygrpc_CallType);
+    if (call == NULL) {
+      goto error;
+    }
     call->c_call = c_event->call;
 
-    service_acceptance_args =
-        Py_BuildValue("(OOOO)", call, method, host, service_deadline);
-    Py_DECREF(call);
-    Py_DECREF(method);
-    Py_DECREF(host);
-    Py_DECREF(service_deadline);
-
     service_acceptance =
-        PyObject_CallObject(service_acceptance_class, service_acceptance_args);
-    Py_DECREF(service_acceptance_args);
+        PyObject_CallFunctionObjArgs(service_acceptance_class, call, method,
+                                     host, service_deadline, NULL);
+    if (service_acceptance == NULL) {
+      goto error;
+    }
+
+    event_args = PyTuple_Pack(7, service_event_kind,
+                              (PyObject *)c_event->tag, Py_None, Py_None,
+                              service_acceptance, Py_None, Py_None);
 
-    event_args = Py_BuildValue("(OOOOOOO)", service_event_kind,
-                               (PyObject *)c_event->tag, Py_None, Py_None,
-                               service_acceptance, Py_None, Py_None);
     Py_DECREF(service_acceptance);
+error:
+    Py_XDECREF(call);
+    Py_XDECREF(method);
+    Py_XDECREF(host);
+    Py_XDECREF(service_deadline);
+
     return event_args;
   }
 }
 
 static PyObject *pygrpc_read_event_args(grpc_event *c_event) {
   if (c_event->data.read == NULL) {
-    return Py_BuildValue("(OOOOOOO)", read_event_kind,
-                         (PyObject *)c_event->tag, Py_None, Py_None, Py_None,
-                         Py_None, Py_None);
+    return PyTuple_Pack(7, read_event_kind, (PyObject *)c_event->tag,
+                        Py_None, Py_None, Py_None, Py_None, Py_None);
   } else {
     size_t length;
     size_t offset;
@@ -198,9 +213,11 @@ static PyObject *pygrpc_read_event_args(grpc_event *c_event) {
     grpc_byte_buffer_reader_destroy(reader);
     bytes = PyBytes_FromStringAndSize(c_bytes, length);
     gpr_free(c_bytes);
-    event_args =
-        Py_BuildValue("(OOOOOOO)", read_event_kind, (PyObject *)c_event->tag,
-                      Py_None, Py_None, Py_None, bytes, Py_None);
+    if (bytes == NULL) {
+      return NULL;
+    }
+    event_args = PyTuple_Pack(7, read_event_kind, (PyObject *)c_event->tag,
+                              Py_None, Py_None, Py_None, bytes, Py_None);
     Py_DECREF(bytes);
     return event_args;
   }
@@ -208,15 +225,13 @@ static PyObject *pygrpc_read_event_args(grpc_event *c_event) {
 
 static PyObject *pygrpc_metadata_event_args(grpc_event *c_event) {
   /* TODO(nathaniel): Actual transmission of metadata. */
-  return Py_BuildValue("(OOOOOOO)", metadata_event_kind,
-                       (PyObject *)c_event->tag, Py_None, Py_None, Py_None,
-                       Py_None, Py_None);
+  return PyTuple_Pack(7, metadata_event_kind, (PyObject *)c_event->tag,
+                      Py_None, Py_None, Py_None, Py_None, Py_None);
 }
 
 static PyObject *pygrpc_finished_event_args(grpc_event *c_event) {
   PyObject *code;
   PyObject *details;
-  PyObject *status_args;
   PyObject *status;
   PyObject *event_args;
 
@@ -230,19 +245,26 @@ static PyObject *pygrpc_finished_event_args(grpc_event *c_event) {
   } else {
     details = PyBytes_FromString(c_event->data.finished.details);
   }
-  status_args = Py_BuildValue("(OO)", code, details);
+  if (details == NULL) {
+    return NULL;
+  }
+  status = PyObject_CallFunctionObjArgs(status_class, code, details, NULL);
   Py_DECREF(details);
-  status = PyObject_CallObject(status_class, status_args);
-  Py_DECREF(status_args);
-  event_args =
-      Py_BuildValue("(OOOOOOO)", finish_event_kind, (PyObject *)c_event->tag,
-                    Py_None, Py_None, Py_None, Py_None, status);
+  if (status == NULL) {
+    return NULL;
+  }
+  event_args = PyTuple_Pack(7, finish_event_kind, (PyObject *)c_event->tag,
+                            Py_None, Py_None, Py_None, Py_None, status);
   Py_DECREF(status);
   return event_args;
 }
 
 static int pygrpc_completion_queue_init(CompletionQueue *self, PyObject *args,
                                         PyObject *kwds) {
+  static char *kwlist[] = {NULL};
+  if (!PyArg_ParseTupleAndKeywords(args, kwds, ":CompletionQueue", kwlist)) {
+    return -1;
+  }
   self->c_completion_queue = grpc_completion_queue_create();
   return 0;
 }
@@ -262,7 +284,7 @@ static PyObject *pygrpc_completion_queue_get(CompletionQueue *self,
   PyObject *event_args;
   PyObject *event;
 
-  if (!(PyArg_ParseTuple(args, "O", &deadline))) {
+  if (!(PyArg_ParseTuple(args, "O:get", &deadline))) {
     return NULL;
   }
 
@@ -270,6 +292,9 @@ static PyObject *pygrpc_completion_queue_get(CompletionQueue *self,
     deadline_timespec = gpr_inf_future;
   } else {
     double_deadline = PyFloat_AsDouble(deadline);
+    if (PyErr_Occurred()) {
+      return NULL;
+    }
     deadline_timespec = gpr_time_from_nanos((long)(double_deadline * 1.0E9));
   }
 
@@ -339,7 +364,7 @@ static PyMethodDef methods[] = {
     {NULL}};
 
 PyTypeObject pygrpc_CompletionQueueType = {
-    PyObject_HEAD_INIT(NULL)0,                   /*ob_size*/
+    PyVarObject_HEAD_INIT(NULL, 0)
     "_gprc.CompletionQueue",                     /*tp_name*/
     sizeof(CompletionQueue),                     /*tp_basicsize*/
     0,                                           /*tp_itemsize*/
@@ -375,6 +400,8 @@ PyTypeObject pygrpc_CompletionQueueType = {
     0,                                           /* tp_descr_set */
     0,                                           /* tp_dictoffset */
     (initproc)pygrpc_completion_queue_init,      /* tp_init */
+    0,                                           /* tp_alloc */
+    PyType_GenericNew,                           /* tp_new */
 };
 
 static int pygrpc_get_status_codes(PyObject *datatypes_module) {
@@ -503,7 +530,6 @@ int pygrpc_add_completion_queue(PyObject *module) {
   char *datatypes_module_path = "grpc._adapter._datatypes";
   PyObject *datatypes_module = PyImport_ImportModule(datatypes_module_path);
   if (datatypes_module == NULL) {
-    PyErr_SetString(PyExc_ImportError, datatypes_module_path);
     return -1;
   }
   status_class = PyObject_GetAttrString(datatypes_module, "Status");
@@ -512,29 +538,21 @@ int pygrpc_add_completion_queue(PyObject *module) {
   event_class = PyObject_GetAttrString(datatypes_module, "Event");
   if (status_class == NULL || service_acceptance_class == NULL ||
       event_class == NULL) {
-    PyErr_SetString(PyExc_ImportError, "Missing classes in _datatypes module!");
     return -1;
   }
   if (pygrpc_get_status_codes(datatypes_module) == -1) {
-    PyErr_SetString(PyExc_ImportError, "Status codes import broken!");
     return -1;
   }
   if (pygrpc_get_event_kinds(event_class) == -1) {
-    PyErr_SetString(PyExc_ImportError, "Event kinds import broken!");
     return -1;
   }
   Py_DECREF(datatypes_module);
 
-  pygrpc_CompletionQueueType.tp_new = PyType_GenericNew;
   if (PyType_Ready(&pygrpc_CompletionQueueType) < 0) {
-    PyErr_SetString(PyExc_RuntimeError,
-                    "Error defining pygrpc_CompletionQueueType!");
     return -1;
   }
   if (PyModule_AddObject(module, "CompletionQueue",
                          (PyObject *)&pygrpc_CompletionQueueType) == -1) {
-    PyErr_SetString(PyExc_ImportError,
-                    "Couldn't add CompletionQueue type to module!");
     return -1;
   }
   return 0;
diff --git a/src/python/src/grpc/_adapter/_server.c b/src/python/src/grpc/_adapter/_server.c
index d4bf5fb8f6..ae7ae5b5d2 100644
--- a/src/python/src/grpc/_adapter/_server.c
+++ b/src/python/src/grpc/_adapter/_server.c
@@ -43,9 +43,11 @@
 static int pygrpc_server_init(Server *self, PyObject *args, PyObject *kwds) {
   const PyObject *completion_queue;
   PyObject *server_credentials;
-  if (!(PyArg_ParseTuple(args, "O!O", &pygrpc_CompletionQueueType,
-                         &completion_queue, &server_credentials))) {
-    self->c_server = NULL;
+  static char *kwlist[] = {"completion_queue", "server_credentials", NULL};
+
+  if (!PyArg_ParseTupleAndKeywords(args, kwds, "O!O:Server", kwlist,
+                                   &pygrpc_CompletionQueueType,
+                                   &completion_queue, &server_credentials)) {
     return -1;
   }
   if (server_credentials == Py_None) {
@@ -59,7 +61,9 @@ static int pygrpc_server_init(Server *self, PyObject *args, PyObject *kwds) {
         ((CompletionQueue *)completion_queue)->c_completion_queue, NULL);
     return 0;
   } else {
-    self->c_server = NULL;
+    PyErr_Format(PyExc_TypeError,
+                 "server_credentials must be _grpc.ServerCredentials, not %s",
+                 Py_TYPE(server_credentials)->tp_name);
     return -1;
   }
 }
@@ -74,7 +78,9 @@ static void pygrpc_server_dealloc(Server *self) {
 static PyObject *pygrpc_server_add_http2_addr(Server *self, PyObject *args) {
   const char *addr;
   int port;
-  PyArg_ParseTuple(args, "s", &addr);
+  if (!PyArg_ParseTuple(args, "s:add_http2_addr", &addr)) {
+    return NULL;
+  }
 
   port = grpc_server_add_http2_port(self->c_server, addr);
   if (port == 0) {
@@ -89,7 +95,9 @@ static PyObject *pygrpc_server_add_secure_http2_addr(Server *self,
                                                      PyObject *args) {
   const char *addr;
   int port;
-  PyArg_ParseTuple(args, "s", &addr);
+  if (!PyArg_ParseTuple(args, "s:add_secure_http2_addr", &addr)) {
+    return NULL;
+  }
   port = grpc_server_add_secure_http2_port(self->c_server, addr);
   if (port == 0) {
     PyErr_SetString(PyExc_RuntimeError, "Couldn't add port to server!");
@@ -104,15 +112,10 @@ static PyObject *pygrpc_server_start(Server *self) {
   Py_RETURN_NONE;
 }
 
-static const PyObject *pygrpc_server_service(Server *self, PyObject *args) {
-  const PyObject *tag;
+static const PyObject *pygrpc_server_service(Server *self, PyObject *tag) {
   grpc_call_error call_error;
   const PyObject *result;
 
-  if (!(PyArg_ParseTuple(args, "O", &tag))) {
-    return NULL;
-  }
-
   call_error = grpc_server_request_call_old(self->c_server, (void *)tag);
 
   result = pygrpc_translate_call_error(call_error);
@@ -135,13 +138,13 @@ static PyMethodDef methods[] = {
      METH_VARARGS, "Add a secure HTTP2 address."},
     {"start", (PyCFunction)pygrpc_server_start, METH_NOARGS,
      "Starts the server."},
-    {"service", (PyCFunction)pygrpc_server_service, METH_VARARGS,
+    {"service", (PyCFunction)pygrpc_server_service, METH_O,
      "Services a call."},
     {"stop", (PyCFunction)pygrpc_server_stop, METH_NOARGS, "Stops the server."},
     {NULL}};
 
 static PyTypeObject pygrpc_ServerType = {
-    PyObject_HEAD_INIT(NULL)0,         /*ob_size*/
+    PyVarObject_HEAD_INIT(NULL, 0)
     "_gprc.Server",                    /*tp_name*/
     sizeof(Server),                    /*tp_basicsize*/
     0,                                 /*tp_itemsize*/
@@ -177,17 +180,16 @@ static PyTypeObject pygrpc_ServerType = {
     0,                                 /* tp_descr_set */
     0,                                 /* tp_dictoffset */
     (initproc)pygrpc_server_init,      /* tp_init */
+    0,                                 /* tp_alloc */
+    PyType_GenericNew,                 /* tp_new */
 };
 
 int pygrpc_add_server(PyObject *module) {
-  pygrpc_ServerType.tp_new = PyType_GenericNew;
   if (PyType_Ready(&pygrpc_ServerType) < 0) {
-    PyErr_SetString(PyExc_RuntimeError, "Error defining pygrpc_ServerType!");
     return -1;
   }
   if (PyModule_AddObject(module, "Server", (PyObject *)&pygrpc_ServerType) ==
       -1) {
-    PyErr_SetString(PyExc_ImportError, "Couldn't add Server type to module!");
     return -1;
   }
   return 0;
diff --git a/src/python/src/grpc/_adapter/_server_credentials.c b/src/python/src/grpc/_adapter/_server_credentials.c
index ae85fd3eb7..06e6b94974 100644
--- a/src/python/src/grpc/_adapter/_server_credentials.c
+++ b/src/python/src/grpc/_adapter/_server_credentials.c
@@ -47,21 +47,20 @@ static int pygrpc_server_credentials_init(ServerCredentials *self,
   PyObject *iterator;
   int i;
   PyObject *pair;
+  static char *kwlist[] = {"root_credentials", "pair_sequence", NULL};
 
-  if (!(PyArg_ParseTuple(args, "zO", &root_certificates, &pair_sequence))) {
-    self->c_server_credentials = NULL;
+  if (!PyArg_ParseTupleAndKeywords(args, kwds, "zO:ServerCredentials", kwlist,
+                                   &root_certificates, &pair_sequence)) {
     return -1;
   }
 
   pair_count = PySequence_Length(pair_sequence);
   if (pair_count == -1) {
-    self->c_server_credentials = NULL;
     return -1;
   }
 
   iterator = PyObject_GetIter(pair_sequence);
   if (iterator == NULL) {
-    self->c_server_credentials = NULL;
     return -1;
   }
   pairs = gpr_malloc(pair_count * sizeof(grpc_ssl_pem_key_cert_pair));
@@ -72,8 +71,8 @@ static int pygrpc_server_credentials_init(ServerCredentials *self,
       error = 1;
       break;
     }
-    if (!(PyArg_ParseTuple(pair, "ss", &pairs[i].private_key,
-                           &pairs[i].cert_chain))) {
+    if (!PyArg_ParseTuple(pair, "ss", &pairs[i].private_key,
+                           &pairs[i].cert_chain)) {
       error = 1;
       Py_DECREF(pair);
       break;
@@ -83,7 +82,6 @@ static int pygrpc_server_credentials_init(ServerCredentials *self,
   Py_DECREF(iterator);
 
   if (error) {
-    self->c_server_credentials = NULL;
     gpr_free(pairs);
     return -1;
   } else {
@@ -102,7 +100,7 @@ static void pygrpc_server_credentials_dealloc(ServerCredentials *self) {
 }
 
 PyTypeObject pygrpc_ServerCredentialsType = {
-    PyObject_HEAD_INIT(NULL)0,                     /*ob_size*/
+    PyVarObject_HEAD_INIT(NULL, 0)
     "_grpc.ServerCredencials",                     /*tp_name*/
     sizeof(ServerCredentials),                     /*tp_basicsize*/
     0,                                             /*tp_itemsize*/
@@ -138,19 +136,16 @@ PyTypeObject pygrpc_ServerCredentialsType = {
     0,                                             /* tp_descr_set */
     0,                                             /* tp_dictoffset */
     (initproc)pygrpc_server_credentials_init,      /* tp_init */
+    0,                                             /* tp_alloc */
+    PyType_GenericNew,                             /* tp_new */
 };
 
 int pygrpc_add_server_credentials(PyObject *module) {
-  pygrpc_ServerCredentialsType.tp_new = PyType_GenericNew;
   if (PyType_Ready(&pygrpc_ServerCredentialsType) < 0) {
-    PyErr_SetString(PyExc_RuntimeError,
-                    "Error defining pygrpc_ServerCredentialsType!");
     return -1;
   }
   if (PyModule_AddObject(module, "ServerCredentials",
                          (PyObject *)&pygrpc_ServerCredentialsType) == -1) {
-    PyErr_SetString(PyExc_ImportError,
-                    "Couldn't add ServerCredentials type to module!");
     return -1;
   }
   return 0;