PYTHON-4542 Improved sessions API #2335
Conversation
aclark4life
commented
May 7, 2025
aclark4life
commented
- Via context variable.
- Via context variable.
| @@ -222,6 +224,7 @@ def __init__( | |||
| ) | |||
| self._default_transaction_options = default_transaction_options | |||
| self._snapshot = snapshot | |||
| self._bind = bind | |||
There was a problem hiding this comment.
We have to bind/unbind the session in ClientSession.__enter__/__exit__. That way the stack of sessions is managed correctly (ie we call _SESSION.reset(token)). Think about how nested cases will work:
session1 = client.start_session(bind=True)
with session1:
session2 = client.start_session(bind=True)
with session2:
coll.find_one() # uses session2
coll.find_one() # uses session1
coll.find_one() # uses implicit session
test/asynchronous/test_session.py
Outdated
| with session2: | ||
| coll.find_one() # uses session2 | ||
| coll.find_one() # uses session1 | ||
| coll.find_one() # uses implicit session |
There was a problem hiding this comment.
This test has to actually verify the correct sessions are used via command monitoring events.
There was a problem hiding this comment.
How about comparing with the cursor session ? E.g. c36e9df
There was a problem hiding this comment.
@ShaneHarvey I'm assuming that commit is wrong, I should be using command monitoring via event listeners right?
pymongo/asynchronous/mongo_client.py
Outdated
| bind = opts._bind | ||
| session = client_session.AsyncClientSession(self, server_session, opts, implicit) | ||
| if bind: | ||
| _SESSION.set(session) |
There was a problem hiding this comment.
This needs to be removed. We should only bind in __enter__.
There was a problem hiding this comment.
Done in 3c68a70
There was a problem hiding this comment.
@ShaneHarvey OK to resolve this one?
| @@ -545,9 +548,12 @@ def _check_ended(self) -> None: | |||
| raise InvalidOperation("Cannot use ended session") | |||
|
|
|||
| async def __aenter__(self) -> AsyncClientSession: | |||
| self._token = _SESSION.set(self) | |||
There was a problem hiding this comment.
We should only do this if the bind option is set to True.
