Warnings overhaul (#790)

robsdedude · web-flow · commit 60662298662f · 2022-09-05T10:06:38.000+02:00
* 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
* TestKit backend: fix missing Python build dependency
* TestKit backend: fix warning assertions
* Test: close driver properly
* TestKit backend: close resources properly
diff --git 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)
 
 
@@ -676,5 +675,4 @@ 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
             return AsyncSession(self._pool, session_config)
diff --git 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
@@ -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/_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/_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
@@ -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)
 
 
@@ -675,5 +674,4 @@ 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
             return Session(self._pool, session_config)
diff --git 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
@@ -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/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/*
 
diff --git 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
@@ -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
@@ -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
@@ -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/backend.py b/testkitbackend/_async/backend.py
@@ -68,6 +68,23 @@ def __init__(self, rd, wr):
         self._requestHandlers = dict(
             [m for m in getmembers(requests, isfunction)])
 
+    async def close(self):
+        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
         return self.key
diff --git a/testkitbackend/_async/requests.py b/testkitbackend/_async/requests.py
@@ -30,6 +30,7 @@
     test_subtest_skips,
     totestkit,
 )
+from .._warning_check import warning_check
 from ..exceptions import MarkdAsDriverException
 
 
@@ -176,8 +177,14 @@ async def GetServerInfo(backend, data):
 async def CheckMultiDBSupport(backend, data):
     driver_id = data["driverId"]
     driver = backend.drivers[driver_id]
+    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", {
-        "id": backend.next_key(), "available": await driver.supports_multi_db()
+        "id": backend.next_key(), "available": available
     })
 
 
@@ -258,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})
 
@@ -374,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})
@@ -534,6 +556,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/backend.py b/testkitbackend/_sync/backend.py
@@ -68,6 +68,23 @@ def __init__(self, rd, wr):
         self._requestHandlers = dict(
             [m for m in getmembers(requests, isfunction)])
 
+    def close(self):
+        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
         return self.key
diff --git a/testkitbackend/_sync/requests.py b/testkitbackend/_sync/requests.py
diff --git a/testkitbackend/_warning_check.py b/testkitbackend/_warning_check.py
diff --git a/testkitbackend/server.py b/testkitbackend/server.py
diff --git a/testkitbackend/totestkit.py b/testkitbackend/totestkit.py
diff --git a/tests/integration/test_readme.py b/tests/integration/test_readme.py
diff --git a/tox.ini b/tox.ini
