Split handling HTTP requests into separate class

Until now HTTP requests were handled in the same class as incoming
connections (http.server.HttpServer). With this change the request
handling is delegated to a separate class which can be re-used in tests
without creating a socket, etc.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

diff --git a/lib/http/server.py b/lib/http/server.py
index 407f5bb..8e9a638 100644 (file)
--- a/lib/http/server.py
+++ b/lib/http/server.py
@@ -276,11 +276,12 @@ class HttpServerRequestExecutor(object):
   READ_TIMEOUT = 10
   CLOSE_TIMEOUT = 1
 
-  def __init__(self, server, sock, client_addr):
+  def __init__(self, server, handler, sock, client_addr):
     """Initializes this class.
 
     """
     self.server = server
+    self.handler = handler
     self.sock = sock
     self.client_addr = client_addr
 
@@ -324,7 +325,7 @@ class HttpServerRequestExecutor(object):
 
             (self.response_msg.start_line.code, self.response_msg.headers,
              self.response_msg.body) = \
-              HandleServerRequest(self.server, self.request_msg)
+              HandleServerRequest(self.handler, self.request_msg)
 
             # Only wait for client to close if we didn't have any exception.
             force_close = False
@@ -363,6 +364,7 @@ class HttpServerRequestExecutor(object):
     """Sends the response to the client.
 
     """
+    # HttpMessage.start_line can be of different types, pylint: disable=E1103
     if self.response_msg.start_line.code is None:
       return
 
@@ -443,12 +445,10 @@ class HttpServerRequestExecutor(object):
 class HttpServer(http.HttpBase, asyncore.dispatcher):
   """Generic HTTP server class
 
-  Users of this class must subclass it and override the HandleRequest function.
-
   """
   MAX_CHILDREN = 20
 
-  def __init__(self, mainloop, local_address, port,
+  def __init__(self, mainloop, local_address, port, handler,
                ssl_params=None, ssl_verify_peer=False,
                request_executor_class=None):
     """Initializes the HTTP server
@@ -480,6 +480,7 @@ class HttpServer(http.HttpBase, asyncore.dispatcher):
     self.mainloop = mainloop
     self.local_address = local_address
     self.port = port
+    self.handler = handler
     family = netutils.IPAddress.GetAddressFamily(local_address)
     self.socket = self._CreateSocket(ssl_params, ssl_verify_peer, family)
 
@@ -560,7 +561,7 @@ class HttpServer(http.HttpBase, asyncore.dispatcher):
         # In case the handler code uses temporary files
         utils.ResetTempfileModule()
 
-        self.request_executor(self, connection, client_addr)
+        self.request_executor(self, self.handler, connection, client_addr)
       except Exception: # pylint: disable=W0703
         logging.exception("Error while handling request from %s:%s",
                           client_addr[0], client_addr[1])
@@ -569,6 +570,14 @@ class HttpServer(http.HttpBase, asyncore.dispatcher):
     else:
       self._children.append(pid)
 
+
+class HttpServerHandler(object):
+  """Base class for handling HTTP server requests.
+
+  Users of this class must subclass it and override the L{HandleRequest}
+  function.
+
+  """
   def PreHandleRequest(self, req):
     """Called before handling a request.
 

diff --git a/lib/server/noded.py b/lib/server/noded.py
index ba3cb90..3442d64 100644 (file)
--- a/lib/server/noded.py
+++ b/lib/server/noded.py
@@ -121,7 +121,7 @@ class MlockallRequestExecutor(http.server.HttpServerRequestExecutor):
     http.server.HttpServerRequestExecutor.__init__(self, *args, **kwargs)
 
 
-class NodeHttpServer(http.server.HttpServer):
+class NodeRequestHandler(http.server.HttpServerHandler):
   """The server implementation.
 
   This class holds all methods exposed over the RPC interface.
@@ -130,8 +130,8 @@ class NodeHttpServer(http.server.HttpServer):
   # too many public methods, and unused args - all methods get params
   # due to the API
   # pylint: disable=R0904,W0613
-  def __init__(self, *args, **kwargs):
-    http.server.HttpServer.__init__(self, *args, **kwargs)
+  def __init__(self):
+    http.server.HttpServerHandler.__init__(self)
     self.noded_pid = os.getpid()
 
   def HandleRequest(self, req):
@@ -1051,11 +1051,15 @@ def PrepNoded(options, _):
     # startup of the whole node daemon because of this
     logging.critical("Can't init/verify the queue, proceeding anyway: %s", err)
 
+  handler = NodeRequestHandler()
+
   mainloop = daemon.Mainloop()
-  server = NodeHttpServer(mainloop, options.bind_address, options.port,
-                          ssl_params=ssl_params, ssl_verify_peer=True,
-                          request_executor_class=request_executor_class)
+  server = \
+    http.server.HttpServer(mainloop, options.bind_address, options.port,
+      handler, ssl_params=ssl_params, ssl_verify_peer=True,
+      request_executor_class=request_executor_class)
   server.Start()
+
   return (mainloop, server)
 
 

diff --git a/lib/server/rapi.py b/lib/server/rapi.py
index f6fe285..b195711 100644 (file)
--- a/lib/server/rapi.py
+++ b/lib/server/rapi.py
@@ -82,17 +82,17 @@ class JsonErrorRequestExecutor(http.server.HttpServerRequestExecutor):
     return serializer.DumpJson(values)
 
 
-class RemoteApiHttpServer(http.auth.HttpServerRequestAuthentication,
-                          http.server.HttpServer):
+class RemoteApiHandler(http.auth.HttpServerRequestAuthentication,
+                       http.server.HttpServerHandler):
   """REST Request Handler Class.
 
   """
   AUTH_REALM = "Ganeti Remote API"
 
-  def __init__(self, *args, **kwargs):
+  def __init__(self):
     # pylint: disable=W0233
     # it seems pylint doesn't see the second parent class there
-    http.server.HttpServer.__init__(self, *args, **kwargs)
+    http.server.HttpServerHandler.__init__(self)
     http.auth.HttpServerRequestAuthentication.__init__(self)
     self._resmap = connector.Mapper()
     self._users = None
@@ -308,21 +308,19 @@ def PrepRapi(options, _):
   """Prep remote API function, executed with the PID file held.
 
   """
-
   mainloop = daemon.Mainloop()
-  server = RemoteApiHttpServer(mainloop, options.bind_address, options.port,
-                               ssl_params=options.ssl_params,
-                               ssl_verify_peer=False,
-                               request_executor_class=JsonErrorRequestExecutor)
+  handler = RemoteApiHandler()
 
   # Setup file watcher (it'll be driven by asyncore)
   SetupFileWatcher(constants.RAPI_USERS_FILE,
-                   compat.partial(server.LoadUsers, constants.RAPI_USERS_FILE))
+                   compat.partial(handler.LoadUsers, constants.RAPI_USERS_FILE))
 
-  server.LoadUsers(constants.RAPI_USERS_FILE)
+  handler.LoadUsers(constants.RAPI_USERS_FILE)
 
-  # pylint: disable=E1101
-  # it seems pylint doesn't see the second parent class there
+  server = \
+    http.server.HttpServer(mainloop, options.bind_address, options.port,
+      handler, ssl_params=options.ssl_params, ssl_verify_peer=False,
+      request_executor_class=JsonErrorRequestExecutor)
   server.Start()
 
   return (mainloop, server)