From 35151c1596a11371773ffd324fa7e1dff09f0276 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Tue, 6 May 2025 23:36:39 +0200 Subject: [PATCH 1/3] Error handling: Fail early when database cluster does not respond --- CHANGES.rst | 1 + docs/by-example/client.rst | 8 ++------ src/crate/client/connection.py | 9 ++++++++- tests/client/test_connection.py | 8 ++++++++ 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index db094c5e..398c67b0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,7 @@ Unreleased ========== - Modernize project definition to latest Python best practices. Thanks, @surister. - Exceptions: Exceptions from the BLOB API now include their full names. +- 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 a06e1036..a693374d 100644 --- a/docs/by-example/client.rst +++ b/docs/by-example/client.rst @@ -29,12 +29,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 0638a018..1e439608 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 90b121f2..1eb496c0 100644 --- a/tests/client/test_connection.py +++ b/tests/client/test_connection.py @@ -4,6 +4,7 @@ import pytest from urllib3 import Timeout +import crate.client.exceptions from crate.client import connect from crate.client.connection import Connection from crate.client.exceptions import ProgrammingError @@ -12,6 +13,13 @@ from .settings import crate_host +def test_invalid_server_address(): + client = Client(servers="localhost:4202") + with pytest.raises(crate.client.exceptions.ConnectionError) as excinfo: + connect(client=client) + assert excinfo.match("Server not available") + + def test_lowest_server_version(): """ Verify the lowest server version is correctly set. From 783614387a481d501785e99d6d83a04e92092c14 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 9 May 2025 13:22:51 +0200 Subject: [PATCH 2/3] 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 1e439608..34a3c8f5 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 12cd6a729ea4110ff9b5fde60f09ac41b4cf4426 Mon Sep 17 00:00:00 2001 From: Andreas Motl Date: Fri, 9 May 2025 13:46:39 +0200 Subject: [PATCH 3/3] 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 34a3c8f5..d05f2649 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):