From 578329546e34064ca69673667b95120d621561f8 Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Tue, 29 Apr 2025 10:48:07 +0200 Subject: [PATCH 1/3] `ValueError` on invalid `connection_acquisition_timeout` - `ValueError` on invalid values (instead of `ClientError`) - Consistently restrict the value to be strictly positive --- CHANGELOG.md | 4 ++++ src/neo4j/_async/io/_pool.py | 29 ++++++++++++++++++++++++----- src/neo4j/_sync/io/_pool.py | 29 ++++++++++++++++++++++++----- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2418d09b..11615bec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,10 @@ See also https://github.com/neo4j/neo4j-python-driver/wiki for a full changelog. - Python 3.7, 3.8, and 3.9 support has been dropped. - Remove deprecated package alias `neo4j-driver`. Use `pip install neo4j` instead. - Remove `setup.py`. Please use a recent enough packaging/build tool that supports `pyproject.toml` +- Changed errors raised under certain circumstances + - `connection_acquisition_timeout` configuration option + - `ValueError` on invalid values (instead of `ClientError`) + - Consistently restrict the value to be strictly positive ## Version 5.28 diff --git a/src/neo4j/_async/io/_pool.py b/src/neo4j/_async/io/_pool.py index 747614d9..080ec3b1 100644 --- a/src/neo4j/_async/io/_pool.py +++ b/src/neo4j/_async/io/_pool.py @@ -19,6 +19,7 @@ import abc import asyncio import logging +import math import typing as t from collections import ( defaultdict, @@ -669,6 +670,7 @@ async def acquire( ): # The access_mode and database is not needed for a direct connection, # it's just there for consistency. + _check_acquisition_timeout(timeout) log.debug( "[#0000] _: acquire direct connection, " "access_mode=%r, database=%r", @@ -969,6 +971,7 @@ async def update_routing_table( :raise neo4j.exceptions.ServiceUnavailable: """ + _check_acquisition_timeout(acquisition_timeout) async with self.refresh_lock: routing_table = await self.get_routing_table(database) if routing_table is not None: @@ -1152,11 +1155,7 @@ async def acquire( if access_mode not in {WRITE_ACCESS, READ_ACCESS}: # TODO: 6.0 - change this to be a ValueError raise ClientError(f"Non valid 'access_mode'; {access_mode}") - if not timeout: - # TODO: 6.0 - change this to be a ValueError - raise ClientError( - f"'timeout' must be a float larger than 0; {timeout}" - ) + _check_acquisition_timeout(timeout) from ...api import check_access_mode @@ -1253,3 +1252,23 @@ async def on_write_failure(self, address, database): if table is not None: table.writers.discard(address) log.debug("[#0000] _: table=%r", self.routing_tables) + + +def _check_acquisition_timeout(timeout: object) -> None: + if isinstance(timeout, int): + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + elif isinstance(timeout, float): + if math.isnan(timeout): + raise ValueError("Connection acquisition timeout must not be NaN") + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + else: + raise TypeError( + "Connection acquisition timeout must be a number, " + f"got {type(timeout)}" + ) diff --git a/src/neo4j/_sync/io/_pool.py b/src/neo4j/_sync/io/_pool.py index 48a14891..a67bbd99 100644 --- a/src/neo4j/_sync/io/_pool.py +++ b/src/neo4j/_sync/io/_pool.py @@ -19,6 +19,7 @@ import abc import asyncio import logging +import math import typing as t from collections import ( defaultdict, @@ -666,6 +667,7 @@ def acquire( ): # The access_mode and database is not needed for a direct connection, # it's just there for consistency. + _check_acquisition_timeout(timeout) log.debug( "[#0000] _: acquire direct connection, " "access_mode=%r, database=%r", @@ -966,6 +968,7 @@ def update_routing_table( :raise neo4j.exceptions.ServiceUnavailable: """ + _check_acquisition_timeout(acquisition_timeout) with self.refresh_lock: routing_table = self.get_routing_table(database) if routing_table is not None: @@ -1149,11 +1152,7 @@ def acquire( if access_mode not in {WRITE_ACCESS, READ_ACCESS}: # TODO: 6.0 - change this to be a ValueError raise ClientError(f"Non valid 'access_mode'; {access_mode}") - if not timeout: - # TODO: 6.0 - change this to be a ValueError - raise ClientError( - f"'timeout' must be a float larger than 0; {timeout}" - ) + _check_acquisition_timeout(timeout) from ...api import check_access_mode @@ -1250,3 +1249,23 @@ def on_write_failure(self, address, database): if table is not None: table.writers.discard(address) log.debug("[#0000] _: table=%r", self.routing_tables) + + +def _check_acquisition_timeout(timeout: object) -> None: + if isinstance(timeout, int): + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + elif isinstance(timeout, float): + if math.isnan(timeout): + raise ValueError("Connection acquisition timeout must not be NaN") + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + else: + raise TypeError( + "Connection acquisition timeout must be a number, " + f"got {type(timeout)}" + ) From ebd6ce88c27b105905b193125e416f99b8dc201e Mon Sep 17 00:00:00 2001 From: Robsdedude Date: Fri, 9 May 2025 09:02:07 +0200 Subject: [PATCH 2/3] Fix forced RT update in TestKit backend --- testkitbackend/_async/requests.py | 8 +++++++- testkitbackend/_sync/requests.py | 8 +++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/testkitbackend/_async/requests.py b/testkitbackend/_async/requests.py index 2239b200..0a86e8ee 100644 --- a/testkitbackend/_async/requests.py +++ b/testkitbackend/_async/requests.py @@ -981,9 +981,15 @@ async def forced_routing_table_update(backend, data): driver = backend.drivers[driver_id] database = data["database"] bookmarks = data["bookmarks"] + acquisition_timeout = ( + driver._default_workspace_config.connection_acquisition_timeout + ) async with driver._pool.refresh_lock: await driver._pool.update_routing_table( - database=database, imp_user=None, bookmarks=bookmarks + database=database, + imp_user=None, + bookmarks=bookmarks, + acquisition_timeout=acquisition_timeout, ) await backend.send_response("Driver", {"id": driver_id}) diff --git a/testkitbackend/_sync/requests.py b/testkitbackend/_sync/requests.py index baa34a95..267f526c 100644 --- a/testkitbackend/_sync/requests.py +++ b/testkitbackend/_sync/requests.py @@ -981,9 +981,15 @@ def forced_routing_table_update(backend, data): driver = backend.drivers[driver_id] database = data["database"] bookmarks = data["bookmarks"] + acquisition_timeout = ( + driver._default_workspace_config.connection_acquisition_timeout + ) with driver._pool.refresh_lock: driver._pool.update_routing_table( - database=database, imp_user=None, bookmarks=bookmarks + database=database, + imp_user=None, + bookmarks=bookmarks, + acquisition_timeout=acquisition_timeout, ) backend.send_response("Driver", {"id": driver_id}) From 02f55aef5242da6ece8409a89a148df2b6d81c9b Mon Sep 17 00:00:00 2001 From: Andy Heap Date: Tue, 13 May 2025 09:39:06 +0200 Subject: [PATCH 3/3] Simplify code Signed-off-by: Rouven Bauer --- src/neo4j/_async/io/_pool.py | 20 +++++++------------- src/neo4j/_sync/io/_pool.py | 20 +++++++------------- 2 files changed, 14 insertions(+), 26 deletions(-) diff --git a/src/neo4j/_async/io/_pool.py b/src/neo4j/_async/io/_pool.py index 4be6288e..bdaa2263 100644 --- a/src/neo4j/_async/io/_pool.py +++ b/src/neo4j/_async/io/_pool.py @@ -1248,20 +1248,14 @@ async def on_write_failure(self, address, database): def _check_acquisition_timeout(timeout: object) -> None: - if isinstance(timeout, int): - if timeout <= 0: - raise ValueError( - f"Connection acquisition timeout must be > 0, got {timeout}" - ) - elif isinstance(timeout, float): - if math.isnan(timeout): - raise ValueError("Connection acquisition timeout must not be NaN") - if timeout <= 0: - raise ValueError( - f"Connection acquisition timeout must be > 0, got {timeout}" - ) - else: + if not isinstance(timeout, (int, float)): raise TypeError( "Connection acquisition timeout must be a number, " f"got {type(timeout)}" ) + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + if math.isnan(timeout): + raise ValueError("Connection acquisition timeout must not be NaN") diff --git a/src/neo4j/_sync/io/_pool.py b/src/neo4j/_sync/io/_pool.py index ce46c487..4bf6f4a8 100644 --- a/src/neo4j/_sync/io/_pool.py +++ b/src/neo4j/_sync/io/_pool.py @@ -1245,20 +1245,14 @@ def on_write_failure(self, address, database): def _check_acquisition_timeout(timeout: object) -> None: - if isinstance(timeout, int): - if timeout <= 0: - raise ValueError( - f"Connection acquisition timeout must be > 0, got {timeout}" - ) - elif isinstance(timeout, float): - if math.isnan(timeout): - raise ValueError("Connection acquisition timeout must not be NaN") - if timeout <= 0: - raise ValueError( - f"Connection acquisition timeout must be > 0, got {timeout}" - ) - else: + if not isinstance(timeout, (int, float)): raise TypeError( "Connection acquisition timeout must be a number, " f"got {type(timeout)}" ) + if timeout <= 0: + raise ValueError( + f"Connection acquisition timeout must be > 0, got {timeout}" + ) + if math.isnan(timeout): + raise ValueError("Connection acquisition timeout must not be NaN")