Skip to content

Part 4 - asyncio: removes all deprecated patterns and adopts modern asyncio idioms.#972

Open
ping-ke wants to merge 15 commits intoupgrade/py313-baselinefrom
upgrade/asyncio
Open

Part 4 - asyncio: removes all deprecated patterns and adopts modern asyncio idioms.#972
ping-ke wants to merge 15 commits intoupgrade/py313-baselinefrom
upgrade/asyncio

Conversation

@ping-ke
Copy link
Copy Markdown
Contributor

@ping-ke ping-ke commented Mar 15, 2026

Summary

Python 3.10 deprecated the loop= parameter in most asyncio APIs, and Python 3.12 removed it entirely. This removes all deprecated patterns across the codebase and adopts modern asyncio idioms.

Key changes:

  • Remove loop= parameter from asyncio.Event, asyncio.wait, asyncio.ensure_future, asyncio.start_server, open_connection, etc.

  • Replace asyncio.ensure_future() with asyncio.create_task()

  • Replace get_event_loop().run_until_complete() with asyncio.run()

  • Replace asyncio.get_event_loop() with asyncio.get_running_loop() where inside a running event loop

  • Add _get_or_create_event_loop() helper in utils.py for call sites that run before the loop starts (SlaveServer, SlaveConnectionManager)

  • Replace asyncio.Future used as a one-shot signal with asyncio.Event (active_future → active_event, ping_received_future → ping_received_event)

  • Make start()/shutdown() methods async where they previously called loop.run_until_complete() internally

  • Update eth_hash import: BasePreImage → PreImageAPI (eth-hash >=0.5 API)

  • Update cryptography API: EllipticCurvePublicNumbers.from_encoded_point → EllipticCurvePublicKey.from_encoded_point

  • Add conftest.py to cancel pending asyncio tasks between tests

  • Replace assertRaisesRegexp (removed in Py 3.12) with assertRaisesRegex

    Test plan

    • pytest quarkchain/cluster/tests/ passes
    • pytest quarkchain/p2p/ passes
    • No DeprecationWarning about event loop parameters

Python 3.10 deprecated the loop= parameter in most asyncio APIs,
and Python 3.12 removed it entirely. This removes all deprecated
patterns across the codebase and adopts modern asyncio idioms.

Changes:
- Remove loop= parameter from asyncio.Event, asyncio.wait,
  asyncio.ensure_future, asyncio.start_server, open_connection, etc.
- Replace asyncio.ensure_future() with asyncio.create_task()
- Replace get_event_loop().run_until_complete() with asyncio.run()
- Replace asyncio.get_event_loop() with asyncio.get_running_loop()
  where inside a running event loop
- Add _get_or_create_event_loop() helper in utils.py for call sites
  that run before the loop starts (SlaveServer, SlaveConnectionManager)
- Replace asyncio.Future used as a one-shot signal with asyncio.Event
  (active_future → active_event, ping_received_future → ping_received_event)
- Make start()/shutdown() methods async where they previously called
  loop.run_until_complete() internally
- Update eth_hash import: BasePreImage → PreImageAPI (eth-hash >=0.5 API)
- Update cryptography API: EllipticCurvePublicNumbers.from_encoded_point
  → EllipticCurvePublicKey.from_encoded_point
- Add conftest.py to cancel pending asyncio tasks between tests
- Replace assertRaisesRegexp (removed in Py 3.12) with assertRaisesRegex

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@ping-ke ping-ke changed the title modernize asyncio usage for Python 3.10+ compatibility asyncio: removes all deprecated patterns and adopts modern asyncio idioms. Mar 15, 2026
@ping-ke ping-ke changed the title asyncio: removes all deprecated patterns and adopts modern asyncio idioms. Part 4 - asyncio: removes all deprecated patterns and adopts modern asyncio idioms. Mar 15, 2026
@ping-ke ping-ke marked this pull request as draft March 17, 2026 18:04
ping-ke added 2 commits March 19, 2026 00:31
Track and cancel all fire-and-forget asyncio tasks that were leaking
across tests, causing resource exhaustion and timeout failures.

- AbstractConnection: add _loop_task and _handler_tasks tracking;
  use try/finally in active_and_loop_forever to ensure cleanup on
  cancellation; cancel _loop_task in close()
- master.py: track SlaveConnection loop task and __init_cluster task;
  cancel _init_task on shutdown
- slave.py: track MasterConnection, SlaveConnection, PeerShardConnection
  loop tasks and __start_server task
- shard.py: track PeerShardConnection loop task
- miner.py: track and cancel mining task in disable()
- simple_network.py: track Peer loop task and connect_seed task
- test_utils.py: restructure shutdown_clusters with try/finally to
  guarantee task cleanup; await server.wait_closed() for slave servers
- conftest.py: multi-round task cancellation; reset aborted_rpc_count
@ping-ke ping-ke marked this pull request as ready for review March 18, 2026 16:42
@ping-ke ping-ke requested review from qizhou, qzhodl and syntrust March 18, 2026 16:42
@qzhodl
Copy link
Copy Markdown
Contributor

qzhodl commented Mar 25, 2026

Code review

Found 2 issues:

  1. self.upnp_service.discover() does not exist on UPnPService — will raise AttributeError at runtime. The correct method is add_nat_portmap(), which was the original call. UPnPService in quarkchain/p2p/nat.py defines add_nat_portmap() (line 81) and _discover_upnp_devices() (private, line 153), but has no public discover() method.

if self.upnp_service:
mapped_external_ip = await self.upnp_service.discover()
external_ip = mapped_external_ip or "0.0.0.0"

  1. asyncio.create_task() in Logger.send_log_to_kafka() requires a running event loop, but this is a sync @classmethod called from sync methods like Logger.error() and Logger.error_exception(). If logging is triggered outside an async context (e.g., during startup or shutdown), this will raise RuntimeError: no running event loop. The previous asyncio.ensure_future() had the same limitation in Python 3.10+, but this is worth noting as a potential runtime crash when Kafka logging is configured.

}
asyncio.create_task(
cls._kafka_logger.log_kafka_sample_async(
cls._kafka_logger.cluster_config.MONITORING.ERRORS, sample
)
)

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@ping-ke
Copy link
Copy Markdown
Contributor Author

ping-ke commented Mar 26, 2026

Code review

Found 2 issues:

  1. self.upnp_service.discover() does not exist on UPnPService — will raise AttributeError at runtime. The correct method is add_nat_portmap(), which was the original call. UPnPService in quarkchain/p2p/nat.py defines add_nat_portmap() (line 81) and _discover_upnp_devices() (private, line 153), but has no public discover() method.

if self.upnp_service:
mapped_external_ip = await self.upnp_service.discover()
external_ip = mapped_external_ip or "0.0.0.0"

  1. asyncio.create_task() in Logger.send_log_to_kafka() requires a running event loop, but this is a sync @classmethod called from sync methods like Logger.error() and Logger.error_exception(). If logging is triggered outside an async context (e.g., during startup or shutdown), this will raise RuntimeError: no running event loop. The previous asyncio.ensure_future() had the same limitation in Python 3.10+, but this is worth noting as a potential runtime crash when Kafka logging is configured.

}
asyncio.create_task(
cls._kafka_logger.log_kafka_sample_async(
cls._kafka_logger.cluster_config.MONITORING.ERRORS, sample
)
)

🤖 Generated with Claude Code

  • If this code review was useful, please react with 👍. Otherwise, react with 👎.

For issue 1:
discover() has been defined in Part 6, move this change to Part 6 now.
For issue 2:
Fix it as following

      384 -            asyncio.create_task(
      384 +            try:
      385 +                loop = asyncio.get_running_loop()
      386 +            except RuntimeError:
      387 +                # No running event loop (e.g., during startup/shutdown).
      388 +                # Silently skip Kafka logging to avoid crashing the caller.
      389 +                return
      390 +            loop.create_task(
      391                  cls._kafka_logger.log_kafka_sample_async(
      392                      cls._kafka_logger.cluster_config.MONITORING.ERRORS, sample
      393                  )

ping-ke added 3 commits March 26, 2026 18:45
- convert test_cluster.py and test_jsonrpc.py to unittest.IsolatedAsyncioTestCase
- make create_test_clusters/shutdown_clusters async, ClusterContext async context manager
- replace call_async() with await, assert_true_with_timeout with async_assert_true_with_timeout
- replace _get_or_create_event_loop() with asyncio.get_running_loop() in master/slave
- fix mock_pay_native_token_as_gas to support both sync and async wrapped functions
- remove obsolete _get_or_create_event_loop, call_async, assert_true_with_timeout helpers
- fix conftest to restore event loop after IsolatedAsyncioTestCase closes it
ping-ke added 2 commits March 29, 2026 00:14
Migrate test classes to IsolatedAsyncioTestCase so that asyncio.create_task
calls in production code (shard_state, root_state) have a running event loop.
- Replace asyncio.ensure_future with asyncio.create_task in production code
- Replace _get_or_create_event_loop with asyncio.get_running_loop in master/slave
- Convert ClusterContext to async context manager
- Convert create_test_clusters / shutdown_clusters to async
- Add fire_and_forget and async_assert_true_with_timeout utilities
- Simplify conftest fixture to only restore event loop after IsolatedAsyncioTestCase teardown
@ping-ke ping-ke requested a review from qzhodl March 29, 2026 07:49
@ping-ke ping-ke changed the base branch from master to upgrade/py313-baseline April 5, 2026 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants