[PYTHON-5904] RTT Thread Flips Between isMaster and hello#2904
[PYTHON-5904] RTT Thread Flips Between isMaster and hello#2904ronan-merrick wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
NoahStapp
left a comment
NoahStapp
left a comment
There was a problem hiding this comment.
Please read our CONTRIBUTING.md and ensure all changes conform to its guidelines, especially the Project Structure and Asyncio Considerations section.
| await conn._hello(None, None) | ||
| # Verify connection continues to use hello | ||
| self.assertEqual(self._sent[2].get("hello"), 1) | ||
| self.assertIsNone(self._sent[1].get("ismaster", None)) |
There was a problem hiding this comment.
| self.assertIsNone(self._sent[1].get("ismaster", None)) | |
| self.assertIsNone(self._sent[2].get("ismaster", None)) |
| from pymongo.synchronous.pool import Connection | ||
|
|
||
|
|
||
| class TestHelloLatched(unittest.TestCase): |
There was a problem hiding this comment.
This file must be generated using the synchro pre-commit hook from the asynchronous version.
There was a problem hiding this comment.
got it. I misunderstood that these run on commit.
| @@ -0,0 +1,67 @@ | |||
| import unittest | |||
There was a problem hiding this comment.
Needs the standard license header, see another test file for an example.
NoahStapp
left a comment
NoahStapp
left a comment
There was a problem hiding this comment.
Both changes need to be mirrored to the synchronous API. Please run just lint-manual locally to re-generate the synchronous code.
| ) | ||
| self.logical_session_timeout_minutes: Optional[int] = hello.logical_session_timeout_minutes | ||
| self.hello_ok = hello.hello_ok | ||
| # PYTHON-5904 |
There was a problem hiding this comment.
Can you make this comment a little more informative rather than just the ticket name?
NoahStapp
left a comment
NoahStapp
left a comment
There was a problem hiding this comment.
Sorry, we need to update our CONTRIBUTING.md to be clearer and more helpful for adding new test suites.
You need to add test_hello_latched.py to the converted_tests list in tools/synchro.py, then run just lint-manual to generate the synchronous test. You'll also need to add "AsyncMock: Mock" to the replacements dictionary in tools/synchro.py to correctly convert the import from async to sync in the generated file.
| @@ -0,0 +1,79 @@ | |||
| # Copyright 2022-present MongoDB, Inc. | |||
There was a problem hiding this comment.
| # Copyright 2022-present MongoDB, Inc. | |
| # Copyright 2026-present MongoDB, Inc. |
|
|
||
| from pymongo.asynchronous.pool import AsyncConnection | ||
|
|
||
|
|
There was a problem hiding this comment.
The line _IS_SYNC = False needs to be here after the imports.
There was a problem hiding this comment.
No worries, I will make these changes.
Thanks Noah.
3 participants
[JIRA TICKET]
Changes in this PR
I changed the connection to not set hello_ok back to False when the connection switches to hello from ismaster
Test Plan
I reperformed the tests described in the ticket to make sure the issue was no longer occurring and also wrote regression tests.
Checklist
Checklist for Author
Checklist for Reviewer