refactor: extract SQL statements to sql_constants.py

node_monitor/node_monitor.py

@@ -5,17 +5,17 @@
 import schedule
 import logging
 
-import node_monitor.ic_api as ic_api
-from node_monitor.ic_api import NodeProvider
 from node_monitor.bot_email import EmailBot
 from node_monitor.bot_slack import SlackBot
 from node_monitor.bot_telegram import TelegramBot
 from node_monitor.node_provider_db import NodeProviderDB
+from node_monitor.node_monitor_helpers.sql_constants import INSERT_SUBSCRIBER
 from node_monitor.node_monitor_helpers.get_compromised_nodes import \
     get_compromised_nodes
 from node_monitor.node_monitor_helpers.get_new_node_providers import \
     get_new_node_providers
 import node_monitor.node_monitor_helpers.messages as messages
+import node_monitor.ic_api as ic_api
 
 Seconds = int
 Principal = str
@@ -194,7 +194,8 @@ def update_node_provider_lookup_if_new(
             override_data: If provided, this arg will be used instead of 
                 live fetching Node Providers from the ic-api. Useful for testing.
         """
-        node_providers_api = override_api_data if override_api_data else ic_api.get_node_providers()
+        node_providers_api = \
+            override_api_data if override_api_data else ic_api.get_node_providers()
         node_providers_api_dict = {
             node_provider.principal_id: node_provider.display_name
             for node_provider in node_providers_api.node_providers
@@ -203,26 +204,11 @@ def update_node_provider_lookup_if_new(
         node_providers_new = get_new_node_providers(
             node_providers_api_dict, node_providers_db)
 
-        query = """
-            INSERT INTO subscribers (
-                node_provider_id,
-                notify_on_status_change,
-                notify_email,
-                notify_slack,
-                node_provider_name,
-                notify_telegram
-            ) VALUES (%s, %s, %s, %s, %s, %s)
-            ON CONFLICT (node_provider_id) DO UPDATE SET
-                notify_on_status_change = EXCLUDED.notify_on_status_change,
-                notify_email = EXCLUDED.notify_email,
-                notify_slack = EXCLUDED.notify_slack,
-                node_provider_name = EXCLUDED.node_provider_name,
-                notify_telegram = EXCLUDED.notify_telegram
-        """
         if node_providers_new:
             for node_provider in node_providers_new:
-                params = (node_provider.principal_id, False, False, False, node_provider.display_name, False)
-                self.node_provider_db.execute_insert(query, params)
+                params = (node_provider.principal_id, False, False, 
+                          False, node_provider.display_name, False)
+                self.node_provider_db.execute_write(INSERT_SUBSCRIBER, params)
 
 
 

node_monitor/node_monitor_helpers/sql_constants.py

@@ -0,0 +1,22 @@
+### SQL queries for table `subscribers`
+INSERT_SUBSCRIBER = """
+    INSERT INTO subscribers (
+        node_provider_id,
+        notify_on_status_change,
+        notify_email,
+        notify_slack,
+        node_provider_name,
+        notify_telegram
+    ) VALUES (%s, %s, %s, %s, %s, %s)
+    ON CONFLICT (node_provider_id) DO UPDATE SET
+        notify_on_status_change = EXCLUDED.notify_on_status_change,
+        notify_email = EXCLUDED.notify_email,
+        notify_slack = EXCLUDED.notify_slack,
+        node_provider_name = EXCLUDED.node_provider_name,
+        notify_telegram = EXCLUDED.notify_telegram
+"""
+
+DELETE_SUBSCRIBER = """
+    DELETE FROM subscribers
+    WHERE node_provider_id = %s
+"""

node_monitor/node_provider_db.py

@@ -151,7 +151,7 @@ def _execute1(self, sql: str, params: Tuple[Any, ...]) -> List[Tuple[Any, ...]]:
         return result
 
 
-    def execute_insert(self, sql: str, params: Tuple[Any, ...]) -> None:
+    def execute_write(self, sql: str, params: Tuple[Any, ...]) -> None:
         conn = self.pool.getconn()
         with conn.cursor() as cur:
             cur.execute(sql, params)

tests/test_node_monitor.py

@@ -232,7 +232,7 @@ def test_no_new_node_provider():
                      mock_slack_bot, mock_telegram_bot)
 
     nm.update_node_provider_lookup_if_new(cached['node_provider_control'])
-    assert mock_node_provider_db.execute_insert.call_count == 0
+    assert mock_node_provider_db.execute_write.call_count == 0
 
 
 
@@ -245,7 +245,7 @@ def test_one_new_node_provider():
                      mock_slack_bot, mock_telegram_bot)
 
     nm.update_node_provider_lookup_if_new(cached['new_node_providers'])
-    assert mock_node_provider_db.execute_insert.call_count == 2
+    assert mock_node_provider_db.execute_write.call_count == 2
 
 
 

tests/test_node_provider_db.py

@@ -2,6 +2,8 @@
 from devtools import debug
 
 from node_monitor.node_provider_db import NodeProviderDB
+from node_monitor.node_monitor_helpers.sql_constants import \
+    DELETE_SUBSCRIBER, INSERT_SUBSCRIBER
 from tests.conftest import cached
 import node_monitor.load_config as c
 
@@ -62,31 +64,19 @@ def test_validate_schema_node_label_lookup():
     expected_schema = NodeProviderDB.schema_table_node_label_lookup
     actual_schema = node_provider_db._get_schema('node_label_lookup')
     assert set(expected_schema.items()) <= set(actual_schema.items())
+
 
 @pytest.mark.db
 def test_insert_subscriber_crud():
+
+    # Insert new node providers
     new_node_providers = cached["new_node_providers"]  
-    query = """
-        INSERT INTO subscribers (
-            node_provider_id,
-            notify_on_status_change,
-            notify_email,
-            notify_slack,
-            node_provider_name,
-            notify_telegram
-        ) VALUES (%s, %s, %s, %s, %s, %s)
-        ON CONFLICT (node_provider_id) DO UPDATE SET
-            notify_on_status_change = EXCLUDED.notify_on_status_change,
-            notify_email = EXCLUDED.notify_email,
-            notify_slack = EXCLUDED.notify_slack,
-            node_provider_name = EXCLUDED.node_provider_name,
-            notify_telegram = EXCLUDED.notify_telegram
-    """
     for node_provider in new_node_providers.node_providers:
         params = (node_provider.principal_id, False, False, 
                     False, node_provider.display_name, False)
-        node_provider_db.execute_insert(query, params)
+        node_provider_db.execute_write(INSERT_SUBSCRIBER, params)
 
+    # Check new node providers were added properly
     subs = node_provider_db.get_subscribers_as_dict()
     assert subs['test-dummy-principal-1'] == \
         {'node_provider_id': 'test-dummy-principal-1', 'notify_on_status_change': False, 'notify_email': False,
@@ -95,21 +85,18 @@ def test_insert_subscriber_crud():
         {'node_provider_id': 'test-dummy-principal-2', 'notify_on_status_change': False, 'notify_email': False,
          'notify_slack': False, 'node_provider_name': 'Node Provider B', 'notify_telegram': False,}
 
+    # Check overwriting existing records occurs properly 
     params = ('test-dummy-principal-1', False, False, 
               False, 'Node Provider C', False)
-    node_provider_db.execute_insert(query, params)
+    node_provider_db.execute_write(INSERT_SUBSCRIBER, params)
     subs = node_provider_db.get_subscribers_as_dict()
     assert subs['test-dummy-principal-1'] == \
         {'node_provider_id': 'test-dummy-principal-1', 'notify_on_status_change': False, 'notify_email': False,
          'notify_slack': False, 'node_provider_name': 'Node Provider C', 'notify_telegram': False,}
 
-    delete_query = """
-        DELETE FROM subscribers
-        WHERE node_provider_id = %s
-    """
     # Delete subscribers
-    node_provider_db.execute_insert(delete_query, ('test-dummy-principal-1',))
-    node_provider_db.execute_insert(delete_query, ('test-dummy-principal-2',))
+    node_provider_db.execute_write(DELETE_SUBSCRIBER, ('test-dummy-principal-1',))
+    node_provider_db.execute_write(DELETE_SUBSCRIBER, ('test-dummy-principal-2',))
     subs = node_provider_db.get_subscribers_as_dict()
     assert 'test-dummy-principal-1' not in subs
     assert 'test-dummy-principal-2' not in subs