From f8c3582715699b81c2bcf763bb63f57beef5aa81 Mon Sep 17 00:00:00 2001
From: Alexander Kukushkin <alexander.kukushkin@zalando.de>
Date: Mon, 1 Jun 2015 10:48:16 +0200
Subject: [PATCH] Refactor api:

Call is_running only when it's not possible to execute query against
governed postgres.
---
 helpers/api.py    | 57 ++++++++++++++++++++++-------------------------
 tests/test_api.py |  9 +-------
 2 files changed, 28 insertions(+), 38 deletions(-)

diff --git a/helpers/api.py b/helpers/api.py
index 06ff07da..53e3f2fb 100644
--- a/helpers/api.py
+++ b/helpers/api.py
@@ -10,21 +10,16 @@ if sys.hexversion >= 0x03000000:
 else:
     from BaseHTTPServer import BaseHTTPRequestHandler, HTTPServer
 
-
 logger = logging.getLogger(__name__)
 
 
 class RestApiHandler(BaseHTTPRequestHandler):
 
     def do_GET(self):
-        try:
-            response = self.get_postgresql_status()
-        except (psycopg2.OperationalError, psycopg2.InterfaceError):
-            logger.exception('get_postgresql_status')
-            response = {'running': False}
+        response = self.get_postgresql_status()
 
         path = '/master' if self.path == '/' else self.path
-        status_code = 200 if response['running'] and response['role'] in path else 503
+        status_code = 200 if response['running'] and 'role' in response and response['role'] in path else 503
 
         self.send_response(status_code)
         self.send_header('Content-Type', 'application/json')
@@ -32,29 +27,31 @@ class RestApiHandler(BaseHTTPRequestHandler):
         self.wfile.write(json.dumps(response).encode('utf-8'))
 
     def get_postgresql_status(self):
-        if not self.server.governor.postgresql.is_running():
-            return {'running': False}
-        cursor = self.server._cursor()
-        cursor.execute("""SELECT to_char(pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ'),
-                                 pg_is_in_recovery(),
-                                 CASE WHEN pg_is_in_recovery()
-                                      THEN null
-                                      ELSE pg_current_xlog_location() END,
-                                 pg_last_xlog_receive_location(),
-                                 pg_last_xlog_replay_location(),
-                                 pg_is_in_recovery() AND pg_is_xlog_replay_paused()""")
-        row = cursor.fetchone()
-        return {
-            'running': True,
-            'postmaster_start_time': row[0],
-            'role': 'slave' if row[1] else 'master',
-            'xlog': ({
-                'received_location': row[3],
-                'replayed_location': row[4],
-                'paused': row[5]} if row[1] else {
-                'location': row[2]
-            })
-        }
+        try:
+            cursor = self.server._cursor()
+            cursor.execute("""SELECT to_char(pg_postmaster_start_time(), 'YYYY-MM-DD HH24:MI:SS.MS TZ'),
+                                     pg_is_in_recovery(),
+                                     CASE WHEN pg_is_in_recovery()
+                                          THEN null
+                                          ELSE pg_current_xlog_location() END,
+                                     pg_last_xlog_receive_location(),
+                                     pg_last_xlog_replay_location(),
+                                     pg_is_in_recovery() AND pg_is_xlog_replay_paused()""")
+            row = cursor.fetchone()
+            return {
+                'running': True,
+                'postmaster_start_time': row[0],
+                'role': 'slave' if row[1] else 'master',
+                'xlog': ({
+                    'received_location': row[3],
+                    'replayed_location': row[4],
+                    'paused': row[5]} if row[1] else {
+                    'location': row[2]
+                })
+            }
+        except (psycopg2.OperationalError, psycopg2.InterfaceError):
+            logger.exception('get_postgresql_status')
+            return {'running': self.server.governor.postgresql.is_running()}
 
 
 class RestApiServer(HTTPServer, Thread):
diff --git a/tests/test_api.py b/tests/test_api.py
index 6b117b48..eff848a3 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -11,10 +11,6 @@ else:
     from StringIO import StringIO as IO
 
 
-def false(*args, **kwargs):
-    return False
-
-
 def throws(*args, **kwargs):
     raise psycopg2.OperationalError()
 
@@ -48,7 +44,7 @@ class MockRestApiServer(RestApiServer):
     def __init__(self, Handler, path, *args):
         self.governor = MockGovernor()
         if len(args) > 0:
-            self.governor.postgresql.is_running = args[0]
+            self._cursor = args[0]
         self._cursor_holder = None
         Handler(MockRequest(path), ('0.0.0.0', 8080), self)
 
@@ -61,6 +57,3 @@ class TestRestApiHandler(unittest.TestCase):
     def test_do_GET(self):
         MockRestApiServer(RestApiHandler, b'GET /')
         MockRestApiServer(RestApiHandler, b'GET /', throws)
-
-    def test_get_postgresql_status(self):
-        MockRestApiServer(RestApiHandler, b'GET /', false)