Skip to content

Type every Proxmox response, check the tree with ty, release 3.0.0 - #17

Merged
MorningLightMountain713 merged 1 commit into
chore/contributing-and-toolingfrom
chore/ty-clean
Oct 23, 2026
Merged

MorningLightMountain713 merged 1 commit into
chore/contributing-and-toolingfrom
chore/ty-clean

Conversation

@MorningLightMountain713

Copy link
Copy Markdown
Collaborator

Stacked on #16, which sets up the tooling and contributing guide this builds on.

Summary

ApiResponse[T] is ApiSuccess[T] | ApiFailure.

  • ApiSuccess always carries its payload, typed per endpoint. For example, get_vms returns ApiResponse[list[VmSummary]].
  • ApiFailure carries status, error and timed_out.
  • if not res: narrows to ApiFailure, so past that check res.payload has the endpoint's type with no further checks:
res = await api.get_vms(node)
if not res:
    return False, res.error        # ApiFailure
for vm in res.payload: ...         # list[VmSummary]

Every answer is validated at the boundary.

  • Each endpoint's data is validated with Pydantic against a TypedDict shape: VmSummary, NetworkIface, NodeStorage, StorageVolume, NodeVersion, TaskStatus, HypervisorNode and StorageConfig, all exported.
  • An answer missing a required field, or carrying data: null where a value is due, becomes an ApiFailure naming the field (Unexpected response at 0.iface: Field required).
  • Fields the shapes don't name are kept, so the Ansible status module still returns the full VM records.

Faults the typing exposed, fixed here:

  • The TUI's add-hypervisor check reported "Validated" when the connection timed out.
  • arcane-mage ping could never report "Authentication failed"; the check was unreachable.
  • Discovery counted a node whose VMs couldn't be listed as running none. It now leaves that node out.
  • download_iso sent a Python bool for verify-certificates. It now sends 0/1, and the Ansible iso_info module uses it instead of posting to a private method.

ty is clean across src, tests and the Ansible collection.

  • ansible-core is in its own ansible dependency group, synced by default; the collection only needs it for type checking.
  • pyrage-stubs is added.
  • aioresponses is removed: it can't build a response under the current aiohttp, and nothing used it. The new response tests run against aiohttp's own test server.
  • CONTRIBUTING.md now requires uv run ty check.

Breaking

Released as 3.0.0. Code that only reads responses (if not res, then .payload, .status, .error) keeps working. Code that constructs ApiResponse(...) now constructs ApiSuccess(payload=...) or ApiFailure(...). Open PRs #12 and #15 touch proxmox.py and provisioner.py, so they will need a rebase.

Checks

  • uv run ruff check / ruff format --check: clean
  • uv run ty check: clean
  • uv run pytest: 229 passed
  • uv build: 3.0.0 wheel built; arcane-mage --help runs

🤖 Generated with Claude Code

ApiResponse[T] is ApiSuccess[T] | ApiFailure. A success always carries its
payload, validated with Pydantic against the endpoint's shape; a failure
carries status, error and timed_out, and `if not res:` narrows to it. An
answer missing a field arcane-mage relies on is a failure naming that field,
so no caller reads a null or partial payload as data. Fields the shapes do
not name are kept, which the Ansible status module returns as Proxmox sent
them.

Reading the payloads typed exposed four faults, fixed here: the TUI accepted
a hypervisor whose connection timed out, `ping` could never report a rejected
token, discovery counted a node whose VMs could not be listed as running none,
and download_iso sent a Python bool for verify-certificates.

ty checks src, tests and the Ansible collection, which needs ansible-core
(its own dependency group) and pyrage-stubs. aioresponses is dropped: it
cannot build a response under the current aiohttp, and the response tests
run against aiohttp's own test server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MorningLightMountain713
MorningLightMountain713 merged commit bc56c80 into master Oct 23, 2026
1 check passed
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.

1 participant