From 1262fe638d7343b94c1a5a47d8236048e0b89cea Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Tue, 6 May 2025 23:36:39 +0200 Subject: [PATCH 1/5] Error handling: Fail early when database cluster does not respond --- CHANGES.rst | 3 +++ docs/by-example/client.rst | 8 ++------ src/crate/client/connection.py | 9 ++++++++- tests/client/test_connection.py | 25 ++++++++++++++++--------- tests/client/tests.py | 5 ++++- 5 files changed, 33 insertions(+), 17 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index e9e73d94..68d8166d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,6 +5,9 @@ Changes for crate Unreleased ========== +- Changed connection behaviour to fail early if the database cluster + does not respond + 2025/01/30 2.0.0 ================ diff --git a/docs/by-example/client.rst b/docs/by-example/client.rst index 6e8f08df..e8d8f29d 100644 --- a/docs/by-example/client.rst +++ b/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: diff --git a/src/crate/client/connection.py b/src/crate/client/connection.py index b0a2a15b..9a0c1108 100644 --- a/src/crate/client/connection.py +++ b/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): diff --git a/tests/client/test_connection.py b/tests/client/test_connection.py index 0cc5e1ef..4a057c26 100644 --- a/tests/client/test_connection.py +++ b/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 @@ -21,21 +22,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 +57,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 +77,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,14 +95,14 @@ 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): @@ -103,5 +110,5 @@ 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) diff --git a/tests/client/tests.py b/tests/client/tests.py index 2e6619b9..325d427c 100644 --- a/tests/client/tests.py +++ b/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", From b74ce4b85a626f19ef15dfd8bdf0759875c2730c Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 9 May 2025 13:22:51 +0200 Subject: [PATCH 2/5] Error handling: Only re-raise ConnectionError when all servers fail The procedure will collect all `ConnectionError` instances and include them into the exception message of the final `ConnectionError`. --- src/crate/client/connection.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/crate/client/connection.py b/src/crate/client/connection.py index 9a0c1108..18ad2595 100644 --- a/src/crate/client/connection.py +++ b/src/crate/client/connection.py @@ -198,20 +198,21 @@ def get_blob_container(self, container_name): def _lowest_server_version(self): lowest = None - last_connection_error = None + server_count = len(self.client.active_servers) + connection_errors = [] for server in self.client.active_servers: try: _, _, version = self.client.server_infos(server) version = Version(version) except ConnectionError as ex: - last_connection_error = ex + connection_errors.append(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 + if connection_errors and len(connection_errors) == server_count: + raise ConnectionError(str(connection_errors)) return lowest or Version("0.0.0") def __repr__(self): From d22fde9648f063bdef688e2b033cdf7f8d21f6bf Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 9 May 2025 13:46:39 +0200 Subject: [PATCH 3/5] Error handling: Use JSON for bundling multiple ConnectionError instances All `ConnectionError` instances that have been collected will be serialized into JSON now. --- src/crate/client/connection.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/crate/client/connection.py b/src/crate/client/connection.py index 18ad2595..a3a91e20 100644 --- a/src/crate/client/connection.py +++ b/src/crate/client/connection.py @@ -18,6 +18,7 @@ # However, if you have executed another commercial license agreement # with Crate these terms will supersede the license and you may use the # software solely pursuant to the terms of the relevant commercial agreement. +import json from verlib2 import Version from verlib2.packaging.version import InvalidVersion @@ -212,7 +213,7 @@ def _lowest_server_version(self): if not lowest or version < lowest: lowest = version if connection_errors and len(connection_errors) == server_count: - raise ConnectionError(str(connection_errors)) + raise ConnectionError(json.dumps(list(map(str, connection_errors)))) return lowest or Version("0.0.0") def __repr__(self): From 51ead0a737df62e097305d94a9ac67befd681eb9 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Sat, 10 May 2025 16:26:30 +0200 Subject: [PATCH 4/5] Dependencies: Update to verlib2>=0.3 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 386b3c35..8e27d622 100644 --- a/setup.py +++ b/setup.py @@ -56,7 +56,7 @@ def read(path): install_requires=[ "orjson<4", "urllib3", - "verlib2", + "verlib2>=0.3", ], extras_require={ "doc": [ From 891fc3cc133daec2d3076ccdb79fa9d5bfe752e7 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Sat, 10 May 2025 16:29:10 +0200 Subject: [PATCH 5/5] Chore: Update project metadata to use SPDX license identifier ... as suggested by warning message emitted by `python -m build`. --- setup.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/setup.py b/setup.py index 8e27d622..c576057e 100644 --- a/setup.py +++ b/setup.py @@ -49,7 +49,8 @@ def read(path): long_description=long_description, long_description_content_type="text/x-rst", platforms=["any"], - license="Apache License 2.0", + license="Apache-2.0", + license_files=["LICENSE"], keywords="cratedb db api dbapi database sql http rdbms olap", packages=find_namespace_packages("src"), package_dir={"": "src"}, @@ -82,7 +83,6 @@ def read(path): classifiers=[ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", - "License :: OSI Approved :: Apache Software License", "Operating System :: OS Independent", "Programming Language :: Python", "Programming Language :: Python :: 3",