PBCKP-588 fixes after review

Author: v.shepard

testgres/logger.py

@@ -5,21 +5,6 @@
 import threading
 import time
 
-# create logger
-log = logging.getLogger('Testgres')
-
-if not log.handlers:
-    log.setLevel(logging.WARN)
-    # create console handler and set level to debug
-    ch = logging.StreamHandler()
-    ch.setLevel(logging.WARN)
-    # create formatter
-    formatter = logging.Formatter('\n%(asctime)s - %(name)s[%(levelname)s]: %(message)s')
-    # add formatter to ch
-    ch.setFormatter(formatter)
-    # add ch to logger
-    log.addHandler(ch)
-
 
 class TestgresLogger(threading.Thread):
     """

testgres/node.py

@@ -245,7 +245,7 @@ def child_processes(self):
         """
 
         # get a list of postmaster's children
-        children = self.os_ops.get_remote_children(self.pid)
+        children = self.os_ops.get_process_children(self.pid)
 
         return [ProcessProxy(p) for p in children]
 

testgres/operations/local_ops.py

@@ -8,7 +8,6 @@
 import psutil
 
 from testgres.exceptions import ExecUtilException
-from testgres.logger import log
 
 from .os_ops import OsOperations
 from .os_ops import pglib
@@ -53,7 +52,6 @@ def exec_command(self, cmd, wait_exit=False, verbose=False,
         """
         if isinstance(cmd, list):
             cmd = ' '.join(item.decode('utf-8') if isinstance(item, bytes) else item for item in cmd)
-        log.debug(f"Executing command: `{cmd}`")
 
         if os.name == 'nt':
             with tempfile.NamedTemporaryFile() as buf:
@@ -252,7 +250,7 @@ def get_pid(self):
         # Get current process id
         return os.getpid()
 
-    def get_remote_children(self, pid):
+    def get_process_children(self, pid):
         return psutil.Process(pid).children()
 
     # Database control

testgres/operations/os_ops.py

@@ -88,6 +88,9 @@ def get_pid(self):
         # Get current process id
         raise NotImplementedError()
 
+    def get_process_children(self, pid):
+        raise NotImplementedError()
+
     # Database control
     def db_connect(self, dbname, user, password=None, host="localhost", port=5432):
         raise NotImplementedError()

testgres/operations/remote_ops.py

@@ -9,7 +9,6 @@
 from paramiko import SSHClient
 
 from testgres.exceptions import ExecUtilException
-from testgres.logger import log
 
 from .os_ops import OsOperations
 from .os_ops import pglib
@@ -90,9 +89,9 @@ def _read_ssh_key(self):
                     key = paramiko.RSAKey.from_private_key_file(self.ssh_key)
                 return key
         except FileNotFoundError:
-            log.error(f"No such file or directory: '{self.ssh_key}'")
+            raise ExecUtilException(message=f"No such file or directory: '{self.ssh_key}'")
         except Exception as e:
-            log.error(f"An error occurred while reading the ssh key: {e}")
+            ExecUtilException(message=f"An error occurred while reading the ssh key: {e}")
 
     def exec_command(self, cmd: str, wait_exit=False, verbose=False, expect_error=False,
                      encoding=None, shell=True, text=False, input=None, stdout=None,
@@ -400,7 +399,7 @@ def get_pid(self):
         # Get current process id
         return int(self.exec_command("echo $$", encoding='utf-8'))
 
-    def get_remote_children(self, pid):
+    def get_process_children(self, pid):
         command = f"pgrep -P {pid}"
         stdin, stdout, stderr = self.ssh.exec_command(command)
         children = stdout.readlines()
@@ -414,8 +413,7 @@ def db_connect(self, dbname, user, password=None, host="127.0.0.1", port=5432, s
         - dbname (str): The name of the database to connect to.
         - user (str): The username for the database connection.
         - password (str, optional): The password for the database connection. Defaults to None.
-        - host (str, optional): The IP address of the remote system. Defaults to "127.0.0.1".
-        - hostname (str, optional): The hostname of the remote system. Defaults to "localhost".
+        - host (str, optional): The IP address of the remote system. Defaults to "localhost".
         - port (int, optional): The port number of the PostgreSQL service. Defaults to 5432.
 
         This function establishes a connection to a PostgreSQL database on the remote system using the specified
@@ -444,4 +442,4 @@ def db_connect(self, dbname, user, password=None, host="127.0.0.1", port=5432, s
             return conn
         except Exception as e:
             self.tunnel.stop()
-            raise e
+            raise ExecUtilException("Could not create db tunnel.")

testgres/utils.py

@@ -12,9 +12,8 @@
 
 from six import iteritems
 
-
+from .exceptions import ExecUtilException
 from .config import testgres_config as tconf
-from .logger import log
 
 # rows returned by PG_CONFIG
 _pg_config_data = {}
@@ -73,7 +72,7 @@ def execute_utility(args, logfile=None, verbose=False):
                 lines = [u'\n'] + ['# ' + line for line in out.splitlines()] + [u'\n']
                 tconf.os_ops.write(filename=logfile, data=lines)
         except IOError:
-            log.warn(f"Problem with writing to logfile `{logfile}` during run command `{args}`")
+            raise ExecUtilException(f"Problem with writing to logfile `{logfile}` during run command `{args}`")
     if verbose:
         return exit_status, out, error
     else:

tests/test_remote.py

@@ -11,7 +11,7 @@ def setup(self):
         self.operations = RemoteOperations(
             host="172.18.0.3",
             username="dev",
-            ssh_key='/home/vika/Desktop/work/probackup/dev-ee-probackup/container_files/postgres/ssh/id_ed25519'
+            ssh_key='../../container_files/postgres/ssh/id_ed25519'
         )
 
         yield

tests/test_simple.py

@@ -151,6 +151,8 @@ def test_init_unique_system_id(self):
                 self.assertGreater(id2, id1)
 
     def test_node_exit(self):
+        base_dir = None
+
         with self.assertRaises(QueryException):
             with get_new_node().init() as node:
                 base_dir = node.base_dir
@@ -252,27 +254,27 @@ def test_psql(self):
 
             # check returned values (1 arg)
             res = node.psql('select 1')
-            self.assertEqual((0, b'1\n', b''), res)
+            self.assertEqual(res, (0, b'1\n', b''))
 
             # check returned values (2 args)
             res = node.psql('postgres', 'select 2')
-            self.assertEqual((0, b'2\n', b''), res)
+            self.assertEqual(res, (0, b'2\n', b''))
 
             # check returned values (named)
             res = node.psql(query='select 3', dbname='postgres')
-            self.assertEqual((0, b'3\n', b''), res)
+            self.assertEqual(res, (0, b'3\n', b''))
 
             # check returned values (1 arg)
             res = node.safe_psql('select 4')
-            self.assertEqual(b'4\n', res)
+            self.assertEqual(res, b'4\n')
 
             # check returned values (2 args)
             res = node.safe_psql('postgres', 'select 5')
-            self.assertEqual(b'5\n', res)
+            self.assertEqual(res, b'5\n')
 
             # check returned values (named)
             res = node.safe_psql(query='select 6', dbname='postgres')
-            self.assertEqual(b'6\n', res)
+            self.assertEqual(res, b'6\n')
 
             # check feeding input
             node.safe_psql('create table horns (w int)')
@@ -612,7 +614,7 @@ def test_users(self):
         with get_new_node().init().start() as node:
             node.psql('create role test_user login')
             value = node.safe_psql('select 1', username='test_user')
-            self.assertEqual(b'1\n', value)
+            self.assertEqual(value, b'1\n')
 
     def test_poll_query_until(self):
         with get_new_node() as node:

tests/test_simple_remote.py

@@ -35,7 +35,8 @@
     NodeStatus, \
     ProcessType, \
     IsolationLevel, \
-    get_new_node, RemoteOperations
+    get_new_node, \
+    RemoteOperations
 
 from testgres import \
     get_bin_path, \
@@ -54,7 +55,7 @@
 
 os_ops = RemoteOperations(host='172.18.0.3',
                           username='dev',
-                          ssh_key='/home/vika/Desktop/work/probackup/dev-ee-probackup/container_files/postgres/ssh/id_ed25519')
+                          ssh_key='../../container_files/postgres/ssh/id_ed25519')
 testgres_config.set_os_ops(os_ops=os_ops)
 
 
@@ -92,8 +93,8 @@ def removing(f):
             os_ops.rmdirs(f, ignore_errors=True)
 
 
-def get_remote_node():
-    return get_new_node(host=os_ops.host, username=os_ops.username, ssh_key=os_ops.ssh_key)
+def get_remote_node(name=None):
+    return get_new_node(name=name, host=os_ops.host, username=os_ops.username, ssh_key=os_ops.ssh_key)
 
 
 class TestgresRemoteTests(unittest.TestCase):
@@ -696,7 +697,7 @@ def test_logging(self):
             'handlers': {
                 'file': {
                     'class': 'logging.FileHandler',
-                    'filename': logfile.name,
+                    'filename': logfile,
                     'formatter': 'base_format',
                     'level': logging.DEBUG,
                 },
@@ -717,7 +718,7 @@ def test_logging(self):
         with scoped_config(use_python_logging=True):
             node_name = 'master'
 
-            with get_new_node(name=node_name) as master:
+            with get_remote_node(name=node_name) as master:
                 master.init().start()
 
                 # execute a dummy query a few times
@@ -729,7 +730,7 @@ def test_logging(self):
                 time.sleep(0.1)
 
                 # check that master's port is found
-                with open(logfile.name, 'r') as log:
+                with open(logfile, 'r') as log:
                     lines = log.readlines()
                     self.assertTrue(any(node_name in s for s in lines))