refactor: Migrate network operations and notifications to use asyncio, aiohttp, aiodns, and aioping. #8

KingPin · 2025-11-24T18:58:54Z

async all the things?

…, aiohttp, aiodns, and aioping.

Copilot

Pull request overview

This PR migrates PingPanda from synchronous network operations to asynchronous I/O using asyncio, aiohttp, aiodns, and aioping. The refactor replaces the ThreadPoolExecutor-based concurrency model with native asyncio concurrency, converting all check functions and notification methods to async/await patterns. This enables more efficient I/O handling and improved scalability when monitoring multiple targets.

Key Changes

Replaced synchronous libraries (requests, pythonping, socket.gethostbyname) with async alternatives (aiohttp, aioping, aiodns)
Converted all check methods and notification sending to async/await patterns with asyncio.gather() for concurrent execution
Updated test suite to use pytest-asyncio with AsyncMock for testing async functions

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
requirements.txt	Updated dependencies to async libraries (aiohttp, aiodns, aioping), added pytest-asyncio for tests
pingpanda.py	Wrapped monitor.run() in asyncio.run() to create event loop for async execution
pingpanda_core/app.py	Removed ThreadPoolExecutor, added aiohttp.ClientSession and aiodns.DNSResolver initialization, converted run() and cleanup to async
pingpanda_core/checks.py	Converted all check classes (DNS, Ping, Website, SSL) to async methods using aiodns, aioping, and aiohttp with asyncio.gather() for parallel execution
pingpanda_core/notifications.py	Converted notification methods to async using aiohttp for webhook delivery with asyncio.gather() for parallel notifications
tests/test_notifications.py	Updated test to use @pytest.mark.asyncio and await async notification calls
tests/test_cli.py	Updated DummyMonitor mock to have async run() method
tests/test_checks.py	Converted all tests to async with @pytest.mark.asyncio, updated mocks to use AsyncMock for async functions

Copilot · 2025-11-24T19:04:24Z

pingpanda_core/checks.py

+            asyncio.create_task(self.app.send_notification(
                f"IP {ip} is flapping (>{self.app.flap_threshold} status changes in {self.app.flap_window_seconds}s)",
                status="error",
                check_type="Flapping",
                target=ip,
-            )
+            ))


Using asyncio.create_task() without storing or awaiting the task can lead to unhandled exceptions and the notification may not complete before the application shuts down. The task should either be awaited (by making _update_stats async) or stored and awaited later during cleanup. Additionally, if an exception occurs in the task, it will be silently ignored unless the task is awaited or has an exception handler.

Copilot · 2025-11-24T19:04:24Z

pingpanda_core/checks.py

-                            website,
-                            response.status_code,
-                            duration_ms,
+                async with session.get(website, timeout=10) as response:


Same issue as in notifications.py: the timeout=10 parameter should be wrapped in an aiohttp.ClientTimeout object. This should be: session.get(website, timeout=aiohttp.ClientTimeout(total=10))

Suggested change

async with session.get(website, timeout=10) as response:

async with session.get(website, timeout=aiohttp.ClientTimeout(total=10)) as response:

Copilot · 2025-11-24T19:04:25Z

pingpanda_core/app.py


+        # Initialize async resources
+        self.http_session = aiohttp.ClientSession()
+        self.dns_resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())


The aiodns.DNSResolver(loop=asyncio.get_running_loop()) uses the deprecated loop parameter. In modern asyncio (Python 3.10+), the loop parameter is deprecated and should be removed. Use aiodns.DNSResolver() instead and let it automatically use the current event loop.

Suggested change

self.dns_resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())

self.dns_resolver = aiodns.DNSResolver()

Copilot · 2025-11-24T19:04:25Z

pingpanda_core/checks.py

+                resolver = getattr(app, "dns_resolver", None)
+                if not resolver:
+                    # Fallback if not initialized in app (though it should be)
+                    resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())


Same issue as in app.py: the loop=asyncio.get_running_loop() parameter is deprecated. Use aiodns.DNSResolver() without the loop parameter.

Suggested change

resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())

resolver = aiodns.DNSResolver()

Copilot · 2025-11-24T19:04:25Z

pingpanda_core/notifications.py

+        # We could log partial failures here if needed



The return_exceptions=True in asyncio.gather() silently swallows exceptions without logging them. If any notification fails with an exception (not just returning False), the failure will be invisible to users. Consider iterating through results and logging any exceptions that occurred, or remove return_exceptions=True to let exceptions propagate.

Suggested change

# We could log partial failures here if needed

# Log any notification channel failures

channels = []

if self.settings.slack_webhook_url:

channels.append("Slack")

if self.settings.teams_webhook_url:

channels.append("Teams")

if self.settings.discord_webhook_url:

channels.append("Discord")

for idx, result in enumerate(results):

if isinstance(result, Exception):

self.logger.error(

f"Notification to {channels[idx]} failed: {result!r}"

)

Copilot · 2025-11-24T19:04:26Z

pingpanda_core/checks.py

+                # aioping returns delay in seconds
+                delay = await aioping.ping(ip, timeout=2)
+
+                elapsed = time.perf_counter() - start_time


Variable elapsed is not used.

Suggested change

elapsed = time.perf_counter() - start_time

Copilot · 2025-11-24T19:04:27Z

pingpanda_core/notifications.py

            self.logger.debug("Notification suppressed; no webhook endpoints configured")
+            return
+
+        results = await asyncio.gather(*tasks, return_exceptions=True)


Variable results is not used.

Copilot · 2025-11-24T19:04:27Z

pingpanda_core/checks.py

 from dataclasses import dataclass
 from datetime import datetime
-from typing import Any, Dict, Optional
+from typing import Any, Dict, Optional, List


Import of 'List' is not used.

Suggested change

from typing import Any, Dict, Optional, List

from typing import Any, Dict, Optional

Copilot · 2025-11-24T19:04:27Z

tests/test_cli.py

@@ -1,5 +1,5 @@
 import sys
-
+import asyncio


Import of 'asyncio' is not used.

Suggested change

import asyncio

Copilot · 2025-11-24T19:04:27Z

pingpanda.py

-    monitor.run()
+    try:
+        asyncio.run(monitor.run())
+    except KeyboardInterrupt:


'except' clause does nothing but pass and there is no explanatory comment.

Suggested change

except KeyboardInterrupt:

except KeyboardInterrupt:

# Allow clean exit on Ctrl+C without traceback

…and notifications.

…alization.

and Discord notifications.

…cation HTTP requests.

…ng duration.

…own on `KeyboardInterrupt`.

Copilot

Pull request overview

Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.

Comments suppressed due to low confidence (1)

tests/test_cli.py:2

Import of 'asyncio' is not used.

import asyncio

Copilot · 2025-11-24T20:57:38Z

pingpanda_core/checks.py

-                    elapsed = time.perf_counter() - start_time
-                    duration_ms = response_list.rtt_avg_ms
+        success = False
+        success = False


Duplicate variable assignment. Remove one of the success = False declarations.

Suggested change

success = False

Copilot · 2025-11-24T20:57:38Z

tests/test_cli.py

@@ -1,5 +1,5 @@
 import sys
-
+import asyncio


The asyncio import is unused in this test file. The test itself doesn't directly use asyncio - it's handled by pingpanda.main(). Remove this unused import.

Suggested change

import asyncio

Copilot · 2025-11-24T20:57:39Z

pingpanda_core/checks.py

+                if attempt < app.retry_count - 1:
+                    await asyncio.sleep(1)


[nitpick] Inconsistent retry delay behavior. In DNS check, retry delay only happens if not on the last attempt (if attempt < app.retry_count - 1), but other checks follow the same pattern. However, the original synchronous code used unconditional time.sleep(1). Consider whether this conditional is intentional or if it should remain unconditional for consistency with original behavior.

Suggested change

if attempt < app.retry_count - 1:

await asyncio.sleep(1)

await asyncio.sleep(1)

Copilot · 2025-11-24T20:57:39Z

pingpanda_core/checks.py

+                if attempt < app.retry_count - 1:
+                    await asyncio.sleep(1)


[nitpick] Same as DNS check - the retry delay is now conditional on not being the last attempt, which differs from the original synchronous code that always slept between attempts. This changes retry timing behavior. Verify this is intentional.

Suggested change

if attempt < app.retry_count - 1:

await asyncio.sleep(1)

await asyncio.sleep(1)

Copilot · 2025-11-24T20:57:39Z

pingpanda.py

-    monitor.run()
+    try:
+        asyncio.run(monitor.run())
+    except KeyboardInterrupt:


'except' clause does nothing but pass and there is no explanatory comment.

refactor: Migrate network operations and notifications to use asyncio…

3e24117

…, aiohttp, aiodns, and aioping.

KingPin requested a review from Copilot November 24, 2025 18:58

Copilot started reviewing on behalf of KingPin November 24, 2025 18:59 View session

Copilot finished reviewing on behalf of KingPin November 24, 2025 19:02

Copilot AI reviewed Nov 24, 2025

View reviewed changes

KingPin added 7 commits November 24, 2025 14:21

refactor: make _update_stats an async function and await its calls …

a4dada5

…and notifications.

refactor: update aiohttp request timeouts to use ClientTimeout object

ae7c2c0

refactor: Remove explicit loop argument from aiodns.DNSResolver initi…

fbb5ac9

…alization.

feat: Log specific errors for failed Slack, Teams,

87bcd09

and Discord notifications.

feat: allow passing and reusing an aiohttp.ClientSession for notifi…

74260d6

…cation HTTP requests.

refactor: Remove perf_counter based elapsed time calculation for pi…

03f0a7f

…ng duration.

chore: Remove unused List import and add comment for graceful shutd…

ba8344f

…own on `KeyboardInterrupt`.

KingPin requested a review from Copilot November 24, 2025 20:54

Copilot started reviewing on behalf of KingPin November 24, 2025 20:54 View session

Copilot finished reviewing on behalf of KingPin November 24, 2025 20:56

Copilot AI reviewed Nov 24, 2025

View reviewed changes

fix: Remove duplicate assignment of success variable.

12c6dea

KingPin merged commit b966cd0 into main Nov 25, 2025
5 checks passed

KingPin deleted the async-2025-1 branch November 25, 2025 17:11

	async with session.get(website, timeout=10) as response:
	async with session.get(website, timeout=aiohttp.ClientTimeout(total=10)) as response:

	self.dns_resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())
	self.dns_resolver = aiodns.DNSResolver()

	resolver = aiodns.DNSResolver(loop=asyncio.get_running_loop())
	resolver = aiodns.DNSResolver()

-        # We could log partial failures here if needed
+        # Log any notification channel failures
+        channels = []
+        if self.settings.slack_webhook_url:
+            channels.append("Slack")
+        if self.settings.teams_webhook_url:
+            channels.append("Teams")
+        if self.settings.discord_webhook_url:
+            channels.append("Discord")
+        for idx, result in enumerate(results):
+            if isinstance(result, Exception):
+                self.logger.error(
+                    f"Notification to {channels[idx]} failed: {result!r}"
+                )

	from typing import Any, Dict, Optional, List
	from typing import Any, Dict, Optional

	except KeyboardInterrupt:
	except KeyboardInterrupt:
	# Allow clean exit on Ctrl+C without traceback

	if attempt < app.retry_count - 1:
	await asyncio.sleep(1)
	await asyncio.sleep(1)

refactor: Migrate network operations and notifications to use asyncio, aiohttp, aiodns, and aioping. #8

refactor: Migrate network operations and notifications to use asyncio, aiohttp, aiodns, and aioping. #8

Uh oh!

Conversation

KingPin commented Nov 24, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants