Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PYTHON-4731 - Explicitly close all MongoClients opened during tests #1855

Merged
merged 31 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
d7d8ce8
PYTHON-3193 - Add ResourceWarning for unclosed MongoClients in __del__
NoahStapp Sep 5, 2024
f5fbeef
Fix typecheck
NoahStapp Sep 5, 2024
2bac24c
Address review
NoahStapp Sep 5, 2024
c9f99a0
Add traceback
NoahStapp Sep 6, 2024
2fcf40b
WIP - done with test_client
NoahStapp Sep 6, 2024
d45a8c7
All async tests converted
NoahStapp Sep 11, 2024
4ddb75e
WIP all
NoahStapp Sep 11, 2024
2f5e9f5
All warnings fixed
NoahStapp Sep 12, 2024
96519b8
Reduce test_client diff
NoahStapp Sep 12, 2024
2c9da45
WIP
NoahStapp Sep 12, 2024
3f7dce5
Cleanup
NoahStapp Sep 12, 2024
4df385c
Cleanup part 2
NoahStapp Sep 12, 2024
772dcf0
Fix failures
NoahStapp Sep 13, 2024
1ea15cb
More fixes
NoahStapp Sep 13, 2024
0512fa0
Use simple_client instead of raw clients
NoahStapp Sep 13, 2024
55e12cb
More DNS fixes
NoahStapp Sep 13, 2024
a27c852
Fix test_cleanup_executors_on_client_del
NoahStapp Sep 16, 2024
a54d4e3
Remove source reference from MongoClient.__del__
NoahStapp Sep 16, 2024
9cf059b
Fix test_srv_polling
NoahStapp Sep 16, 2024
ab018d7
More test fixes
NoahStapp Sep 16, 2024
a41e554
More fixes
NoahStapp Sep 16, 2024
b899e64
More fixes
NoahStapp Sep 16, 2024
6455959
Merge branch 'master' into PYTHON-4731
NoahStapp Sep 16, 2024
58aeb16
Fix change stream tests
NoahStapp Sep 16, 2024
62d2716
Fix oidc and ocsp tests
NoahStapp Sep 16, 2024
0f67b0d
More fixes
NoahStapp Sep 16, 2024
06b5d0e
More fixes
NoahStapp Sep 16, 2024
12732a6
Fixes woo
NoahStapp Sep 16, 2024
a770fd1
Fixes
NoahStapp Sep 16, 2024
4adcffd
Fix serverless
NoahStapp Sep 16, 2024
0678a89
fixes
NoahStapp Sep 16, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pymongo/asynchronous/mongo_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ def __getitem__(self, name: str) -> database.AsyncDatabase[_DocumentType]:
def __del__(self) -> None:
"""Check that this AsyncMongoClient has been closed and issue a warning if not."""
try:
if not self._closed:
if self._opened and not self._closed:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think we should remove source=self here to prevent holding a strong reference to the client.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, unless we think it's useful to provide the user with the instance of the unclosed client?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the time __del__ is called, there's no guarantees about the state of the object.

warnings.warn(
(
f"Unclosed {type(self).__name__} opened at:\n{self._topology_settings._stack}"
Expand Down
2 changes: 1 addition & 1 deletion pymongo/synchronous/mongo_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -1185,7 +1185,7 @@ def __getitem__(self, name: str) -> database.Database[_DocumentType]:
def __del__(self) -> None:
"""Check that this MongoClient has been closed and issue a warning if not."""
try:
if not self._closed:
if self._opened and not self._closed:
warnings.warn(
(
f"Unclosed {type(self).__name__} opened at:\n{self._topology_settings._stack}"
Expand Down
3 changes: 0 additions & 3 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,6 @@ filterwarnings = [
"module:please use dns.resolver.Resolver.resolve:DeprecationWarning",
# https://github.com/dateutil/dateutil/issues/1314
"module:datetime.datetime.utc:DeprecationWarning:dateutil",
# TODO: Remove both of these in https://jira.mongodb.org/browse/PYTHON-4731
"ignore:Unclosed AsyncMongoClient*",
"ignore:Unclosed MongoClient*",
]
markers = [
"auth_aws: tests that rely on pymongo-auth-aws",
Expand Down
188 changes: 179 additions & 9 deletions test/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
from __future__ import annotations

import asyncio
import base64
import contextlib
import gc
import multiprocessing
import os
Expand All @@ -27,7 +25,6 @@
import sys
import threading
import time
import traceback
import unittest
import warnings
from asyncio import iscoroutinefunction
Expand All @@ -54,6 +51,8 @@
sanitize_reply,
)

from pymongo.uri_parser import parse_uri

try:
import ipaddress

Expand All @@ -80,6 +79,12 @@
_IS_SYNC = True


def _connection_string(h):
if h.startswith(("mongodb://", "mongodb+srv://")):
return h
return f"mongodb://{h!s}"


class ClientContext:
client: MongoClient

Expand Down Expand Up @@ -257,6 +262,8 @@ def _init_client(self):
self.replica_set_name = str(hello["setName"])
self.is_rs = True
if self.auth_enabled:
if self.client:
self.client.close()
# It doesn't matter which member we use as the seed here.
self.client = pymongo.MongoClient(
host,
Expand All @@ -267,6 +274,8 @@ def _init_client(self):
**self.default_client_options,
)
else:
if self.client:
self.client.close()
self.client = pymongo.MongoClient(
host, port, replicaSet=self.replica_set_name, **self.default_client_options
)
Expand Down Expand Up @@ -318,6 +327,7 @@ def _init_client(self):
hello = mongos_client.admin.command(HelloCompat.LEGACY_CMD)
if hello.get("msg") == "isdbgrid":
self.mongoses.append(next_address)
mongos_client.close()

def init(self):
with self.conn_lock:
Expand Down Expand Up @@ -537,12 +547,6 @@ def require_auth(self, func):
lambda: self.auth_enabled, "Authentication is not enabled on the server", func=func
)

def require_no_fips(self, func):
"""Run a test only if the host does not have FIPS enabled."""
return self._require(
lambda: not self.fips_enabled, "Test cannot run on a FIPS-enabled host", func=func
)

def require_no_auth(self, func):
"""Run a test only if the server is running without auth enabled."""
return self._require(
Expand Down Expand Up @@ -930,6 +934,172 @@ def _target() -> None:
self.fail(f"child timed out after {timeout}s (see traceback in logs): deadlock?")
self.assertEqual(proc.exitcode, 0)

@classmethod
def _unmanaged_async_mongo_client(
cls, host, port, authenticate=True, directConnection=None, **kwargs
):
"""Create a new client over SSL/TLS if necessary."""
host = host or client_context.host
port = port or client_context.port
client_options: dict = client_context.default_client_options.copy()
if client_context.replica_set_name and not directConnection:
client_options["replicaSet"] = client_context.replica_set_name
if directConnection is not None:
client_options["directConnection"] = directConnection
client_options.update(kwargs)

uri = _connection_string(host)
auth_mech = kwargs.get("authMechanism", "")
if client_context.auth_enabled and authenticate and auth_mech != "MONGODB-OIDC":
# Only add the default username or password if one is not provided.
res = parse_uri(uri)
if (
not res["username"]
and not res["password"]
and "username" not in client_options
and "password" not in client_options
):
client_options["username"] = db_user
client_options["password"] = db_pwd
client = MongoClient(uri, port, **client_options)
if client._options.connect:
client._connect()
return client

def _async_mongo_client(self, host, port, authenticate=True, directConnection=None, **kwargs):
"""Create a new client over SSL/TLS if necessary."""
host = host or client_context.host
port = port or client_context.port
client_options: dict = client_context.default_client_options.copy()
if client_context.replica_set_name and not directConnection:
client_options["replicaSet"] = client_context.replica_set_name
if directConnection is not None:
client_options["directConnection"] = directConnection
client_options.update(kwargs)

uri = _connection_string(host)
auth_mech = kwargs.get("authMechanism", "")
if client_context.auth_enabled and authenticate and auth_mech != "MONGODB-OIDC":
# Only add the default username or password if one is not provided.
res = parse_uri(uri)
if (
not res["username"]
and not res["password"]
and "username" not in client_options
and "password" not in client_options
):
client_options["username"] = db_user
client_options["password"] = db_pwd
client = MongoClient(uri, port, **client_options)
if client._options.connect:
client._connect()
self.addCleanup(client.close)
return client

@classmethod
def unmanaged_single_client_noauth(
cls, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return cls._unmanaged_async_mongo_client(
h, p, authenticate=False, directConnection=True, **kwargs
)

@classmethod
def unmanaged_single_client(
cls, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return cls._unmanaged_async_mongo_client(h, p, directConnection=True, **kwargs)

@classmethod
def unmanaged_rs_client(cls, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient[dict]:
"""Connect to the replica set and authenticate if necessary."""
return cls._unmanaged_async_mongo_client(h, p, **kwargs)

@classmethod
def unmanaged_rs_client_noauth(
cls, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return cls._unmanaged_async_mongo_client(h, p, authenticate=False, **kwargs)

@classmethod
def unmanaged_rs_or_single_client_noauth(
cls, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return cls._unmanaged_async_mongo_client(h, p, authenticate=False, **kwargs)

@classmethod
def unmanaged_rs_or_single_client(
cls, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return cls._unmanaged_async_mongo_client(h, p, **kwargs)

def single_client_noauth(
self, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Make a direct connection. Don't authenticate."""
return self._async_mongo_client(h, p, authenticate=False, directConnection=True, **kwargs)

def single_client(self, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient[dict]:
"""Make a direct connection, and authenticate if necessary."""
return self._async_mongo_client(h, p, directConnection=True, **kwargs)

def rs_client_noauth(self, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient[dict]:
"""Connect to the replica set. Don't authenticate."""
return self._async_mongo_client(h, p, authenticate=False, **kwargs)

def rs_client(self, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient[dict]:
"""Connect to the replica set and authenticate if necessary."""
return self._async_mongo_client(h, p, **kwargs)

def rs_or_single_client_noauth(
self, h: Any = None, p: Any = None, **kwargs: Any
) -> MongoClient[dict]:
"""Connect to the replica set if there is one, otherwise the standalone.

Like rs_or_single_client, but does not authenticate.
"""
return self._async_mongo_client(h, p, authenticate=False, **kwargs)

def rs_or_single_client(self, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient[Any]:
"""Connect to the replica set if there is one, otherwise the standalone.

Authenticates if necessary.
"""
return self._async_mongo_client(h, p, **kwargs)

def simple_client(self, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient:
if not h and not p:
client = MongoClient(**kwargs)
else:
client = MongoClient(h, p, **kwargs)
self.addCleanup(client.close)
return client

@classmethod
def unmanaged_simple_client(cls, h: Any = None, p: Any = None, **kwargs: Any) -> MongoClient:
if not h and not p:
client = MongoClient(**kwargs)
else:
client = MongoClient(h, p, **kwargs)
return client

def disable_replication(self, client):
"""Disable replication on all secondaries."""
for h, p in client.secondaries:
secondary = self.single_client(h, p)
secondary.admin.command("configureFailPoint", "stopReplProducer", mode="alwaysOn")

def enable_replication(self, client):
"""Enable replication on all secondaries."""
for h, p in client.secondaries:
secondary = self.single_client(h, p)
secondary.admin.command("configureFailPoint", "stopReplProducer", mode="off")


class UnitTest(PyMongoTestCase):
"""Async base class for TestCases that don't require a connection to MongoDB."""
Expand Down
Loading
Loading