Add PR suggestions

src/epplibwrapper/client.py

@@ -111,21 +111,21 @@ class EPPLibWrapper:
                 self.start_connection_pool()
         except (ValueError, ParsingError) as err:
             message = f"{cmd_type} failed to execute due to some syntax error."
-            logger.error(message, exc_info=True)
+            logger.error(f"{message} Error: {err}", exc_info=True)
             raise RegistryError(message) from err
         except TransportError as err:
             message = f"{cmd_type} failed to execute due to a connection error."
-            logger.error(message, exc_info=True)
+            logger.error(f"{message} Error: {err}", exc_info=True)
             raise RegistryError(message) from err
         except LoginError as err:
-            # For linter
+            # For linter due to it not liking this line length
             text = "failed to execute due to a registry login error."
             message = f"{cmd_type} {text}"
-            logger.error(message, exc_info=True)
+            logger.error(f"{message} Error: {err}", exc_info=True)
             raise RegistryError(message) from err
         except Exception as err:
             message = f"{cmd_type} failed to execute due to an unknown error."
-            logger.error(message, exc_info=True)
+            logger.error(f"{message} Error: {err}", exc_info=True)
             raise RegistryError(message) from err
         else:
             if response.code >= 2000:
@@ -155,6 +155,8 @@ class EPPLibWrapper:
             except RegistryError as err:
                 raise err
             finally:
+                # Code execution will halt after here.
+                # The end user will need to recall .send.
                 self.start_connection_pool()
 
         counter = 0  # we'll try 3 times

src/epplibwrapper/socket.py

@@ -48,7 +48,7 @@ class Socket:
 
     def send(self, command):
         """Sends a command to the registry.
-        If the response code is >= 2000,
+        If the RegistryError code is >= 2000,
         then this function raises a LoginError.
         The calling function should handle this."""
         response = self.client.send(command)
@@ -59,7 +59,9 @@ class Socket:
         return response
 
     def is_login_error(self, code):
-        """Returns the result of code >= 2000"""
+        """Returns the result of code >= 2000 for RegistryError.
+        This indicates that something weird happened on the Registry,
+        and that we should return a LoginError."""
         return code >= 2000
 
     def test_connection_success(self):
@@ -90,7 +92,7 @@ class Socket:
 
                 # If we encounter a login error, fail
                 if self.is_login_error(response.code):
-                    logger.warning("was login error")
+                    logger.warning("A login error was found in test_connection_success")
                     return False
 
                 # Otherwise, just return true

src/epplibwrapper/tests/test_pool.py

@@ -28,6 +28,7 @@ class TestConnectionPool(TestCase):
     """Tests for our connection pooling behaviour"""
 
     def setUp(self):
+        # Mimic the settings added to settings.py
         self.pool_options = {
             # Current pool size
             "size": 1,

src/epplibwrapper/utility/pool.py

@@ -51,7 +51,8 @@ class EPPConnectionPool(ConnectionPool):
             self.keepalive = options["keepalive"]
 
         # Determines the period in which new
-        # gevent threads are spun up
+        # gevent threads are spun up. 
+        # This time period is in seconds. So for instance, .1 would be .1 seconds.
         self.spawn_frequency = 0.1
         if "spawn_frequency" in options:
             self.spawn_frequency = options["spawn_frequency"]
@@ -77,7 +78,7 @@ class EPPConnectionPool(ConnectionPool):
     def _keepalive(self, c):
         """Sends a command to the server to keep the connection alive."""
         try:
-            # Sends a ping to EPPLib
+            # Sends a ping to the registry via EPPLib
             c.send(Hello())
         except Exception as err:
             message = "Failed to keep the connection alive."
@@ -108,7 +109,7 @@ class EPPConnectionPool(ConnectionPool):
                 logger.info("No connections to kill.")
         except Exception as err:
             logger.error("Could not kill all connections.")
-            raise err
+            raise PoolError(code=PoolErrorCodes.KILL_ALL_FAILED) from err
 
     def populate_all_connections(self):
         """Generates the connection pool.

src/epplibwrapper/utility/pool_error.py

@@ -22,12 +22,20 @@ class PoolError(Exception):
         - 2000 KILL_ALL_FAILED
         - 2001 NEW_CONNECTION_FAILED
         - 2002 KEEP_ALIVE_FAILED
+    
+    Note: These are separate from the error codes returned from EppLib
     """
 
-    # For linter
+    # Used variables due to linter requirements
-    kill_failed = "Could not kill all connections."
+    kill_failed = "Could not kill all connections. Are multiple pools running?"
-    conn_failed = "Failed to execute due to a registry error."
+    conn_failed = (
-    alive_failed = "Failed to keep the connection alive."
+        "Failed to execute due to a registry error." 
+        " See previous logs to determine the cause of the error."
+    )
+    alive_failed = (
+    "Failed to keep the connection alive. " 
+    "It is likely that the registry returned a LoginError."
+    )
     _error_mapping = {
         PoolErrorCodes.KILL_ALL_FAILED: kill_failed,
         PoolErrorCodes.NEW_CONNECTION_FAILED: conn_failed,

src/epplibwrapper/utility/pool_status.py

@@ -1,5 +1,10 @@
 class PoolStatus:
-    """A list of Booleans to keep track of Pool Status"""
+    """A list of Booleans to keep track of Pool Status.
+    
+    pool_running -> bool: Tracks if the pool itself is active or not.
+    connection_success -> bool: Tracks if connection is possible with the registry.
+    pool_hanging -> pool: Tracks if the pool has exceeded its timeout period.
+    """
 
     def __init__(self):
         self.pool_running = False

src/registrar/config/settings.py

@@ -536,11 +536,12 @@ SECRET_REGISTRY_HOSTNAME = secret_registry_hostname
 
 # Use this variable to set the size of our connection pool in client.py
 # WARNING: Setting this value too high could cause frequent app crashes!
+# Having too many connections open could cause the sandbox to timeout,
+# as the spinup time could exceed the timeout time.
 EPP_CONNECTION_POOL_SIZE = 1
 
 # Determines the interval in which we ping open connections in seconds
 # Calculated as POOL_KEEP_ALIVE / EPP_CONNECTION_POOL_SIZE
-# WARNING: Setting this value too high could cause frequent app crashes!
 POOL_KEEP_ALIVE = 60
 
 # Determines how long we try to keep a pool alive for,