From ff85a0af4d0288f0fd33fcebe240061266d8ffea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 13:34:24 +0200 Subject: [PATCH 1/6] Skip GDS notification forwarding if driver configured --- .../query_runner/neo4j_query_runner.py | 21 +++++++++++++++---- .../tests/integration/conftest.py | 2 +- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/graphdatascience/query_runner/neo4j_query_runner.py b/graphdatascience/query_runner/neo4j_query_runner.py index a33ae4fa9..751cd2a9b 100644 --- a/graphdatascience/query_runner/neo4j_query_runner.py +++ b/graphdatascience/query_runner/neo4j_query_runner.py @@ -60,6 +60,15 @@ def create( else: raise ValueError(f"Invalid endpoint type: {type(endpoint)}") + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): + notifications_logger = logging.getLogger("neo4j.notifications") + # the client does not expose YIELD fields so we just skip these warnings for now + notifications_logger.addFilter( + lambda record: ( + "The query used a deprecated field from a procedure" in record.msg and "by 'gds." in record.msg + ) + ) + return query_runner @staticmethod @@ -124,10 +133,14 @@ def run_cypher( else: self._last_bookmarks = session.last_bookmarks() - notifications = result.consume().notifications - if notifications: - for notification in notifications: - self._forward_cypher_warnings(notification) + if ( + Neo4jQueryRunner._NEO4J_DRIVER_VERSION < ServerVersion(5, 21, 0) + or result._warn_notification_severity != "WARNING" + ): + notifications = result.consume().notifications + if notifications: + for notification in notifications: + self._forward_cypher_warnings(notification) return df diff --git a/graphdatascience/tests/integration/conftest.py b/graphdatascience/tests/integration/conftest.py index 5cead0862..810239023 100644 --- a/graphdatascience/tests/integration/conftest.py +++ b/graphdatascience/tests/integration/conftest.py @@ -38,7 +38,7 @@ def neo4j_driver() -> Generator[Driver, None, None]: @pytest.fixture(scope="package") def runner(neo4j_driver: Driver) -> Generator[Neo4jQueryRunner, None, None]: - _runner = Neo4jQueryRunner(neo4j_driver) + _runner = Neo4jQueryRunner.create(neo4j_driver) _runner.set_database(DB) yield _runner From a45f1ec26e1ae2ce890c1bea83db5de6f9cf641f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 14:10:46 +0200 Subject: [PATCH 2/6] Filter out deprecated yield related warnings from driver --- .../query_runner/neo4j_query_runner.py | 5 +++ .../tests/integration/test_error_handling.py | 31 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/graphdatascience/query_runner/neo4j_query_runner.py b/graphdatascience/query_runner/neo4j_query_runner.py index 751cd2a9b..6ebe1a5b8 100644 --- a/graphdatascience/query_runner/neo4j_query_runner.py +++ b/graphdatascience/query_runner/neo4j_query_runner.py @@ -141,6 +141,11 @@ def run_cypher( if notifications: for notification in notifications: self._forward_cypher_warnings(notification) + elif Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): + # the client does not expose YIELD fields so we just skip these warnings for now + warnings.filterwarnings( + "ignore", message=r".*The query used a deprecated field from a procedure\. .* by 'gds.* " + ) return df diff --git a/graphdatascience/tests/integration/test_error_handling.py b/graphdatascience/tests/integration/test_error_handling.py index 9c16551c5..224a68a87 100644 --- a/graphdatascience/tests/integration/test_error_handling.py +++ b/graphdatascience/tests/integration/test_error_handling.py @@ -1,8 +1,12 @@ from typing import Generator import pytest +from neo4j import GraphDatabase, NotificationMinimumSeverity from graphdatascience import GraphDataScience +from graphdatascience.query_runner.neo4j_query_runner import Neo4jQueryRunner +from graphdatascience.server_version.server_version import ServerVersion +from graphdatascience.tests.integration.conftest import AUTH, URI GRAPH_NAME = "g" @@ -189,3 +193,30 @@ def test_auto_completion_false_positives(gds: GraphDataScience) -> None: # Without `graph` prefix with pytest.raises(SyntaxError, match="There is no 'gds.toUndirected' to call"): gds.toUndirected() + + +def test_forward_server_side_warning(gds: GraphDataScience) -> None: + with pytest.raises(Warning, match="The query used a deprecated function: `id`."): + gds.run_cypher("MATCH (n) RETURN id(n)") + + +@pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") +def test_forward_driver_configured_warning() -> None: + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): + driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) + gds = GraphDataScience(driver) + + with pytest.raises(Warning, match="The query used a deprecated function: `id`."): + gds.run_cypher("MATCH (n) RETURN id(n)") + + +def test_filter_out_client_related__warning(gds: GraphDataScience) -> None: + gds.graph.drop("g", failIfMissing=False) + + +@pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") +def test_filter_out_client_related__driver_configured_warning() -> None: + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): + driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) + gds = GraphDataScience(driver) + gds.graph.drop("g", failIfMissing=False) From 0e0319dc94acc772697195899ec51922d58db0f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 16:25:05 +0200 Subject: [PATCH 3/6] Flip condition on neo4j version check Co-authored-by: Paul Horn --- .../query_runner/neo4j_query_runner.py | 16 ++++++------ .../tests/integration/test_error_handling.py | 25 ++++++++++++------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/graphdatascience/query_runner/neo4j_query_runner.py b/graphdatascience/query_runner/neo4j_query_runner.py index 6ebe1a5b8..70a009292 100644 --- a/graphdatascience/query_runner/neo4j_query_runner.py +++ b/graphdatascience/query_runner/neo4j_query_runner.py @@ -60,7 +60,7 @@ def create( else: raise ValueError(f"Invalid endpoint type: {type(endpoint)}") - if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0): notifications_logger = logging.getLogger("neo4j.notifications") # the client does not expose YIELD fields so we just skip these warnings for now notifications_logger.addFilter( @@ -134,18 +134,18 @@ def run_cypher( self._last_bookmarks = session.last_bookmarks() if ( - Neo4jQueryRunner._NEO4J_DRIVER_VERSION < ServerVersion(5, 21, 0) - or result._warn_notification_severity != "WARNING" + Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0) + and result._warn_notification_severity == "WARNING" ): - notifications = result.consume().notifications - if notifications: - for notification in notifications: - self._forward_cypher_warnings(notification) - elif Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): # the client does not expose YIELD fields so we just skip these warnings for now warnings.filterwarnings( "ignore", message=r".*The query used a deprecated field from a procedure\. .* by 'gds.* " ) + else: + notifications = result.consume().notifications + if notifications: + for notification in notifications: + self._forward_cypher_warnings(notification) return df diff --git a/graphdatascience/tests/integration/test_error_handling.py b/graphdatascience/tests/integration/test_error_handling.py index 224a68a87..a3cf5d83f 100644 --- a/graphdatascience/tests/integration/test_error_handling.py +++ b/graphdatascience/tests/integration/test_error_handling.py @@ -1,7 +1,7 @@ from typing import Generator import pytest -from neo4j import GraphDatabase, NotificationMinimumSeverity +from neo4j import Driver, GraphDatabase, NotificationMinimumSeverity from graphdatascience import GraphDataScience from graphdatascience.query_runner.neo4j_query_runner import Neo4jQueryRunner @@ -33,6 +33,15 @@ def run_around_tests(gds: GraphDataScience) -> Generator[None, None, None]: gds.graph.drop(GRAPH_NAME) +@pytest.fixture +def warning_driver() -> Generator[Driver, None, None]: + driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) + + yield driver + + driver.close() + + def test_bogus_algo(gds: GraphDataScience) -> None: G, _ = gds.graph.project(GRAPH_NAME, "*", "*") with pytest.raises(SyntaxError, match="There is no 'gds.bogusAlgoWithLongName.stream' to call$"): @@ -201,10 +210,9 @@ def test_forward_server_side_warning(gds: GraphDataScience) -> None: @pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") -def test_forward_driver_configured_warning() -> None: - if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): - driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) - gds = GraphDataScience(driver) +def test_forward_driver_configured_warning(warning_driver: Driver) -> None: + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0): + gds = GraphDataScience(warning_driver) with pytest.raises(Warning, match="The query used a deprecated function: `id`."): gds.run_cypher("MATCH (n) RETURN id(n)") @@ -215,8 +223,7 @@ def test_filter_out_client_related__warning(gds: GraphDataScience) -> None: @pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") -def test_filter_out_client_related__driver_configured_warning() -> None: - if Neo4jQueryRunner._NEO4J_DRIVER_VERSION > ServerVersion(5, 21, 0): - driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) - gds = GraphDataScience(driver) +def test_filter_out_client_related__driver_configured_warning(warning_driver: Driver) -> None: + if Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0): + gds = GraphDataScience(warning_driver) gds.graph.drop("g", failIfMissing=False) From 23eb310cae21db782f4f1b2be8824238c6d7d83c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 16:25:24 +0200 Subject: [PATCH 4/6] Log change around warn_notification_severity option --- changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/changelog.md b/changelog.md index c73332491..76a2dd2f9 100644 --- a/changelog.md +++ b/changelog.md @@ -28,3 +28,4 @@ ## Other changes * Updated required `neo4j` driver from `4.4.2` to the latest 4.4 path release (`4.4.12`) or later. +* Avoid duplications or user-indepedent logs and warnings introduced by the driver option `warn_notification_severity` in `neo4j>=5.21.0`. From 6bb3e8d9a5b1d2d616813e2b497c7d2b97955de9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 16:33:52 +0200 Subject: [PATCH 5/6] Remove import not existing in neo4j 4.4 --- graphdatascience/tests/integration/test_error_handling.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/graphdatascience/tests/integration/test_error_handling.py b/graphdatascience/tests/integration/test_error_handling.py index a3cf5d83f..118bfbc36 100644 --- a/graphdatascience/tests/integration/test_error_handling.py +++ b/graphdatascience/tests/integration/test_error_handling.py @@ -1,7 +1,7 @@ from typing import Generator import pytest -from neo4j import Driver, GraphDatabase, NotificationMinimumSeverity +from neo4j import Driver, GraphDatabase from graphdatascience import GraphDataScience from graphdatascience.query_runner.neo4j_query_runner import Neo4jQueryRunner @@ -35,7 +35,7 @@ def run_around_tests(gds: GraphDataScience) -> Generator[None, None, None]: @pytest.fixture def warning_driver() -> Generator[Driver, None, None]: - driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity=NotificationMinimumSeverity.WARNING) + driver = GraphDatabase.driver(URI, auth=AUTH, warn_notification_severity="WARNING") yield driver From f87c7c99fbab794e323f15ebcd7e326439a97e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Florentin=20D=C3=B6rre?= Date: Fri, 19 Jul 2024 17:05:57 +0200 Subject: [PATCH 6/6] Run >5.21 driver tests based on pytest mark --- .../tests/integration/conftest.py | 19 ++++++++++++++++--- .../tests/integration/test_error_handling.py | 15 +++++++-------- graphdatascience/tests/pytest.ini | 1 + 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/graphdatascience/tests/integration/conftest.py b/graphdatascience/tests/integration/conftest.py index 810239023..633103068 100644 --- a/graphdatascience/tests/integration/conftest.py +++ b/graphdatascience/tests/integration/conftest.py @@ -194,10 +194,9 @@ def pytest_collection_modifyitems(config: Any, items: Any) -> None: server_version = gds._server_version except Exception as e: print("Could not derive GDS library server version") - gds.close() raise e - - gds.close() + finally: + gds.close() skip_incompatible_versions = pytest.mark.skip(reason=f"incompatible with GDS server version {server_version}") @@ -211,3 +210,17 @@ def pytest_collection_modifyitems(config: Any, items: Any) -> None: if "max_exclusive" in kwargs and kwargs["max_exclusive"] <= server_version: item.add_marker(skip_incompatible_versions) + + db_driver_version = Neo4jQueryRunner._NEO4J_DRIVER_VERSION + skip_incompatible_driver_version = pytest.mark.skip(reason=f"incompatible with driver version {db_driver_version}") + + for item in items: + for mark in item.iter_markers(name="compatible_with_db_driver"): + kwargs = mark.kwargs + + if "min_inclusive" in kwargs and kwargs["min_inclusive"] > db_driver_version: + item.add_marker(skip_incompatible_driver_version) + continue + + if "max_exclusive" in kwargs and kwargs["max_exclusive"] <= db_driver_version: + item.add_marker(skip_incompatible_driver_version) diff --git a/graphdatascience/tests/integration/test_error_handling.py b/graphdatascience/tests/integration/test_error_handling.py index 118bfbc36..1e413869b 100644 --- a/graphdatascience/tests/integration/test_error_handling.py +++ b/graphdatascience/tests/integration/test_error_handling.py @@ -4,7 +4,6 @@ from neo4j import Driver, GraphDatabase from graphdatascience import GraphDataScience -from graphdatascience.query_runner.neo4j_query_runner import Neo4jQueryRunner from graphdatascience.server_version.server_version import ServerVersion from graphdatascience.tests.integration.conftest import AUTH, URI @@ -210,12 +209,12 @@ def test_forward_server_side_warning(gds: GraphDataScience) -> None: @pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") +@pytest.mark.compatible_with_db_driver(min_inclusive=ServerVersion(5, 21, 0)) def test_forward_driver_configured_warning(warning_driver: Driver) -> None: - if Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0): - gds = GraphDataScience(warning_driver) + gds = GraphDataScience(warning_driver) - with pytest.raises(Warning, match="The query used a deprecated function: `id`."): - gds.run_cypher("MATCH (n) RETURN id(n)") + with pytest.raises(Warning, match="The query used a deprecated function: `id`."): + gds.run_cypher("MATCH (n) RETURN id(n)") def test_filter_out_client_related__warning(gds: GraphDataScience) -> None: @@ -223,7 +222,7 @@ def test_filter_out_client_related__warning(gds: GraphDataScience) -> None: @pytest.mark.filterwarnings("ignore: notification warnings are a preview feature") +@pytest.mark.compatible_with_db_driver(min_inclusive=ServerVersion(5, 21, 0)) def test_filter_out_client_related__driver_configured_warning(warning_driver: Driver) -> None: - if Neo4jQueryRunner._NEO4J_DRIVER_VERSION >= ServerVersion(5, 21, 0): - gds = GraphDataScience(warning_driver) - gds.graph.drop("g", failIfMissing=False) + gds = GraphDataScience(warning_driver) + gds.graph.drop("g", failIfMissing=False) diff --git a/graphdatascience/tests/pytest.ini b/graphdatascience/tests/pytest.ini index 467c420dc..1ff7d09a0 100644 --- a/graphdatascience/tests/pytest.ini +++ b/graphdatascience/tests/pytest.ini @@ -4,6 +4,7 @@ markers = encrypted_only: mark a test as requiring GDS with either bolt or https ssl.policy configured. model_store_location: mark a test as a requiring neo4j config for gds.model.store_location compatible_with: mark a test as only being compatible with certain GDS server versions + compatible_with_db_driver: mark a test as only being compatible with a certain Neo4j driver version skip_on_aura: mark a test to not be run when targeting an AuraDS instance only_on_aura: mark a test to be run only when targeting an AuraDS instance ogb: mark a test as requiring the ogb dependency