Fix: Fail early when database server does not respond

Author: amotl

CHANGES.rst

@@ -5,6 +5,9 @@ Changes for crate
 Unreleased
 ==========
 
+- Changed connection behaviour to fail early when database server
+  does not respond
+
 2025/01/30 2.0.0
 ================
 

docs/by-example/client.rst

@@ -35,12 +35,8 @@ respond, the request is automatically routed to the next server:
     >>> connection = client.connect([invalid_host, crate_host])
     >>> connection.close()
 
-If no ``servers`` are given, the default one ``http://127.0.0.1:4200`` is used:
-
-    >>> connection = client.connect()
-    >>> connection.client._active_servers
-    ['http://127.0.0.1:4200']
-    >>> connection.close()
+If no ``servers`` are supplied to the ``connect`` method, the default address
+``http://127.0.0.1:4200`` is used.
 
 If the option ``error_trace`` is set to ``True``, the client will print a whole
 traceback if a server error occurs:

src/crate/client/connection.py

@@ -20,6 +20,7 @@
 # software solely pursuant to the terms of the relevant commercial agreement.
 
 from verlib2 import Version
+from verlib2.packaging.version import InvalidVersion
 
 from .blob import BlobContainer
 from .cursor import Cursor
@@ -197,14 +198,20 @@ def get_blob_container(self, container_name):
 
     def _lowest_server_version(self):
         lowest = None
+        last_connection_error = None
         for server in self.client.active_servers:
             try:
                 _, _, version = self.client.server_infos(server)
                 version = Version(version)
-            except (ValueError, ConnectionError):
+            except ConnectionError as ex:
+                last_connection_error = ex
+                continue
+            except (ValueError, InvalidVersion):
                 continue
             if not lowest or version < lowest:
                 lowest = version
+        if lowest is None and last_connection_error is not None:
+            raise last_connection_error
         return lowest or Version("0.0.0")
 
     def __repr__(self):

tests/client/test_connection.py

@@ -3,6 +3,7 @@
 
 from urllib3 import Timeout
 
+import crate.client.exceptions
 from crate.client import connect
 from crate.client.connection import Connection
 from crate.client.http import Client
@@ -11,6 +12,7 @@
 
 
 class ConnectionTest(TestCase):
+
     def test_connection_mock(self):
         """
         For testing purposes it is often useful to replace the client used for
@@ -21,21 +23,27 @@ def test_connection_mock(self):
         """
 
         class MyConnectionClient:
-            active_servers = ["localhost:4200"]
+            active_servers = [crate_host]
 
             def __init__(self):
                 pass
 
             def server_infos(self, server):
-                return ("localhost:4200", "my server", "0.42.0")
+                return (crate_host, "my server", "0.42.0")
 
         connection = connect([crate_host], client=MyConnectionClient())
         self.assertIsInstance(connection, Connection)
         self.assertEqual(
             connection.client.server_infos("foo"),
-            ("localhost:4200", "my server", "0.42.0"),
+            (crate_host, "my server", "0.42.0"),
         )
 
+    def test_invalid_server_address(self):
+        client = Client(servers="localhost:4202")
+        with self.assertRaises(crate.client.exceptions.ConnectionError) as ex:
+            connect(client=client)
+        self.assertIn("Server not available", ex.exception.message)
+
     def test_lowest_server_version(self):
         infos = [
             (None, None, "0.42.3"),
@@ -50,14 +58,14 @@ def test_lowest_server_version(self):
         connection.close()
 
     def test_invalid_server_version(self):
-        client = Client(servers="localhost:4200")
+        client = Client(servers=crate_host)
         client.server_infos = lambda server: (None, None, "No version")
         connection = connect(client=client)
         self.assertEqual((0, 0, 0), connection.lowest_server_version.version)
         connection.close()
 
     def test_context_manager(self):
-        with connect("localhost:4200") as conn:
+        with connect(crate_host) as conn:
             pass
         self.assertEqual(conn._closed, True)
 
@@ -70,7 +78,7 @@ def test_with_timezone(self):
         """
 
         tz_mst = datetime.timezone(datetime.timedelta(hours=7), name="MST")
-        connection = connect("localhost:4200", time_zone=tz_mst)
+        connection = connect(crate_host, time_zone=tz_mst)
         cursor = connection.cursor()
         self.assertEqual(cursor.time_zone.tzname(None), "MST")
         self.assertEqual(
@@ -88,20 +96,20 @@ def test_timeout_float(self):
         """
         Verify setting the timeout value as a scalar (float) works.
         """
-        with connect("localhost:4200", timeout=2.42) as conn:
+        with connect(crate_host, timeout=2.42) as conn:
             self.assertEqual(conn.client._pool_kw["timeout"], 2.42)
 
     def test_timeout_string(self):
         """
         Verify setting the timeout value as a scalar (string) works.
         """
-        with connect("localhost:4200", timeout="2.42") as conn:
+        with connect(crate_host, timeout="2.42") as conn:
             self.assertEqual(conn.client._pool_kw["timeout"], 2.42)
 
     def test_timeout_object(self):
         """
         Verify setting the timeout value as a Timeout object works.
         """
         timeout = Timeout(connect=2.42, read=0.01)
-        with connect("localhost:4200", timeout=timeout) as conn:
+        with connect(crate_host, timeout=timeout) as conn:
             self.assertEqual(conn.client._pool_kw["timeout"], timeout)

tests/client/tests.py

@@ -34,7 +34,6 @@ def test_suite():
     suite.addTest(makeSuite(KeepAliveClientTest))
     suite.addTest(makeSuite(ThreadSafeHttpClientTest))
     suite.addTest(makeSuite(ParamsTest))
-    suite.addTest(makeSuite(ConnectionTest))
     suite.addTest(makeSuite(RetryOnTimeoutServerTest))
     suite.addTest(makeSuite(RequestsCaBundleTest))
     suite.addTest(makeSuite(TestUsernameSentAsHeader))
@@ -65,6 +64,10 @@ def test_suite():
     # Integration tests.
     layer = ensure_cratedb_layer()
 
+    s = makeSuite(ConnectionTest)
+    s.layer = layer
+    suite.addTest(s)
+
     s = doctest.DocFileSuite(
         "docs/by-example/http.rst",
         "docs/by-example/client.rst",