python-code-style

name	python-code-style
description	Python code conventions. Functions return errors, .env parsing strips quotes, type hints required.

Python Code Style

Rules

1. Functions NEVER call sys.exit(). Return None, raise exception, or return error code.
2. .env parsing MUST strip surrounding quotes from values.
3. Every function MUST have type hints.
4. Use Path from pathlib for file operations.
5. Logging: % formatting for logs, f-strings for non-logging.

Patterns

Function error handling

def find_ansible_playbook(playbook: str) -> str:
    if not found:
        raise FileNotFoundError(f"Playbook not found: {playbook}")
    return path

def get_cached_address() -> str | None:
    path = Path('.state/addresses.json')
    if not path.exists():
        return None
    return json.loads(path.read_text()).get('PRIMARY_HOST')

def probe_host(host: str, port: int, timeout: int = 5) -> bool:
    try:
        with socket.create_connection((host, port), timeout):
            return True
    except (socket.timeout, ConnectionRefusedError):
        return False

.env quote stripping

value = value.strip()
if len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"):
    value = value[1:-1]
env[key.strip()] = value

Path handling

def ensure_file_exists(path: Path) -> Path:
    path = path.expanduser().resolve()
    if not path.is_file():
        raise FileNotFoundError(f"Not a file: {path}")
    return path

def ensure_dir(path: Path) -> Path:
    path.mkdir(parents=True, exist_ok=True)
    return path

Dictionary merging

def merge_dicts(*dicts: dict) -> dict:
    result = {}
    for d in dicts:
        result.update(d)
    return result

config = merge_dicts(default_config, env_config, cli_config)

Logging

import logging
log = logging.getLogger(__name__)

log.debug("Probing host %s", host)  # Use % formatting
msg = f"Built command for {playbook}"  # OK for non-logging

Domain model classes (MVC pattern)

ALWAYS separate business logic from UI rendering using domain classes in the data layer. Domain objects own derived state — the UI only reads properties.

# Domain class in data.py — owns identity, deploy history, AND live telemetry
class Host:
    def __init__(self, name: str, ip: str, *, is_lan: bool = False) -> None:
        self.name = name
        self.ip = ip
        self.is_lan = is_lan
        self.deploys: list[DeployRecord] = []
        self.telemetry: HostTelemetry | None = None

    def attach_telemetry(self, t: HostTelemetry) -> None:
        self.telemetry = t
        self._guests = _parse_guests(t.services)

    @property
    def healthy(self) -> bool: ...   # from deploy history
    @property
    def errors(self) -> list[str]: ...  # deploy + offline errors
    @property
    def online(self) -> bool: ...    # from telemetry (False if None)
    @property
    def disk_pct(self) -> : ... 

     () -> [GuestInfo]: ...  


 :
     () -> :
        .hosts = hosts


     () -> : ...

     () -> : ...

     () -> : ...

     () -> : ...  

     () -> Host | : ...

     () -> Host | : ...


 () -> Fleet:
    hosts = [Host(name=i.name, ip=i.ip, ...)  i  get_known_hosts(env)]
     record  load_deploy_history(state_dir):
         host  hosts:
             _deploy_targets_host(record, host.name):
                host.deploys.append(record)
     node  load_node_registry(state_dir):
        host = ...  
         host:
            host.attach_telemetry(HostTelemetry(...))
     Fleet(hosts)


 () -> :
     fleet.healthy:
        ui.badge(, color=)
    :
         err  fleet.errors:
            ui.label(err)

Rules:

• NEVER compute health/status in page modules. Use .healthy / .errors properties.
• ALWAYS put exit code mapping, status computation, and error descriptions in data.py.
• ALWAYS expose .healthy -> bool and .errors -> list[str] on domain objects.
• ALWAYS use factory functions to wire raw data sources into domain objects.
• ALWAYS return graceful defaults when telemetry is None (0.0, [], False, "--").
• NEVER read RegisteredNode directly in page modules — use Host/Fleet.
• Test domain logic with plain pytest — no NiceGUI required.

UI string constants (NiceGUI web UI)

# ALWAYS centralize UI strings as class constants in data.py
class Labels:
    START_DEPLOY = "Start Deploy"  # used by both page module AND tests

# Page module:
ui.button(Labels.START_DEPLOY, ...)

# Test:
user.find(Labels.START_DEPLOY).click()

# NEVER duplicate strings across page + test:
# BAD: ui.button("Start Deploy") + user.find("Start Deploy")
# GOOD: both reference Labels.START_DEPLOY

Previous bugs

• sys.exit() in functions → untestable, breaks composition
• Missing quote stripping → SSH fails with literal "192.168.1.100"
• Missing type hints → MyPy errors
• Broad except Exception in UI code caught too broadly. Use specific exceptions.
• format_last_seen_relative crashed on timezone-aware timestamps. Fix: .replace(tzinfo=None).
• 200+ magic strings duplicated between page modules and 6 test files. Changing a label required updating tests in multiple files. Fix: centralized into data.py constants (Routes, PageTitles, Labels, ApiRoutes).
• Health computation (exit code mapping, deploy history analysis) was embedded in dashboard.py UI functions. Couldn't test health logic without NiceGUI. Fix: extracted Host/Fleet domain classes into data.py with .healthy/.errors properties. Dashboard became a thin renderer. 29 pure-Python tests cover all health scenarios.
• Redundant "Host" forward reference in kickstart_callhome(host: "Host") when from __future__ import annotations is already at the top. With postponed annotations, all annotations are strings by default — quoting is unnecessary.
• kickstart_callhome returned success=True even with partial container restart failures (errors collected in errors list). This is intentional — success means "SSH connected and discovered containers"; errors captures per-container issues. Document this semantic in the docstring when success has a nuanced meaning.
• HostRegistry.register() is the single write path to registry.json. NEVER duplicate upsert logic. Every caller (env seeding, manual form, TEST_UNITS) delegates to register(). Immutable-on-create fields (bucket, source, first_seen) are preserved on upsert. MAC match takes precedence over name match for identity resolution.
• NEVER register container/service heartbeats into HostRegistry. The registry is for PHYSICAL HOSTS only (Proxmox nodes). Container heartbeats go into nodes.json (telemetry) via register_checkin(). build_fleet() groups container telemetry under parent hosts — containers never appear as independent fleet members. Previous bug: called for every heartbeat. Containers (pihole, wireguard, netdata, etc.) appeared as independent hosts on the dashboard with LAN IPs (10.10.10.x) instead of being grouped under their physical host. Fixed by removing the call from .