PYTHON-4731 - Explicitly close all MongoClients opened during tests #1855
Conversation
|
|
|
The other half of the fix is to remove |
Yup that did it, fantastic catch! |
pymongo/asynchronous/mongo_client.py
Outdated
| @@ -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: | |||
There was a problem hiding this comment.
I still think we should remove source=self here to prevent holding a strong reference to the client.
There was a problem hiding this comment.
Good point, unless we think it's useful to provide the user with the instance of the unclosed client?
There was a problem hiding this comment.
By the time __del__ is called, there's no guarantees about the state of the object.
|
|
|
Also |
Missed one of the branches in |
|
I think you might need to pull from master again to pick up #1853 |
| client_options["password"] = db_pwd | ||
| client = AsyncMongoClient(uri, port, **client_options) | ||
| if client._options.connect: | ||
| await client.aconnect() |
There was a problem hiding this comment.
Can you explain what motivated calling connect() here? It makes it so that _async_mongo_client() behaves differently than AsyncMongoClient() which I worry could cause issues down the line.
There was a problem hiding this comment.
Several places that use an _async_mongo_client() wrapper expect the client it returns to already be connected. In the interest of minimizing the amount of changes, I wanted to keep that expectation where it existed.
Notably, simple_client() directly wraps AsyncMongoClient() instead without any of the extra wrapping we do in _async_mongo_client(). I plan to write up a new section of CONTRIBUTING.md to give guidelines for writing tests and which helper methods to use in creating clients.
No description provided.