Skip to content

Appliance session left duplicated wait loops and spool readers #871

Description

@VijitSingh97

Two clusters of copy-paste from the develop-v2 appliance work. Neither is a bug; both are the kind of duplication that makes the next edit land in four places and get missed in a fifth.

1. tests/os/run.sh — seven DHCP-lease waits and five wizard-page waits

The same five-line "poll until the guest has an IPv4 lease" loop appears seven times, at lines 284, 331, 533, 651, 751, 897, 981:

ip=$(virsh domifaddr "$VM" 2>/dev/null | awk '/ipv4/{print $4}' | cut -d/ -f1 | head -1)

each wrapped in its own tries/tries2 counter with a different limit. And the "poll until the setup page answers" loop appears five times, at 300, 585, 693, 926, 1052:

while ! curl -fsSk -m 5 "https://$ip/" 2>/dev/null | grep -qi "Pithead setup"; do

The file already has the right pattern for both — _wait_ssh and _dash_marker_served take a deadline and return a status. Two more helpers in that style (_wait_dhcp_ip, _wait_setup_page) collapse roughly 45 lines and, more usefully, put the timeout in one place. Today the twelve call sites disagree about how long to wait, and the ad-hoc tries vs tries2 naming is there only to dodge shadowing in the longer functions.

Worth doing when someone is next in this file for another reason. It does need a bench run to verify, since nothing below tier 4 exercises this harness.

2. build/dashboard/mining_dashboard/wizard.py — three identical spool-JSON readers

_reference() (76-83), _last_attempt() (116-121) and _rig_defaults() (124-133) are the same function three times: read a spool file, json.loads it, swallow ValueError, return {} on anything unexpected. One helper covers all three:

def _spool_json(name: str) -> dict:
    try:
        d = json.loads(_spool_read(name) or "{}")
    except ValueError:
        d = {}
    return d if isinstance(d, dict) else {}

_reference keeps its _-prefix filter on top; the other two become one-liners. About 14 lines.

Separately, _spool_write_config (310-318) re-implements _spool_write_text's (300-307) mkstemp/os.replace atomic write instead of calling it — it is _spool_clear_error() plus _spool_write_text("config.json", json.dumps(cfg, indent=2)). About 8 lines, and it removes the chance of the two atomic writes drifting apart.

test_wizard.py covers all of these paths (68 tests), so this one is verifiable at tier 1 with no bench.

Found during the develop-v2 quality pass (PR #867), which deliberately shipped no refactors.

Metadata

Metadata

Assignees

No one assigned

    Labels

    appliancePithead OS appliance work — lands on feat/phase2-bakery-imagerefactorCode health / tech debt; no behavior change

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions