feat: Phase 2 - Network Health Checker #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Rename 3-network-health-checker to network_health_checker (valid Python package name) - Add Pydantic data models (PingResult, PortScanResult, DNSRecord, SubnetInfo, etc.) - Add configuration management with pydantic-settings - Implement network tools: - ping_monitor: ICMP ping with ProcessPoolExecutor - port_scanner: TCP port scanning with ThreadPoolExecutor - dns_lookup: DNS queries using dnspython - subnet_calculator: IP/subnet calculations - snmp_query: SNMP v2c queries using pysnmp - network_info: Local network interface info using psutil - Add Typer CLI with Rich output for all network tools - Add comprehensive unit tests (118 tests passing) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Reviewer's GuideImplements a full-featured Sequence diagram for CLI ping command flowsequenceDiagram
actor User
participant CLI as netcheck_Typer_CLI
participant PingMonitor as ping_monitor.ping_host
participant Socket as socket.gethostbyname
participant Ping3 as ping3.ping
participant Models as PingResult
participant Rich as Rich_Console
User->>CLI: netcheck ping host -c count -t timeout
CLI->>PingMonitor: ping_host(host, timeout, count)
PingMonitor->>Socket: gethostbyname(host)
Socket-->>PingMonitor: ip_address or error
alt hostname_resolution_error
PingMonitor-->>CLI: PingResult(status=ERROR)
CLI->>Rich: render error Panel
else resolved_ok
loop count times
PingMonitor->>Ping3: ping(ip_address, timeout)
Ping3-->>PingMonitor: latency_ms or None
end
alt any_successful_ping
PingMonitor-->>CLI: PingResult(status=UP, latency_ms)
CLI->>Rich: render green Panel with latency
else timeout_or_error
PingMonitor-->>CLI: PingResult(status=TIMEOUT or ERROR)
CLI->>Rich: render yellow or red Panel
end
end
Class diagram for core pydantic models and settingsclassDiagram
class Settings {
+str snmp_community
+str snmp_version
+int snmp_port
+float default_timeout
+float ping_timeout
+float port_scan_timeout
+float dns_timeout
+str mikrotik_host
+str ubiquiti_host
+str omada_host
+str log_level
+get_settings()
}
class HostStatus {
<<enumeration>>
UP
DOWN
TIMEOUT
ERROR
}
class PingResult {
+str host
+str ip_address
+HostStatus status
+float latency_ms
+datetime timestamp
+str error_message
}
class PortScanResult {
+str host
+int port
+bool is_open
+str service_name
+str banner
+float latency_ms
}
class DNSRecord {
+str query
+str record_type
+List~str~ values
+int ttl
}
class SubnetInfo {
+str network
+str netmask
+str broadcast
+str first_host
+str last_host
+int total_hosts
+int cidr
}
class SNMPInterface {
+int index
+str name
+int type
+int mtu
+int speed
+str phys_address
+str oper_status
+int in_octets
+int out_octets
}
class NetworkDevice {
+str host
+str sys_name
+str sys_descr
+str sys_object_id
+int uptime_seconds
+str sys_contact
+str sys_location
+List~SNMPInterface~ interfaces
}
class NetworkInterface {
+str name
+str ipv4_address
+str ipv4_netmask
+str ipv6_address
+str mac_address
+bool is_up
+int speed_mbps
+int mtu
}
class ConnectionInfo {
+str protocol
+str local_address
+int local_port
+str remote_address
+int remote_port
+str status
+int pid
+str process_name
}
Settings ..> Settings : get_settings
PingResult --> HostStatus
NetworkDevice "1" *-- "0..*" SNMPInterface
NetworkDevice ..> SNMPInterface : contains
NetworkInterface ..> ConnectionInfo : used_by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughThe PR introduces a comprehensive network health checker package, replacing the old Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Key areas requiring focused review:
Poem
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (21)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The implementation of
get_default_gatewayinnetwork_tools/network_info.pycurrently guesses the gateway asx.x.x.1based on the first non-loopback address, which is a fragile heuristic for a function named as if it returns the actual system default gateway; consider either using a more reliable source (e.g.psutil.net_if_stats()/psutil.net_if_addrs()combined with platform routing info) or renaming/adjusting the function and its docstring to reflect the best-effort nature of the result. - In
snmp_query.get_system_infothe comment claims "Párhuzamos lekérdezések" but the OIDs are awaited sequentially; either switch toasyncio.gatherto actually parallelize the SNMP GETs or update the comment to avoid implying concurrency that is not there.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The implementation of `get_default_gateway` in `network_tools/network_info.py` currently guesses the gateway as `x.x.x.1` based on the first non-loopback address, which is a fragile heuristic for a function named as if it returns the actual system default gateway; consider either using a more reliable source (e.g. `psutil.net_if_stats()/psutil.net_if_addrs()` combined with platform routing info) or renaming/adjusting the function and its docstring to reflect the best-effort nature of the result.
- In `snmp_query.get_system_info` the comment claims "Párhuzamos lekérdezések" but the OIDs are awaited sequentially; either switch to `asyncio.gather` to actually parallelize the SNMP GETs or update the comment to avoid implying concurrency that is not there.
## Individual Comments
### Comment 1
<location> `network_health_checker/network_tools/subnet_calculator.py:47-48` </location>
<code_context>
+ # A hálózati és broadcast cím nem használható host-ként
+ total_hosts = network.num_addresses - 2 if network.prefixlen < 31 else network.num_addresses
+
+ # Első és utolsó host meghatározása
+ hosts = list(network.hosts())
+ first_host = str(hosts[0]) if hosts else str(network.network_address)
+ last_host = str(hosts[-1]) if hosts else str(network.broadcast_address)
</code_context>
<issue_to_address>
**issue (performance):** Materializing all hosts into a list can blow up memory for large subnets.
For networks like /16 this can create millions of `IPv4Address` objects just to derive first/last. Instead, get the first host via `next(network.hosts(), None)` and compute the last host from `network.broadcast_address`/`network.network_address` (or by iterating to the last element without materializing the whole sequence) so you avoid building the full list in memory.
</issue_to_address>
### Comment 2
<location> `network_health_checker/network_tools/subnet_calculator.py:39-48` </location>
<code_context>
+ """
+ try:
+ # IPv4Network objektum létrehozása
+ network = ipaddress.IPv4Network(cidr, strict=False)
+ except ValueError as e:
+ raise ValueError(f"Invalid CIDR notation: {cidr} - {e}")
+
+ # Hostok számának kiszámítása
+ # A hálózati és broadcast cím nem használható host-ként
+ total_hosts = network.num_addresses - 2 if network.prefixlen < 31 else network.num_addresses
+
+ # Első és utolsó host meghatározása
+ hosts = list(network.hosts())
+ first_host = str(hosts[0]) if hosts else str(network.network_address)
+ last_host = str(hosts[-1]) if hosts else str(network.broadcast_address)
</code_context>
<issue_to_address>
**issue (performance):** get_subnet_hosts also eagerly allocates all hosts, which is risky for big CIDR ranges.
Because `list(network.hosts())` materializes every host, the `limit` parameter and docstring warning don’t prevent high memory usage on large networks. Consider iterating over `network.hosts()` and collecting up to `limit` items (or until exhaustion) instead of building the full list.
</issue_to_address>
### Comment 3
<location> `network_health_checker/network_tools/port_scanner.py:226-229` </location>
<code_context>
+
+ for part in ports_str.split(","):
+ part = part.strip()
+ if "-" in part:
+ # Port tartomány
+ start, end = part.split("-", 1)
+ port_list.extend(range(int(start), int(end) + 1))
+ else:
+ # Egyedi port
</code_context>
<issue_to_address>
**issue (bug_risk):** Port string parsing lacks validation and can raise ValueError on bad input or invalid ranges.
In `_parse_ports`, cases like empty parts, non-numeric values (e.g. `80-abc`), or reversed ranges (e.g. `100-1`) will cause `int(...)`/`range(...)` to raise and crash the CLI. Please validate each segment before converting, handle conversion errors, enforce valid port bounds (1–65535), and surface a clear user-facing error instead of letting the exception propagate.
</issue_to_address>
### Comment 4
<location> `network_health_checker/network_tools/network_info.py:133-142` </location>
<code_context>
+ """
+ # psutil nem ad direkt gateway infót, gateways() használata
+ try:
+ gateways = psutil.net_if_addrs()
+ # Alternatív: netifaces library vagy socket route info
+ # Itt egyszerűsített megoldás: próbáljuk kitalálni az első
+ # nem-loopback interfész hálózatából
+
+ for name, addrs in gateways.items():
+ if name.lower() in ("lo", "loopback", "lo0"):
+ continue
+ for addr in addrs:
+ if addr.family == socket.AF_INET and addr.address:
+ # Tipikus gateway: x.x.x.1 vagy x.x.x.254
+ parts = addr.address.split(".")
+ if len(parts) == 4:
+ # Próbáljuk meg a .1-es címet
+ gateway = f"{parts[0]}.{parts[1]}.{parts[2]}.1"
+ return gateway
+ except Exception:
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Default gateway detection is heuristic and can be misleading on many setups.
Guessing the gateway as `x.x.x.1` from `psutil.net_if_addrs()` will fail on many real networks (`.254`, multiple routers, PPP, etc.). Since psutil doesn’t expose gateways, either augment this with OS-specific routing inspection, or clearly mark this as a best-effort heuristic (e.g. different name/return type) so callers don’t treat it as authoritative.
</issue_to_address>
### Comment 5
<location> `network_health_checker/network_tools/ping_monitor.py:28` </location>
<code_context>
+ host: str,
+ timeout: float | None = None,
+ count: int = 1,
+ privileged: bool = False,
+) -> PingResult:
+ """
</code_context>
<issue_to_address>
**issue:** The `privileged` parameter is unused, which may confuse API consumers.
Since `privileged` is neither passed to `ping3.ping` nor used to change behavior, it currently has no effect. Please either connect it to the implementation (e.g., pass `privileged` through or branch on its value) or remove it to avoid implying unsupported behavior.
</issue_to_address>
### Comment 6
<location> `network_health_checker/network_tools/snmp_query.py:185-191` </location>
<code_context>
+ ... print(f"Uptime: {device.uptime_seconds}s")
+ """
+ try:
+ # Párhuzamos lekérdezések az összes system OID-re
+ sys_descr = await snmp_get(host, SNMP_OIDS["sysDescr"], community)
+ sys_object_id = await snmp_get(host, SNMP_OIDS["sysObjectID"], community)
+ sys_uptime = await snmp_get(host, SNMP_OIDS["sysUpTime"], community)
+ sys_contact = await snmp_get(host, SNMP_OIDS["sysContact"], community)
+ sys_name = await snmp_get(host, SNMP_OIDS["sysName"], community)
+ sys_location = await snmp_get(host, SNMP_OIDS["sysLocation"], community)
+
+ # Ha semmit nem kaptunk, az eszköz nem elérhető
</code_context>
<issue_to_address>
**suggestion (performance):** System OID SNMP GETs are performed sequentially, despite the comment stating they are parallel.
These SNMP GETs are awaited sequentially, which adds unnecessary latency, particularly on high-latency links or when timeouts occur. To match the comment and improve responsiveness, consider running them concurrently with something like `asyncio.gather(...)` instead of awaiting each call in sequence.
Suggested implementation:
```python
Example:
>>> device = await get_system_info("192.168.1.1")
>>> if device:
... print(f"Name: {device.sys_name}")
... print(f"Uptime: {device.uptime_seconds}s")
"""
try:
# Párhuzamos lekérdezések az összes system OID-re asyncio.gather-rel
(
sys_descr,
sys_object_id,
sys_uptime,
sys_contact,
sys_name,
sys_location,
) = await asyncio.gather(
snmp_get(host, SNMP_OIDS["sysDescr"], community),
snmp_get(host, SNMP_OIDS["sysObjectID"], community),
snmp_get(host, SNMP_OIDS["sysUpTime"], community),
snmp_get(host, SNMP_OIDS["sysContact"], community),
snmp_get(host, SNMP_OIDS["sysName"], community),
snmp_get(host, SNMP_OIDS["sysLocation"], community),
)
# Ha semmit nem kaptunk, az eszköz nem elérhető
if all(v is None for v in (sys_descr, sys_name, sys_uptime)):
return None
```
To compile and run correctly, this function also needs access to the `asyncio` module. At the top of `network_health_checker/network_tools/snmp_query.py`, add `import asyncio` alongside the other imports (or ensure it is already present).
</issue_to_address>
### Comment 7
<location> `tests/test_ping_monitor.py:89-95` </location>
<code_context>
+
+ assert results == []
+
+ def test_mixed_results(
+ self, mock_ping_mixed, mock_socket_resolve, mock_socket_resolve_mixed
+ ):
+ """Vegyes eredmények (UP és TIMEOUT)."""
+ # Ezt nehéz mockkolni ProcessPoolExecutor miatt
+ # Egyszerűsített teszt
+ pass
+
+
</code_context>
<issue_to_address>
**issue (testing):** The `test_mixed_results` test is effectively a stub and doesn’t verify `ping_hosts` behavior
Since this test only has a `pass`, the main multi-host path of `ping_hosts` isn’t exercised at all. To make it testable, you could either inject an executor/helper into `ping_hosts` so tests can swap in a synchronous fake, or patch `ProcessPoolExecutor` in the test with a fake that just calls the target function directly. Then you can assert that a mix of UP and TIMEOUT inputs produces the expected `PingResult` list.
</issue_to_address>
### Comment 8
<location> `tests/test_snmp_query.py:60-66` </location>
<code_context>
+ # Mock hiba esetén None
+ assert result is None
+
+ async def test_returns_none_on_timeout(self):
+ """Timeout esetén None visszaadása."""
+ # Rövid timeout nem létező host-ra
+ result = await snmp_get("192.168.254.254", "1.3.6.1.2.1.1.1.0", timeout=0.1)
+ # Valós hálózat nélkül valószínűleg None
+ # Reason: Ez integrációs teszt lenne, mockkolni kellene
+ pass
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Several async SNMP tests are placeholders or rely on real network behavior instead of asserting concrete behavior via mocks
Several tests here are effectively no-ops or rely on real network behavior (e.g. `test_returns_none_on_timeout`, `test_returns_network_device_on_success`, `test_returns_list`, `test_returns_dict`). To make these true unit tests and deterministic, consider:
- Mocking pysnmp primitives (`get_cmd`, `bulk_cmd`) and asserting the parsed result for a happy-path `snmp_get`.
- For `snmp_get_bulk`, simulating multiple `var_binds` and asserting iteration stops when the OID leaves the requested subtree.
- For `get_interfaces`/`get_interface_stats`, mocking `snmp_get_bulk`/`snmp_get` to return a small synthetic table and asserting the resulting `SNMPInterface` objects and stats dict.
- For `check_snmp_reachable`, mocking `snmp_get` to return both `None` and non-`None` and asserting the boolean result.
This will make the tests deterministic and remove the `pass` placeholders without depending on a real SNMP device.
</issue_to_address>
### Comment 9
<location> `network_health_checker/cli.py:73` </location>
<code_context>
+
+ result = ping_host(host, timeout=timeout, count=count)
+
+ if result.status == HostStatus.UP:
+ console.print(
+ Panel(
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared formatting and table-building helpers so this CLI module avoids repetition and is easier to maintain.
You can keep the same behavior while cutting down a lot of repetition and making the file easier to maintain by introducing small, focused helpers for:
- status → colored string mapping
- table construction for repeated patterns
- shared formatting (e.g. bytes, uptime)
Below are concrete, minimal refactors that preserve behavior but reduce complexity.
---
### 1. Extract common status / latency rendering
Ping, port scan, interfaces, SNMP all encode similar status/color and latency formatting inline.
```python
# helpers.py (new module or top of this file)
from rich.table import Table
from .models import HostStatus
def format_ping_status(result) -> tuple[str, str]:
if result.status == HostStatus.UP:
return "[green]UP[/green]", f"{result.latency_ms}ms"
if result.status == HostStatus.TIMEOUT:
return "[yellow]TIMEOUT[/yellow]", "-"
return "[red]ERROR[/red]", "-"
def format_port_status(is_open: bool, latency_ms: float | None) -> tuple[str, str]:
if is_open:
return "[green]OPEN[/green]", f"{latency_ms}ms" if latency_ms else "-"
return "[red]CLOSED[/red]", "-"
def format_conn_status(status: str) -> str:
color = "green" if status == "ESTABLISHED" else "yellow" if status == "LISTEN" else "white"
return f"[{color}]{status}[/{color}]"
```
Usage example in `check`:
```python
from .helpers import format_ping_status
for host in hosts:
result = ping_host(host, timeout=timeout, count=1)
status, latency = format_ping_status(result)
table.add_row(host, status, latency)
```
Usage in `scan`:
```python
from .helpers import format_port_status
for result in results:
status, latency = format_port_status(result.is_open, result.latency_ms)
row = [str(result.port), status, result.service_name or "-", latency]
...
```
Usage in `connections`:
```python
from .helpers import format_conn_status
for conn in connections[:50]:
...
table.add_row(
conn.protocol.upper(),
local,
remote,
format_conn_status(conn.status),
conn.process_name or "-",
)
```
---
### 2. Extract reusable table renderers for repeated patterns
DNS commands (`dns`, `dns-all`, `mx`, `ns`) build nearly identical tables.
```python
# helpers.py
from rich.console import Console
from rich.table import Table
console = Console()
def render_dns_records_table(title: str, records) -> None:
table = Table(title=title)
table.add_column("Value", style="cyan")
table.add_column("TTL", justify="right", style="dim")
for value in records.values:
table.add_row(value, str(records.ttl) if records.ttl else "-")
console.print(table)
```
Use in `dns`:
```python
from .helpers import render_dns_records_table
if result.values:
render_dns_records_table(f"DNS {record_type} Records", result)
else:
console.print(f"[yellow]No {record_type} records found for {domain}[/yellow]")
```
Use in `dns_all`:
```python
from .helpers import render_dns_records_table
if results:
for r in results:
render_dns_records_table(f"{r.record_type} Records", r)
console.print()
else:
console.print(f"[yellow]No DNS records found for {domain}[/yellow]")
```
Similarly, for interfaces:
```python
# helpers.py
def render_interfaces_table(title: str, interfaces) -> Table:
table = Table(title=title)
table.add_column("Name", style="cyan")
table.add_column("IPv4 Address")
table.add_column("Netmask")
table.add_column("MAC Address", style="dim")
table.add_column("Status", justify="center")
table.add_column("Speed")
for iface in interfaces:
status = "[green]UP[/green]" if iface.is_up else "[red]DOWN[/red]"
speed = f"{iface.speed_mbps} Mbps" if iface.speed_mbps else "-"
table.add_row(
iface.name,
iface.ipv4_address or "-",
iface.ipv4_netmask or "-",
iface.mac_address or "-",
status,
speed,
)
return table
```
Use in `list_interfaces`:
```python
from .helpers import render_interfaces_table
interfaces = get_local_interfaces(include_loopback=all_interfaces)
table = render_interfaces_table("Network Interfaces", interfaces)
console.print(table)
```
---
### 3. Move generic formatting helpers out of the CLI module
`_format_bytes` is generic and used by SNMP commands. Pull it into a util module that can be reused and tested independently.
```python
# formatting.py (new module)
def format_bytes(num_bytes: int) -> str:
for unit in ["B", "KB", "MB", "GB", "TB"]:
if abs(num_bytes) < 1024.0:
return f"{num_bytes:.1f} {unit}"
num_bytes /= 1024.0
return f"{num_bytes:.1f} PB"
```
Use in `snmp_interfaces`:
```python
from .formatting import format_bytes
in_bytes = format_bytes(iface.in_octets) if iface.in_octets else "-"
out_bytes = format_bytes(iface.out_octets) if iface.out_octets else "-"
```
---
### 4. Isolate pure “model → table” mapping from CLI plumbing
Each command currently does: parse args → call network_tools → build table row-by-row. You can keep this file as the Typer entrypoint and move the “model → Table” logic into small pure functions or submodules without affecting behavior.
Example with `scan`:
```python
# cli_ports.py
from rich.table import Table
from .helpers import format_port_status
def build_port_scan_table(host: str, results, banner: bool) -> Table:
table = Table(title=f"Port Scan Results: {host}")
table.add_column("Port", style="cyan", justify="right")
table.add_column("Status", justify="center")
table.add_column("Service", style="dim")
table.add_column("Latency", justify="right")
if banner:
table.add_column("Banner")
open_count = 0
for r in results:
status, latency = format_port_status(r.is_open, r.latency_ms)
if r.is_open:
open_count += 1
row = [str(r.port), status, r.service_name or "-", latency]
if banner:
row.append(r.banner[:50] if r.banner else "-")
table.add_row(*row)
table.open_count = open_count # or return (table, open_count)
return table
```
Use in the Typer command:
```python
from .cli_ports import build_port_scan_table
results = scan_ports(host, ports, timeout=timeout, grab_banner=banner)
table = build_port_scan_table(host, results, banner=banner)
console.print(table)
console.print(f"\n[bold]Summary:[/bold] {table.open_count} open, {len(results) - table.open_count} closed")
```
---
By extracting these kinds of helpers gradually (starting with status formatting and DNS/port/interface table builders), you can significantly reduce duplication and cognitive load in the CLI file without changing any behavior or features.
</issue_to_address>
### Comment 10
<location> `tests/test_dns_lookup.py:102-103` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 11
<location> `tests/test_network_info.py:109-114` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid conditionals in tests. ([`no-conditionals-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-conditionals-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like conditionals, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 12
<location> `tests/test_network_info.py:121` </location>
<code_context>
listen_count_all = sum(1 for c in all_conn if c.status == "LISTEN")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Simplify constant sum() call ([`simplify-constant-sum`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/simplify-constant-sum))
```suggestion
listen_count_all = sum(bool(c.status == "LISTEN")
```
<br/><details><summary>Explanation</summary>As `sum` add the values it treats `True` as `1`, and `False` as `0`. We make use
of this fact to simplify the generator expression inside the `sum` call.
</details>
</issue_to_address>
### Comment 13
<location> `tests/test_network_info.py:122` </location>
<code_context>
listen_count_filtered = sum(1 for c in no_listen if c.status == "LISTEN")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Simplify constant sum() call ([`simplify-constant-sum`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/simplify-constant-sum))
```suggestion
listen_count_filtered = sum(bool(c.status == "LISTEN")
```
<br/><details><summary>Explanation</summary>As `sum` add the values it treats `True` as `1`, and `False` as `0`. We make use
of this fact to simplify the generator expression inside the `sum` call.
</details>
</issue_to_address>
### Comment 14
<location> `tests/test_network_info.py:135-136` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 15
<location> `tests/test_ping_monitor.py:79-81` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid loops in tests. ([`no-loop-in-tests`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/Python/Default-Rules/no-loop-in-tests))
<details><summary>Explanation</summary>Avoid complex code, like loops, in test functions.
Google's software engineering guidelines says:
"Clear tests are trivially correct upon inspection"
To reach that avoid complex code in tests:
* loops
* conditionals
Some ways to fix this:
* Use parametrized tests to get rid of the loop.
* Move the complex logic into helpers.
* Move the complex part into pytest fixtures.
> Complexity is most often introduced in the form of logic. Logic is defined via the imperative parts of programming languages such as operators, loops, and conditionals. When a piece of code contains logic, you need to do a bit of mental computation to determine its result instead of just reading it off of the screen. It doesn't take much logic to make a test more difficult to reason about.
Software Engineering at Google / [Don't Put Logic in Tests](https://abseil.io/resources/swe-book/html/ch12.html#donapostrophet_put_logic_in_tests)
</details>
</issue_to_address>
### Comment 16
<location> `network_health_checker/cli.py:291-293` </location>
<code_context>
@app.command("dns-all")
def dns_all(
domain: str = typer.Argument(..., help="Lekérdezendő domain"),
):
"""
Összes DNS rekord típus lekérdezése.
Példa:
netcheck dns-all google.com
"""
console.print(f"\n[bold]Full DNS lookup: {domain}[/bold]\n")
results = lookup_all_records(domain)
if results:
for result in results:
table = Table(title=f"{result.record_type} Records")
table.add_column("Value", style="cyan")
table.add_column("TTL", justify="right", style="dim")
for value in result.values:
table.add_row(value, str(result.ttl) if result.ttl else "-")
console.print(table)
console.print()
else:
console.print(f"[yellow]No DNS records found for {domain}[/yellow]")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if results := lookup_all_records(domain):
```
</issue_to_address>
### Comment 17
<location> `network_health_checker/cli.py:340-342` </location>
<code_context>
@app.command("mx")
def mx_records(
domain: str = typer.Argument(..., help="Domain a mail szerverek lekérdezéséhez"),
):
"""
Mail szerverek (MX rekordok) lekérdezése.
Példa:
netcheck mx gmail.com
"""
console.print(f"\n[bold]Mail servers for {domain}[/bold]\n")
records = get_mx_records(domain)
if records:
table = Table(title="MX Records")
table.add_column("Priority", justify="right", style="cyan")
table.add_column("Mail Server")
for record in records:
parts = record.split(" ", 1)
if len(parts) == 2:
table.add_row(parts[0], parts[1])
else:
table.add_row("-", record)
console.print(table)
else:
console.print(f"[yellow]No MX records found for {domain}[/yellow]")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if records := get_mx_records(domain):
```
</issue_to_address>
### Comment 18
<location> `network_health_checker/cli.py:371-373` </location>
<code_context>
@app.command("ns")
def ns_records(
domain: str = typer.Argument(..., help="Domain a nameserverek lekérdezéséhez"),
):
"""
Authoritative nameserverek lekérdezése.
Példa:
netcheck ns google.com
"""
console.print(f"\n[bold]Nameservers for {domain}[/bold]\n")
records = get_nameservers(domain)
if records:
for ns in records:
console.print(f" • {ns}")
else:
console.print(f"[yellow]No NS records found for {domain}[/yellow]")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if records := get_nameservers(domain):
```
</issue_to_address>
### Comment 19
<location> `network_health_checker/cli.py:599-602` </location>
<code_context>
@app.command("snmp-info")
def snmp_info(
host: str = typer.Argument(..., help="SNMP eszköz IP címe"),
community: str = typer.Option("public", "--community", "-c", help="SNMP community string"),
):
"""
SNMP eszköz információk lekérdezése.
Példa:
netcheck snmp-info 192.168.1.1
netcheck snmp-info 192.168.1.1 -c private
"""
console.print(f"\n[bold]SNMP System Info: {host}[/bold]\n")
# Async függvény futtatása
device = asyncio.run(get_system_info(host, community))
if device:
table = Table(title="Device Information", show_header=False)
table.add_column("Property", style="cyan")
table.add_column("Value")
table.add_row("Host", device.host)
table.add_row("System Name", device.sys_name or "-")
table.add_row("Description", device.sys_descr or "-")
table.add_row("Location", device.sys_location or "-")
table.add_row("Contact", device.sys_contact or "-")
if device.uptime_seconds:
days, remainder = divmod(device.uptime_seconds, 86400)
hours, remainder = divmod(remainder, 3600)
minutes, seconds = divmod(remainder, 60)
uptime_str = f"{days}d {hours}h {minutes}m {seconds}s"
table.add_row("Uptime", uptime_str)
console.print(table)
else:
console.print(f"[red]Could not connect to {host} via SNMP[/red]")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if device := asyncio.run(get_system_info(host, community)):
```
</issue_to_address>
### Comment 20
<location> `network_health_checker/cli.py:638-641` </location>
<code_context>
@app.command("snmp-interfaces")
def snmp_interfaces(
host: str = typer.Argument(..., help="SNMP eszköz IP címe"),
community: str = typer.Option("public", "--community", "-c", help="SNMP community string"),
):
"""
SNMP eszköz interfészeinek listázása.
Példa:
netcheck snmp-interfaces 192.168.1.1
"""
console.print(f"\n[bold]SNMP Interfaces: {host}[/bold]\n")
# Async függvény futtatása
interfaces = asyncio.run(get_interfaces(host, community))
if interfaces:
table = Table(title="Network Interfaces")
table.add_column("Index", justify="right", style="dim")
table.add_column("Name", style="cyan")
table.add_column("Status", justify="center")
table.add_column("Speed")
table.add_column("In", justify="right")
table.add_column("Out", justify="right")
for iface in interfaces:
status = "[green]UP[/green]" if iface.oper_status == "up" else "[red]DOWN[/red]"
speed = f"{iface.speed // 1000000} Mbps" if iface.speed else "-"
# Byte to human readable
in_bytes = _format_bytes(iface.in_octets) if iface.in_octets else "-"
out_bytes = _format_bytes(iface.out_octets) if iface.out_octets else "-"
table.add_row(
str(iface.index),
iface.name or "-",
status,
speed,
in_bytes,
out_bytes,
)
console.print(table)
else:
console.print(f"[red]Could not get interfaces from {host}[/red]")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if interfaces := asyncio.run(get_interfaces(host, community)):
```
</issue_to_address>
### Comment 21
<location> `network_health_checker/network_tools/network_info.py:114-117` </location>
<code_context>
def get_interface_by_name(name: str) -> Optional[NetworkInterface]:
"""
Specifikus interfész információinak lekérdezése név alapján.
Args:
name: Interfész neve (pl. "eth0", "enp0s3")
Returns:
NetworkInterface vagy None ha nem létezik
Example:
>>> eth0 = get_interface_by_name("eth0")
>>> if eth0:
... print(f"IP: {eth0.ipv4_address}")
"""
interfaces = get_local_interfaces(include_loopback=True)
for iface in interfaces:
if iface.name == name:
return iface
return None
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use the built-in function `next` instead of a for-loop ([`use-next`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-next/))
```suggestion
return next((iface for iface in interfaces if iface.name == name), None)
```
</issue_to_address>
### Comment 22
<location> `network_health_checker/network_tools/network_info.py:146-148` </location>
<code_context>
def get_default_gateway() -> Optional[str]:
"""
Alapértelmezett gateway IP címének lekérdezése.
Returns:
Gateway IP címe vagy None ha nem található
Example:
>>> gw = get_default_gateway()
>>> print(f"Default gateway: {gw}")
"""
# psutil nem ad direkt gateway infót, gateways() használata
try:
gateways = psutil.net_if_addrs()
# Alternatív: netifaces library vagy socket route info
# Itt egyszerűsített megoldás: próbáljuk kitalálni az első
# nem-loopback interfész hálózatából
for name, addrs in gateways.items():
if name.lower() in ("lo", "loopback", "lo0"):
continue
for addr in addrs:
if addr.family == socket.AF_INET and addr.address:
# Tipikus gateway: x.x.x.1 vagy x.x.x.254
parts = addr.address.split(".")
if len(parts) == 4:
# Próbáljuk meg a .1-es címet
gateway = f"{parts[0]}.{parts[1]}.{parts[2]}.1"
return gateway
except Exception:
pass
return None
</code_context>
<issue_to_address>
**suggestion (code-quality):** Inline variable that is immediately returned ([`inline-immediately-returned-variable`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/inline-immediately-returned-variable/))
```suggestion
return f"{parts[0]}.{parts[1]}.{parts[2]}.1"
```
</issue_to_address>
### Comment 23
<location> `network_health_checker/network_tools/subnet_calculator.py:41` </location>
<code_context>
def calculate_subnet(cidr: str) -> SubnetInfo:
"""
Alhálózat információk kiszámítása CIDR notation alapján.
Args:
cidr: CIDR formátumú hálózati cím (pl. "192.168.1.0/24")
Returns:
SubnetInfo objektum a számított értékekkel
Raises:
ValueError: Ha a CIDR formátum érvénytelen
Example:
>>> info = calculate_subnet("10.0.0.0/8")
>>> print(f"Broadcast: {info.broadcast}")
>>> print(f"Usable hosts: {info.total_hosts}")
"""
try:
# IPv4Network objektum létrehozása
network = ipaddress.IPv4Network(cidr, strict=False)
except ValueError as e:
raise ValueError(f"Invalid CIDR notation: {cidr} - {e}")
# Hostok számának kiszámítása
# A hálózati és broadcast cím nem használható host-ként
total_hosts = network.num_addresses - 2 if network.prefixlen < 31 else network.num_addresses
# Első és utolsó host meghatározása
hosts = list(network.hosts())
first_host = str(hosts[0]) if hosts else str(network.network_address)
last_host = str(hosts[-1]) if hosts else str(network.broadcast_address)
return SubnetInfo(
network=str(network.network_address),
netmask=str(network.netmask),
broadcast=str(network.broadcast_address),
first_host=first_host,
last_host=last_host,
total_hosts=max(0, total_hosts),
cidr=network.prefixlen,
)
</code_context>
<issue_to_address>
**suggestion (code-quality):** Explicitly raise from a previous error ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
```suggestion
raise ValueError(f"Invalid CIDR notation: {cidr} - {e}") from e
```
</issue_to_address>
### Comment 24
<location> `network_health_checker/network_tools/subnet_calculator.py:166-167` </location>
<code_context>
def netmask_to_cidr(netmask: str) -> int:
"""
Alhálózati maszk konvertálása CIDR prefix hosszra.
Args:
netmask: Alhálózati maszk (pl. "255.255.255.0")
Returns:
CIDR prefix hossz (pl. 24)
Example:
>>> netmask_to_cidr("255.255.255.0")
24
>>> netmask_to_cidr("255.255.0.0")
16
"""
try:
# IP cím objektumként értelmezve a maszk
mask = ipaddress.IPv4Address(netmask)
# Bináris reprezentáció 1-eseinek számlálása
binary = bin(int(mask))
return binary.count("1")
except ValueError:
raise ValueError(f"Invalid netmask: {netmask}")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Explicitly raise from a previous error ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
```suggestion
except ValueError as e:
raise ValueError(f"Invalid netmask: {netmask}") from e
```
</issue_to_address>
### Comment 25
<location> `network_health_checker/network_tools/subnet_calculator.py:220` </location>
<code_context>
def split_subnet(cidr: str, new_prefix: int) -> List[SubnetInfo]:
"""
Alhálózat felosztása kisebb alhálózatokra.
Args:
cidr: Eredeti alhálózat CIDR formátumban
new_prefix: Új prefix hossz (nagyobb mint az eredeti)
Returns:
Felosztott alhálózatok listája
Example:
>>> subnets = split_subnet("192.168.0.0/24", 26)
>>> for s in subnets:
... print(f"{s.network}/{s.cidr}: {s.total_hosts} hosts")
"""
try:
network = ipaddress.IPv4Network(cidr, strict=False)
if new_prefix <= network.prefixlen:
raise ValueError(f"New prefix must be larger than {network.prefixlen}")
subnets = list(network.subnets(new_prefix=new_prefix))
return [calculate_subnet(str(s)) for s in subnets]
except ValueError as e:
raise ValueError(f"Cannot split subnet: {e}")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Explicitly raise from a previous error ([`raise-from-previous-error`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/raise-from-previous-error/))
```suggestion
raise ValueError(f"Cannot split subnet: {e}") from e
```
</issue_to_address>
### Comment 26
<location> `tests/test_network_info.py:107-109` </location>
<code_context>
def test_connection_has_required_fields(self, mock_psutil_connections):
"""Kapcsolat objektumoknak megvannak a szükséges mezői."""
connections = get_active_connections()
if connections:
conn = connections[0]
assert hasattr(conn, "protocol")
assert hasattr(conn, "local_address")
assert hasattr(conn, "local_port")
assert hasattr(conn, "status")
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
```suggestion
if connections := get_active_connections():
```
</issue_to_address>
### Comment 27
<location> `tests/test_network_info.py:320-322` </location>
<code_context>
@pytest.fixture
def mock_psutil_io_counters():
"""Mock psutil I/O számláló lekérdezéshez."""
with patch("network_health_checker.network_tools.network_info.psutil.net_io_counters") as mock_io:
mock_counters = MagicMock()
mock_counters.bytes_sent = 1000000
mock_counters.bytes_recv = 2000000
mock_counters.packets_sent = 1000
mock_counters.packets_recv = 2000
mock_counters.errin = 0
mock_counters.errout = 0
mock_counters.dropin = 0
mock_counters.dropout = 0
def side_effect(pernic=False):
if pernic:
return {"eth0": mock_counters}
return mock_counters
mock_io.side_effect = side_effect
yield mock_io
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return {"eth0": mock_counters} if pernic else mock_counters
```
</issue_to_address>
### Comment 28
<location> `tests/test_ping_monitor.py:162-164` </location>
<code_context>
@pytest.fixture
def mock_ping_mixed():
"""Mock vegyes eredményekhez."""
with patch("network_health_checker.network_tools.ping_monitor.ping") as mock_ping:
def side_effect(host, **kwargs):
if host == "8.8.8.8":
return 10.0
return None # timeout
mock_ping.side_effect = side_effect
yield mock_ping
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return 10.0 if host == "8.8.8.8" else None
```
</issue_to_address>
### Comment 29
<location> `tests/test_snmp_query.py:64-66` </location>
<code_context>
async def test_returns_none_on_timeout(self):
"""Timeout esetén None visszaadása."""
# Rövid timeout nem létező host-ra
result = await snmp_get("192.168.254.254", "1.3.6.1.2.1.1.1.0", timeout=0.1)
# Valós hálózat nélkül valószínűleg None
# Reason: Ez integrációs teszt lenne, mockkolni kellene
pass
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove redundant pass statement ([`remove-redundant-pass`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-pass/))
```suggestion
```
</issue_to_address>
### Comment 30
<location> `tests/test_snmp_query.py:90-92` </location>
<code_context>
async def test_returns_none_on_unreachable(self):
"""Nem elérhető host esetén None."""
result = await get_system_info("192.168.254.254")
# Valószínűleg timeout vagy None
# Reason: Integrációs teszt lenne
pass
</code_context>
<issue_to_address>
**suggestion (code-quality):** Remove redundant pass statement ([`remove-redundant-pass`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-redundant-pass/))
```suggestion
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+47
to
+48
| # Első és utolsó host meghatározása | ||
| hosts = list(network.hosts()) |
There was a problem hiding this comment.
issue (performance): Materializing all hosts into a list can blow up memory for large subnets.
For networks like /16 this can create millions of IPv4Address objects just to derive first/last. Instead, get the first host via next(network.hosts(), None) and compute the last host from network.broadcast_address/network.network_address (or by iterating to the last element without materializing the whole sequence) so you avoid building the full list in memory.
Comment on lines
+39
to
+48
| network = ipaddress.IPv4Network(cidr, strict=False) | ||
| except ValueError as e: | ||
| raise ValueError(f"Invalid CIDR notation: {cidr} - {e}") | ||
|
|
||
| # Hostok számának kiszámítása | ||
| # A hálózati és broadcast cím nem használható host-ként | ||
| total_hosts = network.num_addresses - 2 if network.prefixlen < 31 else network.num_addresses | ||
|
|
||
| # Első és utolsó host meghatározása | ||
| hosts = list(network.hosts()) |
There was a problem hiding this comment.
issue (performance): get_subnet_hosts also eagerly allocates all hosts, which is risky for big CIDR ranges.
Because list(network.hosts()) materializes every host, the limit parameter and docstring warning don’t prevent high memory usage on large networks. Consider iterating over network.hosts() and collecting up to limit items (or until exhaustion) instead of building the full list.
Comment on lines
+226
to
+229
| if "-" in part: | ||
| # Port tartomány | ||
| start, end = part.split("-", 1) | ||
| port_list.extend(range(int(start), int(end) + 1)) |
There was a problem hiding this comment.
issue (bug_risk): Port string parsing lacks validation and can raise ValueError on bad input or invalid ranges.
In _parse_ports, cases like empty parts, non-numeric values (e.g. 80-abc), or reversed ranges (e.g. 100-1) will cause int(...)/range(...) to raise and crash the CLI. Please validate each segment before converting, handle conversion errors, enforce valid port bounds (1–65535), and surface a clear user-facing error instead of letting the exception propagate.
Comment on lines
+133
to
+142
| gateways = psutil.net_if_addrs() | ||
| # Alternatív: netifaces library vagy socket route info | ||
| # Itt egyszerűsített megoldás: próbáljuk kitalálni az első | ||
| # nem-loopback interfész hálózatából | ||
|
|
||
| for name, addrs in gateways.items(): | ||
| if name.lower() in ("lo", "loopback", "lo0"): | ||
| continue | ||
| for addr in addrs: | ||
| if addr.family == socket.AF_INET and addr.address: |
There was a problem hiding this comment.
suggestion (bug_risk): Default gateway detection is heuristic and can be misleading on many setups.
Guessing the gateway as x.x.x.1 from psutil.net_if_addrs() will fail on many real networks (.254, multiple routers, PPP, etc.). Since psutil doesn’t expose gateways, either augment this with OS-specific routing inspection, or clearly mark this as a best-effort heuristic (e.g. different name/return type) so callers don’t treat it as authoritative.
| host: str, | ||
| timeout: float | None = None, | ||
| count: int = 1, | ||
| privileged: bool = False, |
There was a problem hiding this comment.
issue: The privileged parameter is unused, which may confuse API consumers.
Since privileged is neither passed to ping3.ping nor used to change behavior, it currently has no effect. Please either connect it to the implementation (e.g., pass privileged through or branch on its value) or remove it to avoid implying unsupported behavior.
Comment on lines
+107
to
+109
| connections = get_active_connections() | ||
|
|
||
| if connections: |
There was a problem hiding this comment.
suggestion (code-quality): Use named expression to simplify assignment and conditional (use-named-expression)
Suggested change
| connections = get_active_connections() | |
| if connections: | |
| if connections := get_active_connections(): |
Comment on lines
+320
to
+322
| if pernic: | ||
| return {"eth0": mock_counters} | ||
| return mock_counters |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| if pernic: | |
| return {"eth0": mock_counters} | |
| return mock_counters | |
| return {"eth0": mock_counters} if pernic else mock_counters |
Comment on lines
+162
to
+164
| if host == "8.8.8.8": | ||
| return 10.0 | ||
| return None # timeout |
There was a problem hiding this comment.
suggestion (code-quality): We've found these issues:
- Lift code into else after jump in control flow (
reintroduce-else) - Replace if statement with if expression (
assign-if-exp)
Suggested change
| if host == "8.8.8.8": | |
| return 10.0 | |
| return None # timeout | |
| return 10.0 if host == "8.8.8.8" else None |
Comment on lines
+64
to
+66
| # Valós hálózat nélkül valószínűleg None | ||
| # Reason: Ez integrációs teszt lenne, mockkolni kellene | ||
| pass |
There was a problem hiding this comment.
suggestion (code-quality): Remove redundant pass statement (remove-redundant-pass)
Suggested change
| # Valós hálózat nélkül valószínűleg None | |
| # Reason: Ez integrációs teszt lenne, mockkolni kellene | |
| pass |
Comment on lines
+90
to
+92
| # Valószínűleg timeout vagy None | ||
| # Reason: Integrációs teszt lenne | ||
| pass |
There was a problem hiding this comment.
suggestion (code-quality): Remove redundant pass statement (remove-redundant-pass)
Suggested change
| # Valószínűleg timeout vagy None | |
| # Reason: Integrációs teszt lenne | |
| pass |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
3-network-health-checkertonetwork_health_checker(valid Python package name)ping_monitor: ICMP ping with batch processing using ProcessPoolExecutorport_scanner: TCP port scanning with ThreadPoolExecutor and banner grabbingdns_lookup: DNS queries for all record types (A, AAAA, MX, TXT, CNAME, NS, SOA, PTR, SRV)subnet_calculator: IP/subnet calculations with CIDR supportsnmp_query: SNMP v2c device queries using pysnmp (async)network_info: Local network interface and connection info using psutilTest plan
🤖 Generated with Claude Code
Summary by Sourcery
Introduce a new
network_health_checkerPython package providing a full-featured network diagnostics toolkit with CLI, configuration, models, and tooling, replacing the previous3-network-health-checkerpackage.New Features:
python -m network_health_checkerentry point.Enhancements:
Tests:
Chores:
3-network-health-checkerpackage in favor of the newnetwork_health_checkerpackage structure.Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.