PYTHON-5078 Convert test.test_discovery_and_monitoring to async by sleepyStick · Pull Request #2093 · mongodb/mongo-python-driver

sleepyStick · 2025-01-29T18:33:04Z

No description provided.

NoahStapp · 2025-01-29T21:10:22Z

test/test_discovery_and_monitoring.py

-                    "maxWireVersion": common.MIN_SUPPORTED_WIRE_VERSION,
-                    "$clusterTime": new,
-                },
+                {"ok": 1, "minWireVersion": 0, "maxWireVersion": 6, "$clusterTime": new},


Intentional change?

nope! i'll undo it, sorry!

Okay not intentional, but it keeps getting reverted by pre-commit hooks so I think that means I should stop fighting it?

You'd need to change it in the async file, otherwise the sync file will be overwritten by the synchro script.

I am changing it in the async file...?

NoahStapp · 2025-01-29T21:12:28Z

tools/synchro.py

    "test_database.py",
+    "test_discovery_and_monitoring.py",
    "test_data_lake.py",
+    "test_discovery_and_monitoring.py",


Duplicate addition here.

oop sorry about that!

NoahStapp · 2025-01-29T21:15:37Z

test/asynchronous/test_discovery_and_monitoring.py

+
+
+class TestHeartbeatStartOrdering(AsyncPyMongoTestCase):
+    @async_client_context.require_sync


We can also re-write this one for async:

class TestHeartbeatStartOrdering(AsyncPyMongoTestCase): def test_heartbeat_start_ordering(self): events = [] def handle_client(reader, writer): ... listener = HeartbeatEventsListListener(events) with asyncio.start_server(handle_client, "localhost", 9999): ...

We'll still keep the current version for the sync API.

test/asynchronous/test_discovery_and_monitoring.py

NoahStapp · 2025-01-29T21:27:12Z

test/asynchronous/test_discovery_and_monitoring.py

+            setattr(TestAllScenarios, new_test.__name__, new_test)
+
+
+create_tests()


Let's assume that #2094 will be merged before this and also make create_tests here async-compatible.

NoahStapp · 2025-01-29T21:29:09Z

test/asynchronous/test_discovery_and_monitoring.py

+
+
+class TestIgnoreStaleErrors(AsyncIntegrationTest):
+    @async_client_context.require_sync


This one can be re-written for async:

N_TASKS = 5 barrier = asyncio.Barrier(N_TASKS) ... for i in range(N_TASKS): tasks.append(asyncio.create_task(insert_command(i))) await asyncio.gather(*tasks)

We can keep the sync version instead of trying to generate it.

Co-authored-by: Noah Stapp <noah@noahstapp.com>

NoahStapp · 2025-02-07T15:06:54Z

test/asynchronous/test_discovery_and_monitoring.py

+
+
+class TestIgnoreStaleErrors(AsyncIntegrationTest):
+    if _IS_SYNC:


We can remove this branch by creating a helper method for Barrier similar to create_async_event and create_event.

I couldn't find a create_async_event and create_event? but i think i did what you wanted? Let me know if I imagined it incorrectly!

NoahStapp · 2025-02-07T15:07:07Z

test/asynchronous/test_discovery_and_monitoring.py

+                    pass
+
+            threads = []
+            for i in range(N_THREADS):


Similarly, we should use ConcurrentRunner here as well.

NoahStapp · 2025-02-07T15:07:38Z

test/asynchronous/test_discovery_and_monitoring.py

+class TestIgnoreStaleErrors(AsyncIntegrationTest):
+    if _IS_SYNC:
+
+        async def test_ignore_stale_connection_errors(self):


Unfortunately, asyncio.Barrier was added in Python 3.11, so we'll need to only run this async test on that version and newer.

How do i get it to only run in python 3.11 and newer?

Add a block like this to the very start of the test:

if not _IS_SYNC and sys.version_info < (3, 11): self.skipTest("Test requires asyncio.Barrier (added in Python 3.11)")

I see, thanks!

NoahStapp · 2025-02-07T15:08:33Z

test/asynchronous/test_discovery_and_monitoring.py

+            server = await asyncio.start_server(handle_client, "localhost", 9999)
+            server.events = events
+            await server.start_serving()
+            print(server.is_serving())


Leftover print statement?

HAHA yup, sorry

NoahStapp · 2025-02-07T15:08:57Z

test/asynchronous/test_discovery_and_monitoring.py

+                serverSelectionTimeoutMS=500,
+                event_listeners=(listener,),
+            )
+            if _c._options.connect:


We always want to connect here, so this if can be removed.

… newer for async

NoahStapp · 2025-02-11T17:05:10Z

test/asynchronous/helpers.py

Whitespace change?

i believe its not there anymore, sorry about that.

NoahStapp · 2025-02-11T18:03:33Z

test/test_discovery_and_monitoring.py

-                    "maxWireVersion": common.MIN_SUPPORTED_WIRE_VERSION,
-                    "$clusterTime": new,
-                },
+                {"ok": 1, "minWireVersion": 0, "maxWireVersion": 6, "$clusterTime": new},


You'd need to change it in the async file, otherwise the sync file will be overwritten by the synchro script.

…ver into PYTHON-5078

NoahStapp · 2025-02-12T13:28:34Z

test/asynchronous/test_discovery_and_monitoring.py

+            setattr(TestAllScenarios, new_test.__name__, new_test)
+
+
+class TestClusterTimeComparison(unittest.IsolatedAsyncioTestCase):


Suggested change

class TestClusterTimeComparison(unittest.IsolatedAsyncioTestCase):

class TestClusterTimeComparison(AsyncPyMongoTestCase):

NoahStapp · 2025-02-12T13:30:04Z

test/asynchronous/test_discovery_and_monitoring.py

+        await async_wait_until(lambda: len(pool.conns) == N_TASKS, "created conns")
+
+        async def mock_command(*args, **kwargs):
+            # Synchronize all threads to ensure they use the same generation.


Suggested change

# Synchronize all threads to ensure they use the same generation.

# Synchronize all tasks to ensure they use the same generation.

NoahStapp · 2025-02-12T13:33:28Z

test/asynchronous/test_discovery_and_monitoring.py

+        if _IS_SYNC:
+            server = TCPServer(("localhost", 9999), MockTCPHandler)
+            server.events = events
+            server_thread = threading.Thread(target=server.handle_request_and_shutdown)


Can we use ConcurrentRunner here and collapse this branch entirely?

would using ConcurrentRunner allow the branch to be squashed entirely? I mainly kept them separate because of the TCPServer vs asyncio.start_server

Okay i don't see how i'd join TCPServer and asyncio.start_server but i changed threading.Thread to be ConcurrentRunner

What if we don't use asyncio.start_server at all, and only use TCPServer and ConcurrentRunner?

TCPServer.handle_request_and_shutdown() is blocking the loop so nothing else runs once that's started >.<

NoahStapp · 2025-02-13T22:22:54Z

Still seeing some failures on Windows.

NoahStapp · 2025-02-18T16:18:30Z

test/test_discovery_and_monitoring.py

            setattr(TestAllScenarios, new_test.__name__, new_test)


-create_tests()


Unintentional removal?

yeah,, should be back now! sorry about that!

NoahStapp · 2025-02-18T16:19:12Z

test/test_discovery_and_monitoring.py

 # Generate unified tests.
 globals().update(generate_test_classes(os.path.join(SDAM_PATH, "unified"), module=__name__))

-


Whitespace.

huh i think my pycharm was reformatting and removed that extra line, fixed pycharm settings and it should be back now :)

ShaneHarvey

I just merged #1925 which adds some small changes to test_discovery_and_monitoring that will need to be applied to the async version in this PR.

ShaneHarvey · 2025-02-28T22:16:28Z

Looks like one more newly added flaky test on the latest patch:

 [2025/02/28 14:01:50.193] _____ TestUnifiedHelloTimeout.test_Driver_extends_timeout_while_streaming _____
 [2025/02/28 14:01:50.193] self = <test.asynchronous.test_discovery_and_monitoring.TestUnifiedHelloTimeout testMethod=test_Driver_extends_timeout_while_streaming>
 [2025/02/28 14:01:50.193]     async def test_case(self):
 [2025/02/28 14:01:50.193] >       await self.run_scenario(spec)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1474:
 [2025/02/28 14:01:50.193] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1416: in run_scenario
 [2025/02/28 14:01:50.193]     await self._run_scenario(spec, uri)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1451: in _run_scenario
 [2025/02/28 14:01:50.193]     await self.run_operations(spec["operations"])
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1251: in run_operations
 [2025/02/28 14:01:50.193]     await self.run_special_operation(op)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1246: in run_special_operation
 [2025/02/28 14:01:50.193]     method(spec["arguments"])
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1134: in _testOperation_assertEventCount
 [2025/02/28 14:01:50.193]     self.assertEqual(self._event_count(client, event), count, f"expected {count} not {event!r}")
 [2025/02/28 14:01:50.193] E   AssertionError: 1 != 0 : expected 0 not {'serverDescriptionChangedEvent': {'newDescription': {'type': 'Unknown'}}}

https://spruce.mongodb.com/task/mongo_python_driver_test_win64_python3.9_test_latest_sharded_cluster_auth_ssl_async_patch_c9a85ad321f98caf314fc9da7a367e94d661abac_67c229ce475953000739759c_25_02_28_21_25_35/logs?execution=0&sortBy=STATUS&sortDir=ASC

How should we deal with this one?

sleepyStick · 2025-02-28T23:14:29Z

Looks like one more newly added flaky test on the latest patch:

 [2025/02/28 14:01:50.193] _____ TestUnifiedHelloTimeout.test_Driver_extends_timeout_while_streaming _____
 [2025/02/28 14:01:50.193] self = <test.asynchronous.test_discovery_and_monitoring.TestUnifiedHelloTimeout testMethod=test_Driver_extends_timeout_while_streaming>
 [2025/02/28 14:01:50.193]     async def test_case(self):
 [2025/02/28 14:01:50.193] >       await self.run_scenario(spec)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1474:
 [2025/02/28 14:01:50.193] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1416: in run_scenario
 [2025/02/28 14:01:50.193]     await self._run_scenario(spec, uri)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1451: in _run_scenario
 [2025/02/28 14:01:50.193]     await self.run_operations(spec["operations"])
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1251: in run_operations
 [2025/02/28 14:01:50.193]     await self.run_special_operation(op)
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1246: in run_special_operation
 [2025/02/28 14:01:50.193]     method(spec["arguments"])
 [2025/02/28 14:01:50.193] test\asynchronous\unified_format.py:1134: in _testOperation_assertEventCount
 [2025/02/28 14:01:50.193]     self.assertEqual(self._event_count(client, event), count, f"expected {count} not {event!r}")
 [2025/02/28 14:01:50.193] E   AssertionError: 1 != 0 : expected 0 not {'serverDescriptionChangedEvent': {'newDescription': {'type': 'Unknown'}}}

https://spruce.mongodb.com/task/mongo_python_driver_test_win64_python3.9_test_latest_sharded_cluster_auth_ssl_async_patch_c9a85ad321f98caf314fc9da7a367e94d661abac_67c229ce475953000739759c_25_02_28_21_25_35/logs?execution=0&sortBy=STATUS&sortDir=ASC

How should we deal with this one?

I'm down to try de-bugging. But there's a part of me that feels like it'll be a similar bug to the other flakey tests? So we can make a ticket for this tests and link it to the other one? Thoughts?

ShaneHarvey · 2025-02-28T23:17:22Z

Sounds good to me.

sleepyStick and others added 3 commits January 29, 2025 10:32

Convert test.test_discovery_and_monitoring to async

2e62605

modify path

3b63840

Merge branch 'master' into PYTHON-5078

a2b75d5

sleepyStick requested a review from NoahStapp January 29, 2025 20:40

sleepyStick marked this pull request as ready for review January 29, 2025 21:04

NoahStapp requested changes Jan 29, 2025

View reviewed changes

sleepyStick and others added 4 commits January 29, 2025 13:48

Update test/asynchronous/test_discovery_and_monitoring.py

004f8f0

Co-authored-by: Noah Stapp <noah@noahstapp.com>

Update test/asynchronous/test_discovery_and_monitoring.py

70d09d3

Co-authored-by: Noah Stapp <noah@noahstapp.com>

debugging..

568cc64

Merge branch 'master' into PYTHON-5078

d7a4a28

sleepyStick changed the title ~~PYTHOH-5078 Convert test.test_discovery_and_monitoring to async~~ PYTHON-5078 Convert test.test_discovery_and_monitoring to async Feb 6, 2025

make test_heartbeat_start_ordering async

47d9ebd

sleepyStick requested a review from NoahStapp February 7, 2025 00:25

NoahStapp requested changes Feb 7, 2025

View reviewed changes

ShaneHarvey mentioned this pull request Feb 7, 2025

PYTHON-4960 More informative error message for stale primary #2115

Merged

sleepyStick and others added 2 commits February 11, 2025 08:56

address review

24d30af

Merge branch 'master' into PYTHON-5078

9b3cd72

sleepyStick requested a review from NoahStapp February 11, 2025 16:59

sleepyStick added 3 commits February 11, 2025 09:10

fix typing and lint

ffb07b3

fix typing

1c1ebef

test_ignore_stale_connection_errors should only run on python3.11 and…

d00eb45

… newer for async

NoahStapp requested changes Feb 11, 2025

View reviewed changes

sleepyStick and others added 5 commits February 11, 2025 11:57

Merge branch 'master' into PYTHON-5078

f22e84b

fix imports

8d0921b

Merge branch 'PYTHON-5078' of github.com:sleepyStick/mongo-python-dri…

d314742

…ver into PYTHON-5078

undo changes to hello response

a5c8930

Merge branch 'master' into PYTHON-5078

041e4e7

sleepyStick requested a review from NoahStapp February 11, 2025 22:09

blink1073 mentioned this pull request Feb 12, 2025

PYTHON-4815 Migrate test_discovery_and_monitoring.py to async #1893

Closed

NoahStapp requested changes Feb 12, 2025

View reviewed changes

address review -ish

17d0e4c

sleepyStick requested a review from NoahStapp February 13, 2025 20:02

Merge branch 'master' into PYTHON-5078

31d2d81

NoahStapp requested changes Feb 18, 2025

View reviewed changes

sleepyStick and others added 5 commits February 18, 2025 11:12

undo accidental changes

c4c9e00

Merge branch 'master' into PYTHON-5078

1e6016e

Merge branch 'master' into PYTHON-5078

c454ade

retrying the tests because they're flakey

a3465e9

Merge branch 'master' into PYTHON-5078

7aab775

ShaneHarvey requested changes Feb 26, 2025

View reviewed changes

sleepyStick and others added 4 commits February 27, 2025 08:07

Merge branch 'master' into PYTHON-5078

9484d7a

update discovery and monitoring based on mongodb#1925

c4beba1

skip flakey test for now

9b4fd45

remove import

c0e37e5

sleepyStick requested review from NoahStapp and ShaneHarvey February 28, 2025 21:58

another flakey test to skip

81f7a1b

NoahStapp approved these changes Mar 3, 2025

View reviewed changes

ShaneHarvey approved these changes Mar 3, 2025

View reviewed changes

sleepyStick merged commit 150a3ba into mongodb:master Mar 3, 2025
46 of 51 checks passed



		class TestHeartbeatStartOrdering(AsyncPyMongoTestCase):
		@async_client_context.require_sync

		setattr(TestAllScenarios, new_test.__name__, new_test)


		create_tests()



		class TestIgnoreStaleErrors(AsyncIntegrationTest):
		@async_client_context.require_sync



		class TestIgnoreStaleErrors(AsyncIntegrationTest):
		if _IS_SYNC:

		setattr(TestAllScenarios, new_test.__name__, new_test)


		class TestClusterTimeComparison(unittest.IsolatedAsyncioTestCase):

	class TestClusterTimeComparison(unittest.IsolatedAsyncioTestCase):
	class TestClusterTimeComparison(AsyncPyMongoTestCase):

	# Synchronize all threads to ensure they use the same generation.
	# Synchronize all tasks to ensure they use the same generation.

		# Generate unified tests.
		globals().update(generate_test_classes(os.path.join(SDAM_PATH, "unified"), module=__name__))

Conversation

sleepyStick commented Jan 29, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp Feb 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NoahStapp commented Feb 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneHarvey left a comment

Choose a reason for hiding this comment

Uh oh!

NoahStapp Feb 11, 2025 •

edited

Loading