From abac9ef3e5c44a82c7ccb8cf5d49241dda517e90 Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Thu, 27 Oct 2022 16:26:26 -0400 Subject: [PATCH 1/3] PyMongo 4 bug fix: don't preemptively close in use client Fixes #2627. --- docs/changelog.rst | 2 ++ mongoengine/connection.py | 8 +++++--- tests/test_connection.py | 20 +++++++++++++++++++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/docs/changelog.rst b/docs/changelog.rst index 282934168..b4fe920d2 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,6 +10,8 @@ Development - Support MONGODB-AWS authentication mechanism (with `authmechanismproperties`) #2507 - Turning off dereferencing for the results of distinct query. #2663 - Add tests against Mongo 5.0 in pipeline +- Bug fix support for PyMongo>=4 to fix "InvalidOperation: Cannot use MongoClient after close" + errors. Changes in 0.24.2 ================= diff --git a/mongoengine/connection.py b/mongoengine/connection.py index 799c98d96..ef285f424 100644 --- a/mongoengine/connection.py +++ b/mongoengine/connection.py @@ -246,9 +246,11 @@ def disconnect(alias=DEFAULT_CONNECTION_NAME): from mongoengine import Document from mongoengine.base.common import _get_documents_by_db - if alias in _connections: - get_connection(alias=alias).close() - del _connections[alias] + connection = _connections.pop(alias, None) + if connection: + # Only close the client if we're removing the final reference. + if connection not in _connections.values(): + connection.close() if alias in _dbs: # Detach all cached collections in Documents diff --git a/tests/test_connection.py b/tests/test_connection.py index c736baf91..b62cec96e 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -5,7 +5,11 @@ import pytest from bson.tz_util import utc from pymongo import MongoClient, ReadPreference -from pymongo.errors import InvalidName, OperationFailure +from pymongo.errors import ( + InvalidName, + InvalidOperation, + OperationFailure, +) import mongoengine.connection from mongoengine import ( @@ -287,6 +291,20 @@ def test_disconnect_silently_pass_if_alias_does_not_exist(self): assert len(connections) == 0 disconnect(alias="not_exist") + def test_disconnect_does_not_close_client_used_by_another_alias(self): + client1 = connect(alias="disconnect_reused_client_test_1") + client2 = connect(alias="disconnect_reused_client_test_2") + assert client1 is client2 + client1.admin.command("ping") + disconnect("disconnect_reused_client_test_1") + # The client is not closed because the second alias still exists. + client2.admin.command("ping") + disconnect("disconnect_reused_client_test_2") + # The client is now closed: + if PYMONGO_VERSION >= (4,): + with pytest.raises(InvalidOperation): + client2.admin.command("ping") + def test_disconnect_all(self): connections = mongoengine.connection._connections dbs = mongoengine.connection._dbs From f1ae62440abea0d141c6518964fda613343f7a5c Mon Sep 17 00:00:00 2001 From: Shane Harvey Date: Fri, 28 Oct 2022 09:28:58 -0400 Subject: [PATCH 2/3] Fix client comparison to use is, not == to correctly close clients and update test to cover this case --- mongoengine/connection.py | 4 +++- tests/test_connection.py | 10 ++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/mongoengine/connection.py b/mongoengine/connection.py index ef285f424..e9968c7f2 100644 --- a/mongoengine/connection.py +++ b/mongoengine/connection.py @@ -249,7 +249,9 @@ def disconnect(alias=DEFAULT_CONNECTION_NAME): connection = _connections.pop(alias, None) if connection: # Only close the client if we're removing the final reference. - if connection not in _connections.values(): + # Use 'is' instead of 'in' or '==' to ensure the clients are + # the same instance. + if all(connection is not c for c in _connections.values()): connection.close() if alias in _dbs: diff --git a/tests/test_connection.py b/tests/test_connection.py index b62cec96e..27be9dd99 100644 --- a/tests/test_connection.py +++ b/tests/test_connection.py @@ -294,7 +294,9 @@ def test_disconnect_silently_pass_if_alias_does_not_exist(self): def test_disconnect_does_not_close_client_used_by_another_alias(self): client1 = connect(alias="disconnect_reused_client_test_1") client2 = connect(alias="disconnect_reused_client_test_2") + client3 = connect(alias="disconnect_reused_client_test_3", maxPoolSize=10) assert client1 is client2 + assert client1 is not client3 client1.admin.command("ping") disconnect("disconnect_reused_client_test_1") # The client is not closed because the second alias still exists. @@ -304,6 +306,14 @@ def test_disconnect_does_not_close_client_used_by_another_alias(self): if PYMONGO_VERSION >= (4,): with pytest.raises(InvalidOperation): client2.admin.command("ping") + # 3rd client connected to the same cluster with different options + # is not closed either. + client3.admin.command("ping") + disconnect("disconnect_reused_client_test_3") + # 3rd client is now closed: + if PYMONGO_VERSION >= (4,): + with pytest.raises(InvalidOperation): + client3.admin.command("ping") def test_disconnect_all(self): connections = mongoengine.connection._connections From f9b6218dd59c561b600ef15a484699ffdcfbf1be Mon Sep 17 00:00:00 2001 From: Bastien Gerard Date: Wed, 28 Dec 2022 10:55:05 +0100 Subject: [PATCH 3/3] improve doc --- mongoengine/connection.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/mongoengine/connection.py b/mongoengine/connection.py index e9968c7f2..b804cc3d8 100644 --- a/mongoengine/connection.py +++ b/mongoengine/connection.py @@ -248,9 +248,11 @@ def disconnect(alias=DEFAULT_CONNECTION_NAME): connection = _connections.pop(alias, None) if connection: - # Only close the client if we're removing the final reference. - # Use 'is' instead of 'in' or '==' to ensure the clients are - # the same instance. + # MongoEngine may share the same MongoClient across multiple aliases + # if connection settings are the same so we only close + # the client if we're removing the final reference. + # Important to use 'is' instead of '==' because clients connected to the same cluster + # will compare equal even with different options if all(connection is not c for c in _connections.values()): connection.close()