From dd3ed51d605178beab9f74602aa2bb0e91a98ff7 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 11:03:49 +0200 Subject: [PATCH 1/9] Warnings overhaul * Fix potential to falsely emitted `DeprecationWarning` for deprecated `PoolConfig` options. * Treat warnings as errors during testing * Change TestKit backend to expect and catch warnings --- neo4j/_async/io/_bolt.py | 16 +++++++++------- neo4j/_async/io/_pool.py | 4 ++-- neo4j/_sync/io/_bolt.py | 16 +++++++++------- neo4j/_sync/io/_pool.py | 4 ++-- testkit/backend.py | 2 +- testkit/integration.py | 2 +- testkit/unittests.py | 2 +- testkitbackend/__main__.py | 2 ++ testkitbackend/_async/requests.py | 11 ++++++++++- testkitbackend/_sync/requests.py | 11 ++++++++++- testkitbackend/totestkit.py | 30 ++++++++++++++++++++++++++---- tox.ini | 4 ++-- 12 files changed, 75 insertions(+), 29 deletions(-) diff --git a/neo4j/_async/io/_bolt.py b/neo4j/_async/io/_bolt.py index 8344b2696..51b50f65d 100644 --- a/neo4j/_async/io/_bolt.py +++ b/neo4j/_async/io/_bolt.py @@ -259,19 +259,20 @@ def get_handshake(cls): return b"".join(version.to_bytes() for version in offered_versions).ljust(16, b"\x00") @classmethod - async def ping(cls, address, *, timeout=None, **config): + async def ping(cls, address, *, timeout=None, pool_config=None): """ Attempt to establish a Bolt connection, returning the agreed Bolt protocol version if successful. """ - config = PoolConfig.consume(config) + if pool_config is None: + pool_config = PoolConfig try: s, protocol_version, handshake, data = \ await AsyncBoltSocket.connect( address, timeout=timeout, - custom_resolver=config.resolver, - ssl_context=config.get_ssl_context(), - keep_alive=config.keep_alive, + custom_resolver=pool_config.resolver, + ssl_context=pool_config.get_ssl_context(), + keep_alive=pool_config.keep_alive, ) except (ServiceUnavailable, SessionExpired, BoltHandshakeError): return None @@ -282,7 +283,7 @@ async def ping(cls, address, *, timeout=None, **config): @classmethod async def open( cls, address, *, auth=None, timeout=None, routing_context=None, - **pool_config + pool_config=None ): """Open a new Bolt connection to a given server address. @@ -305,7 +306,8 @@ def time_remaining(): return t if t > 0 else 0 t0 = perf_counter() - pool_config = PoolConfig.consume(pool_config) + if pool_config is None: + pool_config = PoolConfig() socket_connection_timeout = pool_config.connection_timeout if socket_connection_timeout is None: diff --git a/neo4j/_async/io/_pool.py b/neo4j/_async/io/_pool.py index b530b3a2a..d0a9acd4f 100644 --- a/neo4j/_async/io/_pool.py +++ b/neo4j/_async/io/_pool.py @@ -379,7 +379,7 @@ def open(cls, address, *, auth, pool_config, workspace_config): async def opener(addr, timeout): return await AsyncBolt.open( addr, auth=auth, timeout=timeout, routing_context=None, - **pool_config + pool_config=pool_config ) pool = cls(opener, pool_config, workspace_config, address) @@ -431,7 +431,7 @@ def open(cls, *addresses, auth, pool_config, workspace_config, async def opener(addr, timeout): return await AsyncBolt.open( addr, auth=auth, timeout=timeout, - routing_context=routing_context, **pool_config + routing_context=routing_context, pool_config=pool_config ) pool = cls(opener, pool_config, workspace_config, address) diff --git a/neo4j/_sync/io/_bolt.py b/neo4j/_sync/io/_bolt.py index 7d5ceeb98..0d8f07a84 100644 --- a/neo4j/_sync/io/_bolt.py +++ b/neo4j/_sync/io/_bolt.py @@ -259,19 +259,20 @@ def get_handshake(cls): return b"".join(version.to_bytes() for version in offered_versions).ljust(16, b"\x00") @classmethod - def ping(cls, address, *, timeout=None, **config): + def ping(cls, address, *, timeout=None, pool_config=None): """ Attempt to establish a Bolt connection, returning the agreed Bolt protocol version if successful. """ - config = PoolConfig.consume(config) + if pool_config is None: + pool_config = PoolConfig try: s, protocol_version, handshake, data = \ BoltSocket.connect( address, timeout=timeout, - custom_resolver=config.resolver, - ssl_context=config.get_ssl_context(), - keep_alive=config.keep_alive, + custom_resolver=pool_config.resolver, + ssl_context=pool_config.get_ssl_context(), + keep_alive=pool_config.keep_alive, ) except (ServiceUnavailable, SessionExpired, BoltHandshakeError): return None @@ -282,7 +283,7 @@ def ping(cls, address, *, timeout=None, **config): @classmethod def open( cls, address, *, auth=None, timeout=None, routing_context=None, - **pool_config + pool_config=None ): """Open a new Bolt connection to a given server address. @@ -305,7 +306,8 @@ def time_remaining(): return t if t > 0 else 0 t0 = perf_counter() - pool_config = PoolConfig.consume(pool_config) + if pool_config is None: + pool_config = PoolConfig() socket_connection_timeout = pool_config.connection_timeout if socket_connection_timeout is None: diff --git a/neo4j/_sync/io/_pool.py b/neo4j/_sync/io/_pool.py index 947555a01..d031ad8c8 100644 --- a/neo4j/_sync/io/_pool.py +++ b/neo4j/_sync/io/_pool.py @@ -379,7 +379,7 @@ def open(cls, address, *, auth, pool_config, workspace_config): def opener(addr, timeout): return Bolt.open( addr, auth=auth, timeout=timeout, routing_context=None, - **pool_config + pool_config=pool_config ) pool = cls(opener, pool_config, workspace_config, address) @@ -431,7 +431,7 @@ def open(cls, *addresses, auth, pool_config, workspace_config, def opener(addr, timeout): return Bolt.open( addr, auth=auth, timeout=timeout, - routing_context=routing_context, **pool_config + routing_context=routing_context, pool_config=pool_config ) pool = cls(opener, pool_config, workspace_config, address) diff --git a/testkit/backend.py b/testkit/backend.py index 745ca8e02..b0c3c09f8 100644 --- a/testkit/backend.py +++ b/testkit/backend.py @@ -24,7 +24,7 @@ if __name__ == "__main__": - cmd = ["python", "-m", "testkitbackend"] + cmd = ["python", "-W", "error", "-m", "testkitbackend"] if "TEST_BACKEND_SERVER" in os.environ: cmd.append(os.environ["TEST_BACKEND_SERVER"]) subprocess.check_call(cmd, stdout=sys.stdout, stderr=sys.stderr) diff --git a/testkit/integration.py b/testkit/integration.py index 1c8f435d7..81b864f0c 100644 --- a/testkit/integration.py +++ b/testkit/integration.py @@ -27,4 +27,4 @@ def run(args): if __name__ == "__main__": - run(["python", "-m", "tox", "-f", "integration"]) + run(["python", "-W", "error", "-m", "tox", "-f", "integration"]) diff --git a/testkit/unittests.py b/testkit/unittests.py index 262f1bbc0..3cea1355c 100644 --- a/testkit/unittests.py +++ b/testkit/unittests.py @@ -27,4 +27,4 @@ def run(args): if __name__ == "__main__": - run(["python", "-m", "tox", "-f", "unit"]) + run(["python", "-W", "error", "-m", "tox", "-f", "unit"]) diff --git a/testkitbackend/__main__.py b/testkitbackend/__main__.py index a6c7a8ecd..91a4a6e1c 100644 --- a/testkitbackend/__main__.py +++ b/testkitbackend/__main__.py @@ -20,6 +20,7 @@ import asyncio import sys +import warnings from .server import ( AsyncServer, @@ -46,6 +47,7 @@ async def main(): if __name__ == "__main__": + warnings.simplefilter("error") if len(sys.argv) == 2 and sys.argv[1].lower().strip() == "async": async_main() else: diff --git a/testkitbackend/_async/requests.py b/testkitbackend/_async/requests.py index 6f939e9cb..eea2eb88f 100644 --- a/testkitbackend/_async/requests.py +++ b/testkitbackend/_async/requests.py @@ -21,6 +21,8 @@ import warnings from os import path +import pytest + import neo4j import neo4j.api from neo4j._async_compat.util import AsyncUtil @@ -176,8 +178,14 @@ async def GetServerInfo(backend, data): async def CheckMultiDBSupport(backend, data): driver_id = data["driverId"] driver = backend.drivers[driver_id] + with pytest.warns( + DeprecationWarning, + match="Feature support query, based on Bolt protocol version and " + "Neo4j server version will change in the future." + ): + available = await driver.supports_multi_db() await backend.send_response("MultiDBSupport", { - "id": backend.next_key(), "available": await driver.supports_multi_db() + "id": backend.next_key(), "available": available }) @@ -534,6 +542,7 @@ async def ResultSingle(backend, data): async def ResultSingleOptional(backend, data): result = backend.results[data["resultId"]] with warnings.catch_warnings(record=True) as warning_list: + warnings.simplefilter("always") record = await result.single(strict=False) if record: record = totestkit.record(record) diff --git a/testkitbackend/_sync/requests.py b/testkitbackend/_sync/requests.py index c46b65db9..7ba840057 100644 --- a/testkitbackend/_sync/requests.py +++ b/testkitbackend/_sync/requests.py @@ -21,6 +21,8 @@ import warnings from os import path +import pytest + import neo4j import neo4j.api from neo4j._async_compat.util import Util @@ -176,8 +178,14 @@ def GetServerInfo(backend, data): def CheckMultiDBSupport(backend, data): driver_id = data["driverId"] driver = backend.drivers[driver_id] + with pytest.warns( + DeprecationWarning, + match="Feature support query, based on Bolt protocol version and " + "Neo4j server version will change in the future." + ): + available = driver.supports_multi_db() backend.send_response("MultiDBSupport", { - "id": backend.next_key(), "available": driver.supports_multi_db() + "id": backend.next_key(), "available": available }) @@ -534,6 +542,7 @@ def ResultSingle(backend, data): def ResultSingleOptional(backend, data): result = backend.results[data["resultId"]] with warnings.catch_warnings(record=True) as warning_list: + warnings.simplefilter("always") record = result.single(strict=False) if record: record = totestkit.record(record) diff --git a/testkitbackend/totestkit.py b/testkitbackend/totestkit.py index e81ff8fb9..96e6414b3 100644 --- a/testkitbackend/totestkit.py +++ b/testkitbackend/totestkit.py @@ -18,6 +18,8 @@ import math +import pytest + from neo4j.graph import ( Node, Path, @@ -73,18 +75,38 @@ def to(name, val): if isinstance(v, (bytes, bytearray)): return to("CypherBytes", " ".join("{:02x}".format(byte) for byte in v)) if isinstance(v, Node): + with pytest.warns( + DeprecationWarning, + match="`id` is deprecated, use `element_id` instead" + ): + id_ = v.id node = { - "id": field(v.id), + "id": field(id_), "labels": field(v.labels), "props": field(v._properties), "elementId": field(v.element_id), } return {"name": "Node", "data": node} if isinstance(v, Relationship): + with pytest.warns( + DeprecationWarning, + match="`id` is deprecated, use `element_id` instead" + ): + id_ = v.id + with pytest.warns( + DeprecationWarning, + match="`id` is deprecated, use `element_id` instead" + ): + start_id = v.start_node.id + with pytest.warns( + DeprecationWarning, + match="`id` is deprecated, use `element_id` instead" + ): + end_id = v.end_node.id rel = { - "id": field(v.id), - "startNodeId": field(v.start_node.id), - "endNodeId": field(v.end_node.id), + "id": field(id_), + "startNodeId": field(start_id), + "endNodeId": field(end_id), "type": field(v.type), "props": field(v._properties), "elementId": field(v.element_id), diff --git a/tox.ini b/tox.ini index 203bfb01b..2aea6fe50 100644 --- a/tox.ini +++ b/tox.ini @@ -8,7 +8,7 @@ deps = -r tests/requirements.txt commands = coverage erase - unit: coverage run -m pytest -v {posargs} tests/unit - integration: coverage run -m pytest -v {posargs} tests/integration + unit: coverage run -W error -m pytest -v {posargs} tests/unit + integration: coverage run -W error -m pytest -v {posargs} tests/integration performance: python -m pytest --benchmark-autosave -v {posargs} tests/performance unit,integration: coverage report From 45283f709dd5f13eff686c265e0f8685b87310f8 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 12:01:15 +0200 Subject: [PATCH 2/9] TestKit backend: fix missing Python build dependency --- testkit/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testkit/Dockerfile b/testkit/Dockerfile index 74068ce46..e3322d281 100644 --- a/testkit/Dockerfile +++ b/testkit/Dockerfile @@ -25,7 +25,7 @@ RUN apt-get update && \ apt-get install -y --no-install-recommends \ make build-essential libssl-dev zlib1g-dev libbz2-dev libreadline-dev \ libsqlite3-dev wget curl llvm libncurses5-dev xz-utils tk-dev \ - libxml2-dev libxmlsec1-dev libffi-dev \ + libxml2-dev libxmlsec1-dev libffi-dev liblzma-dev \ ca-certificates && \ apt-get clean && rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/* From c33037000b6d56626a8bd5da02520f82684bdd66 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 12:56:41 +0200 Subject: [PATCH 3/9] TestKit backend: fix warning assertions --- neo4j/_async/driver.py | 3 +- neo4j/_meta.py | 3 +- neo4j/_sync/driver.py | 3 +- testkitbackend/_async/requests.py | 30 +++++++++++---- testkitbackend/_sync/requests.py | 30 +++++++++++---- testkitbackend/_warning_check.py | 64 +++++++++++++++++++++++++++++++ testkitbackend/totestkit.py | 24 +++++------- 7 files changed, 121 insertions(+), 36 deletions(-) create mode 100644 testkitbackend/_warning_check.py diff --git a/neo4j/_async/driver.py b/neo4j/_async/driver.py index 36d56732d..5b18e4ae8 100644 --- a/neo4j/_async/driver.py +++ b/neo4j/_async/driver.py @@ -675,6 +675,5 @@ def __init__(self, pool, default_workspace_config): def session(self, **config) -> AsyncSession: session_config = SessionConfig(self._default_workspace_config, - config) - SessionConfig.consume(config) # Consume the config + SessionConfig.consume(config)) return AsyncSession(self._pool, session_config) diff --git a/neo4j/_meta.py b/neo4j/_meta.py index 3e41e3467..970ead8a3 100644 --- a/neo4j/_meta.py +++ b/neo4j/_meta.py @@ -17,6 +17,7 @@ import asyncio +import tracemalloc import typing as t from functools import wraps from warnings import warn @@ -130,8 +131,6 @@ def inner(*args, **kwargs): def unclosed_resource_warn(obj): - import tracemalloc - from warnings import warn msg = f"Unclosed {obj!r}." trace = tracemalloc.get_object_traceback(obj) if trace: diff --git a/neo4j/_sync/driver.py b/neo4j/_sync/driver.py index 058928634..5b2ee82ae 100644 --- a/neo4j/_sync/driver.py +++ b/neo4j/_sync/driver.py @@ -674,6 +674,5 @@ def __init__(self, pool, default_workspace_config): def session(self, **config) -> Session: session_config = SessionConfig(self._default_workspace_config, - config) - SessionConfig.consume(config) # Consume the config + SessionConfig.consume(config)) return Session(self._pool, session_config) diff --git a/testkitbackend/_async/requests.py b/testkitbackend/_async/requests.py index eea2eb88f..1a8daeb80 100644 --- a/testkitbackend/_async/requests.py +++ b/testkitbackend/_async/requests.py @@ -21,8 +21,6 @@ import warnings from os import path -import pytest - import neo4j import neo4j.api from neo4j._async_compat.util import AsyncUtil @@ -32,6 +30,7 @@ test_subtest_skips, totestkit, ) +from .._warning_check import warning_check from ..exceptions import MarkdAsDriverException @@ -178,10 +177,10 @@ async def GetServerInfo(backend, data): async def CheckMultiDBSupport(backend, data): driver_id = data["driverId"] driver = backend.drivers[driver_id] - with pytest.warns( - DeprecationWarning, - match="Feature support query, based on Bolt protocol version and " - "Neo4j server version will change in the future." + with warning_check( + neo4j.ExperimentalWarning, + "Feature support query, based on Bolt protocol version and Neo4j " + "server version will change in the future." ): available = await driver.supports_multi_db() await backend.send_response("MultiDBSupport", { @@ -266,7 +265,14 @@ async def NewBookmarkManager(backend, data): backend, bookmark_manager_id ) - bookmark_manager = neo4j.AsyncGraphDatabase.bookmark_manager(**bmm_kwargs) + with warning_check( + neo4j.ExperimentalWarning, + "The bookmark manager feature is experimental. It might be changed or " + "removed any time even without prior notice." + ): + bookmark_manager = neo4j.AsyncGraphDatabase.bookmark_manager( + **bmm_kwargs + ) backend.bookmark_managers[bookmark_manager_id] = bookmark_manager await backend.send_response("BookmarkManager", {"id": bookmark_manager_id}) @@ -382,7 +388,15 @@ async def NewSession(backend, data): ): if data_name in data: config[conf_name] = data[data_name] - session = driver.session(**config) + if "bookmark_manager" in config: + with warning_check( + neo4j.ExperimentalWarning, + "The 'bookmark_manager' config key is experimental. It might be " + "changed or removed any time even without prior notice." + ): + session = driver.session(**config) + else: + session = driver.session(**config) key = backend.next_key() backend.sessions[key] = SessionTracker(session) await backend.send_response("Session", {"id": key}) diff --git a/testkitbackend/_sync/requests.py b/testkitbackend/_sync/requests.py index 7ba840057..47be9aff8 100644 --- a/testkitbackend/_sync/requests.py +++ b/testkitbackend/_sync/requests.py @@ -21,8 +21,6 @@ import warnings from os import path -import pytest - import neo4j import neo4j.api from neo4j._async_compat.util import Util @@ -32,6 +30,7 @@ test_subtest_skips, totestkit, ) +from .._warning_check import warning_check from ..exceptions import MarkdAsDriverException @@ -178,10 +177,10 @@ def GetServerInfo(backend, data): def CheckMultiDBSupport(backend, data): driver_id = data["driverId"] driver = backend.drivers[driver_id] - with pytest.warns( - DeprecationWarning, - match="Feature support query, based on Bolt protocol version and " - "Neo4j server version will change in the future." + with warning_check( + neo4j.ExperimentalWarning, + "Feature support query, based on Bolt protocol version and Neo4j " + "server version will change in the future." ): available = driver.supports_multi_db() backend.send_response("MultiDBSupport", { @@ -266,7 +265,14 @@ def NewBookmarkManager(backend, data): backend, bookmark_manager_id ) - bookmark_manager = neo4j.GraphDatabase.bookmark_manager(**bmm_kwargs) + with warning_check( + neo4j.ExperimentalWarning, + "The bookmark manager feature is experimental. It might be changed or " + "removed any time even without prior notice." + ): + bookmark_manager = neo4j.GraphDatabase.bookmark_manager( + **bmm_kwargs + ) backend.bookmark_managers[bookmark_manager_id] = bookmark_manager backend.send_response("BookmarkManager", {"id": bookmark_manager_id}) @@ -382,7 +388,15 @@ def NewSession(backend, data): ): if data_name in data: config[conf_name] = data[data_name] - session = driver.session(**config) + if "bookmark_manager" in config: + with warning_check( + neo4j.ExperimentalWarning, + "The 'bookmark_manager' config key is experimental. It might be " + "changed or removed any time even without prior notice." + ): + session = driver.session(**config) + else: + session = driver.session(**config) key = backend.next_key() backend.sessions[key] = SessionTracker(session) backend.send_response("Session", {"id": key}) diff --git a/testkitbackend/_warning_check.py b/testkitbackend/_warning_check.py new file mode 100644 index 000000000..7fb25aa23 --- /dev/null +++ b/testkitbackend/_warning_check.py @@ -0,0 +1,64 @@ +# Copyright (c) "Neo4j" +# Neo4j Sweden AB [https://neo4j.com] +# +# This file is part of Neo4j. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +import re +import warnings +from contextlib import contextmanager + + +@contextmanager +def warning_check(category, message): + with warnings.catch_warnings(record=True) as warn_log: + warnings.filterwarnings("always", category=category, message=message) + yield + if len(warn_log) != 1: + raise AssertionError("Expected 1 warning, found %d: %s" + % (len(warn_log), warn_log)) + + +@contextmanager +def warnings_check(category_message_pairs): + with warnings.catch_warnings(record=True) as warn_log: + for category, message in category_message_pairs: + warnings.filterwarnings("always", category=category, + message=message) + yield + if len(warn_log) != len(category_message_pairs): + raise AssertionError( + "Expected %d warnings, found %d: %s" + % (len(category_message_pairs), len(warn_log), warn_log) + ) + category_message_pairs = [ + (category, re.compile(message, re.I)) + for category, message in category_message_pairs + ] + for category, matcher in category_message_pairs: + match = None + for i, warning in enumerate(warn_log): + if ( + warning.category == category + and matcher.match(warning.message.args[0]) + ): + match = i + break + if match is None: + raise AssertionError( + "Expected warning not found: %r %r" + % (category, matcher.pattern) + ) + warn_log.pop(match) diff --git a/testkitbackend/totestkit.py b/testkitbackend/totestkit.py index 96e6414b3..180f5b4e8 100644 --- a/testkitbackend/totestkit.py +++ b/testkitbackend/totestkit.py @@ -18,8 +18,6 @@ import math -import pytest - from neo4j.graph import ( Node, Path, @@ -36,6 +34,8 @@ Time, ) +from ._warning_check import warning_check + def record(rec): fields = [] @@ -75,9 +75,8 @@ def to(name, val): if isinstance(v, (bytes, bytearray)): return to("CypherBytes", " ".join("{:02x}".format(byte) for byte in v)) if isinstance(v, Node): - with pytest.warns( - DeprecationWarning, - match="`id` is deprecated, use `element_id` instead" + with warning_check( + DeprecationWarning, "`id` is deprecated, use `element_id` instead" ): id_ = v.id node = { @@ -88,19 +87,16 @@ def to(name, val): } return {"name": "Node", "data": node} if isinstance(v, Relationship): - with pytest.warns( - DeprecationWarning, - match="`id` is deprecated, use `element_id` instead" + with warning_check( + DeprecationWarning, "`id` is deprecated, use `element_id` instead" ): id_ = v.id - with pytest.warns( - DeprecationWarning, - match="`id` is deprecated, use `element_id` instead" + with warning_check( + DeprecationWarning, "`id` is deprecated, use `element_id` instead" ): start_id = v.start_node.id - with pytest.warns( - DeprecationWarning, - match="`id` is deprecated, use `element_id` instead" + with warning_check( + DeprecationWarning, "`id` is deprecated, use `element_id` instead" ): end_id = v.end_node.id rel = { From 0072d47b5894d87675e8027c628e53fdcb684703 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 14:16:33 +0200 Subject: [PATCH 4/9] Test fixes --- neo4j/_async/io/_bolt.py | 2 +- neo4j/_sync/io/_bolt.py | 2 +- tox.ini | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/neo4j/_async/io/_bolt.py b/neo4j/_async/io/_bolt.py index 51b50f65d..fb67d9131 100644 --- a/neo4j/_async/io/_bolt.py +++ b/neo4j/_async/io/_bolt.py @@ -264,7 +264,7 @@ async def ping(cls, address, *, timeout=None, pool_config=None): agreed Bolt protocol version if successful. """ if pool_config is None: - pool_config = PoolConfig + pool_config = PoolConfig() try: s, protocol_version, handshake, data = \ await AsyncBoltSocket.connect( diff --git a/neo4j/_sync/io/_bolt.py b/neo4j/_sync/io/_bolt.py index 0d8f07a84..eaa30fd1b 100644 --- a/neo4j/_sync/io/_bolt.py +++ b/neo4j/_sync/io/_bolt.py @@ -264,7 +264,7 @@ def ping(cls, address, *, timeout=None, pool_config=None): agreed Bolt protocol version if successful. """ if pool_config is None: - pool_config = PoolConfig + pool_config = PoolConfig() try: s, protocol_version, handshake, data = \ BoltSocket.connect( diff --git a/tox.ini b/tox.ini index 2aea6fe50..b414ff11f 100644 --- a/tox.ini +++ b/tox.ini @@ -8,7 +8,7 @@ deps = -r tests/requirements.txt commands = coverage erase - unit: coverage run -W error -m pytest -v {posargs} tests/unit - integration: coverage run -W error -m pytest -v {posargs} tests/integration + unit: coverage run -m pytest -W error -v {posargs} tests/unit + integration: coverage run -m pytest -W error -v {posargs} tests/integration performance: python -m pytest --benchmark-autosave -v {posargs} tests/performance unit,integration: coverage report From b3ebc942a8e0542c61d7ab5673bd3b064c33cec2 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 15:19:56 +0200 Subject: [PATCH 5/9] Test: close driver properly --- tests/integration/test_readme.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/integration/test_readme.py b/tests/integration/test_readme.py index 872461307..15fa60cc4 100644 --- a/tests/integration/test_readme.py +++ b/tests/integration/test_readme.py @@ -32,6 +32,7 @@ def test_should_run_readme(uri, auth): driver = GraphDatabase.driver("neo4j://localhost:7687", auth=("neo4j", "password")) # === END: README === + driver.close() driver = GraphDatabase.driver(uri, auth=auth) # === START: README === From a4ffca9a9ec401cb03be4a18000929f0a816a2fe Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 16:07:55 +0200 Subject: [PATCH 6/9] TestKit backend: close resources properly --- testkitbackend/_async/backend.py | 8 ++++++++ testkitbackend/_sync/backend.py | 8 ++++++++ testkitbackend/server.py | 3 +++ 3 files changed, 19 insertions(+) diff --git a/testkitbackend/_async/backend.py b/testkitbackend/_async/backend.py index 851865c6b..11026c79c 100644 --- a/testkitbackend/_async/backend.py +++ b/testkitbackend/_async/backend.py @@ -68,6 +68,14 @@ def __init__(self, rd, wr): self._requestHandlers = dict( [m for m in getmembers(requests, isfunction)]) + async def close(self): + for _, transaction in self.transactions: + await transaction.close() + for _, session in self.sessions: + await session.close() + for _, driver in self.drivers: + await driver.close() + def next_key(self): self.key = self.key + 1 return self.key diff --git a/testkitbackend/_sync/backend.py b/testkitbackend/_sync/backend.py index 5dae1753d..bf378931b 100644 --- a/testkitbackend/_sync/backend.py +++ b/testkitbackend/_sync/backend.py @@ -68,6 +68,14 @@ def __init__(self, rd, wr): self._requestHandlers = dict( [m for m in getmembers(requests, isfunction)]) + def close(self): + for _, transaction in self.transactions: + transaction.close() + for _, session in self.sessions: + session.close() + for _, driver in self.drivers: + driver.close() + def next_key(self): self.key = self.key + 1 return self.key diff --git a/testkitbackend/server.py b/testkitbackend/server.py index f7ec85b9d..f4acc2987 100644 --- a/testkitbackend/server.py +++ b/testkitbackend/server.py @@ -35,6 +35,7 @@ def handle(self): backend = Backend(self.rfile, self.wfile) while backend.process_request(): pass + backend.close() print("Disconnected") super(Server, self).__init__(address, Handler) @@ -49,6 +50,8 @@ async def _handler(reader, writer): backend = AsyncBackend(reader, writer) while await backend.process_request(): pass + writer.close() + await backend.close() print("Disconnected") async def start(self): From ed194d6dca2bc2807a31bdc8bcb00d28e736d5f7 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 17:06:51 +0200 Subject: [PATCH 7/9] TestKit backend: close resources correctly --- testkitbackend/_async/backend.py | 6 +++--- testkitbackend/_sync/backend.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/testkitbackend/_async/backend.py b/testkitbackend/_async/backend.py index 11026c79c..545b110ce 100644 --- a/testkitbackend/_async/backend.py +++ b/testkitbackend/_async/backend.py @@ -69,11 +69,11 @@ def __init__(self, rd, wr): [m for m in getmembers(requests, isfunction)]) async def close(self): - for _, transaction in self.transactions: + for transaction in self.transactions.values(): await transaction.close() - for _, session in self.sessions: + for session in self.sessions.values(): await session.close() - for _, driver in self.drivers: + for driver in self.drivers.values(): await driver.close() def next_key(self): diff --git a/testkitbackend/_sync/backend.py b/testkitbackend/_sync/backend.py index bf378931b..3fed3eaae 100644 --- a/testkitbackend/_sync/backend.py +++ b/testkitbackend/_sync/backend.py @@ -69,11 +69,11 @@ def __init__(self, rd, wr): [m for m in getmembers(requests, isfunction)]) def close(self): - for _, transaction in self.transactions: + for transaction in self.transactions.values(): transaction.close() - for _, session in self.sessions: + for session in self.sessions.values(): session.close() - for _, driver in self.drivers: + for driver in self.drivers.values(): driver.close() def next_key(self): From d2a74b0b3ceb255190a9ca028470d0e2a3d66a09 Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Thu, 1 Sep 2022 17:27:55 +0200 Subject: [PATCH 8/9] Fix session config consumption --- neo4j/_async/driver.py | 3 +-- neo4j/_conf.py | 12 +++++++++++- neo4j/_sync/driver.py | 3 +-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/neo4j/_async/driver.py b/neo4j/_async/driver.py index 5b18e4ae8..a29b17980 100644 --- a/neo4j/_async/driver.py +++ b/neo4j/_async/driver.py @@ -644,7 +644,6 @@ def session(self, **config) -> AsyncSession: """ session_config = SessionConfig(self._default_workspace_config, config) - SessionConfig.consume(config) # Consume the config return AsyncSession(self._pool, session_config) @@ -675,5 +674,5 @@ def __init__(self, pool, default_workspace_config): def session(self, **config) -> AsyncSession: session_config = SessionConfig(self._default_workspace_config, - SessionConfig.consume(config)) + config) return AsyncSession(self._pool, session_config) diff --git a/neo4j/_conf.py b/neo4j/_conf.py index 434670853..1fa4ec4ff 100644 --- a/neo4j/_conf.py +++ b/neo4j/_conf.py @@ -300,9 +300,19 @@ def set_attr(k, v): else: raise AttributeError(k) + rejected_keys = [] for key, value in data_dict.items(): if value is not None: - set_attr(key, value) + try: + set_attr(key, value) + except AttributeError as exc: + if not exc.args == (key,): + raise + rejected_keys.append(key) + + if rejected_keys: + raise ConfigurationError("Unexpected config keys: " + + ", ".join(rejected_keys)) def __init__(self, *args, **kwargs): for arg in args: diff --git a/neo4j/_sync/driver.py b/neo4j/_sync/driver.py index 5b2ee82ae..604900ef3 100644 --- a/neo4j/_sync/driver.py +++ b/neo4j/_sync/driver.py @@ -643,7 +643,6 @@ def session(self, **config) -> Session: """ session_config = SessionConfig(self._default_workspace_config, config) - SessionConfig.consume(config) # Consume the config return Session(self._pool, session_config) @@ -674,5 +673,5 @@ def __init__(self, pool, default_workspace_config): def session(self, **config) -> Session: session_config = SessionConfig(self._default_workspace_config, - SessionConfig.consume(config)) + config) return Session(self._pool, session_config) From 480c2f985069b78ceb939cc40dbb6192140d134b Mon Sep 17 00:00:00 2001 From: Rouven Bauer Date: Fri, 2 Sep 2022 08:34:10 +0200 Subject: [PATCH 9/9] Improve TestKit backend clean-up --- testkitbackend/_async/backend.py | 21 +++++++++++++++------ testkitbackend/_sync/backend.py | 21 +++++++++++++++------ testkitbackend/server.py | 18 +++++++++++------- 3 files changed, 41 insertions(+), 19 deletions(-) diff --git a/testkitbackend/_async/backend.py b/testkitbackend/_async/backend.py index 545b110ce..7f74e024c 100644 --- a/testkitbackend/_async/backend.py +++ b/testkitbackend/_async/backend.py @@ -69,12 +69,21 @@ def __init__(self, rd, wr): [m for m in getmembers(requests, isfunction)]) async def close(self): - for transaction in self.transactions.values(): - await transaction.close() - for session in self.sessions.values(): - await session.close() - for driver in self.drivers.values(): - await driver.close() + for dict_of_closables in ( + self.transactions, + {key: tracker.session for key, tracker in self.sessions.items()}, + self.drivers, + ): + for key, closable in dict_of_closables.items(): + try: + await closable.close() + except (Neo4jError, DriverError, OSError): + log.error( + "Error during TestKit backend garbage collection. " + "While collecting: (key: %s) %s\n%s", + key, closable, traceback.format_exc() + ) + dict_of_closables.clear() def next_key(self): self.key = self.key + 1 diff --git a/testkitbackend/_sync/backend.py b/testkitbackend/_sync/backend.py index 3fed3eaae..346c3dd0e 100644 --- a/testkitbackend/_sync/backend.py +++ b/testkitbackend/_sync/backend.py @@ -69,12 +69,21 @@ def __init__(self, rd, wr): [m for m in getmembers(requests, isfunction)]) def close(self): - for transaction in self.transactions.values(): - transaction.close() - for session in self.sessions.values(): - session.close() - for driver in self.drivers.values(): - driver.close() + for dict_of_closables in ( + self.transactions, + {key: tracker.session for key, tracker in self.sessions.items()}, + self.drivers, + ): + for key, closable in dict_of_closables.items(): + try: + closable.close() + except (Neo4jError, DriverError, OSError): + log.error( + "Error during TestKit backend garbage collection. " + "While collecting: (key: %s) %s\n%s", + key, closable, traceback.format_exc() + ) + dict_of_closables.clear() def next_key(self): self.key = self.key + 1 diff --git a/testkitbackend/server.py b/testkitbackend/server.py index f4acc2987..bae92cb54 100644 --- a/testkitbackend/server.py +++ b/testkitbackend/server.py @@ -33,9 +33,11 @@ def __init__(self, address): class Handler(StreamRequestHandler): def handle(self): backend = Backend(self.rfile, self.wfile) - while backend.process_request(): - pass - backend.close() + try: + while backend.process_request(): + pass + finally: + backend.close() print("Disconnected") super(Server, self).__init__(address, Handler) @@ -48,10 +50,12 @@ def __init__(self, address): @staticmethod async def _handler(reader, writer): backend = AsyncBackend(reader, writer) - while await backend.process_request(): - pass - writer.close() - await backend.close() + try: + while await backend.process_request(): + pass + finally: + writer.close() + await backend.close() print("Disconnected") async def start(self):