diff --git a/.github/workflows/test-python-build.yaml b/.github/workflows/test-python-build.yaml index 9a161f0..f92211a 100644 --- a/.github/workflows/test-python-build.yaml +++ b/.github/workflows/test-python-build.yaml @@ -35,4 +35,4 @@ jobs: - name: Install run: pip install -e ".[dev]" - name: Test - run: pytest tests/ -v --tb=short 2>/dev/null || echo "No tests yet" + run: pytest tests/ -v --tb=short diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fc9856..a68a135 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,35 @@ Release tags use the `python-vX.Y.Z` prefix. ## [Unreleased] +## [0.1.3] - 2026-07-09 + +### Fixed +- **`spore.truffle` / `spore.spawn` now work as documented.** The top-level + quickstart (`import spore; spore.truffle.find(...)`) resolved `spore.truffle` + to the *module* (which has no `.find`), because the same-named submodule + shadowed the module-level `__getattr__` hook — and importing it is unavoidable + (the client does it internally). The implementation modules are renamed to + `spore._truffle` / `spore._spawn` (private), and `spore.truffle`/`spore.spawn` + are now robust lazy proxies to a default client's sub-clients. Public classes + are re-exported from the top level: `from spore import Client, SpawnClient, + TruffleClient, Instance, InstanceType, SpotPrice, QuotaInfo`. +- **`spore.spawn.launch()` no longer raises `TypeError`.** It constructed + `Instance(private_ip=…, availability_zone=…)` but those fields didn't exist on + the dataclass. Added `private_ip` / `availability_zone` to `Instance` (populated + by launch/status/list), and `launch()` now builds its `Instance` through the + same `_parse` path as the rest of the client. +- **`spore.truffle.find()` no longer returns zeroed memory, GPU memory, and AZs.** + The parser read mangled JSON keys (`memory_mi_b`, `gpu_memory_mi_b`, + `available_a_zs`) that the REST API never sends; it now reads the real keys + (`memory_mib`, `gpu_memory_mib`, `availability_zones`, `vcpus`, `gpus`), so + `memory_gib` / `gpu_memory_gib` / `available_azs` are populated correctly. + +### Added +- Test suite (`tests/test_sdk.py`) covering the quickstart entry points and the + parsers against the real REST API JSON keys. CI now runs it as a gate (it + previously swallowed a missing/failing suite with `|| echo "No tests yet"`), + so a broken SDK can no longer ship green. + ## [0.1.2] Baseline. Earlier history is in the @@ -16,5 +45,6 @@ Baseline. Earlier history is in the --- -[Unreleased]: https://github.com/spore-host/python-sdk/compare/python-v0.1.2...HEAD +[Unreleased]: https://github.com/spore-host/python-sdk/compare/python-v0.1.3...HEAD +[0.1.3]: https://github.com/spore-host/python-sdk/compare/python-v0.1.2...python-v0.1.3 [0.1.2]: https://github.com/spore-host/python-sdk/releases/tag/python-v0.1.2 diff --git a/pyproject.toml b/pyproject.toml index 51d859c..fc8f5ea 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "spore-host" -version = "0.1.2" +version = "0.1.3" description = "Python SDK for spore.host — ephemeral EC2 compute for researchers" readme = "README.md" requires-python = ">=3.9" diff --git a/spore/__init__.py b/spore/__init__.py index c82c5cc..cfc9b35 100644 --- a/spore/__init__.py +++ b/spore/__init__.py @@ -5,12 +5,23 @@ import spore results = spore.truffle.find("nvidia h100", region="us-east-1") instance = spore.spawn.launch("c8a.2xlarge", ttl="8h") + +Or with an explicit client: + from spore import Client + c = Client(region="us-east-1") + c.truffle.find("amd epyc genoa") """ +# Defer annotation evaluation so `_default: Client | None` (PEP 604 union) doesn't +# execute at import on Python 3.9 (requires-python >=3.9); without this it raises +# `TypeError: unsupported operand type(s) for |`. +from __future__ import annotations + from .client import Client -from . import truffle, spawn +from ._spawn import Instance, SpawnClient +from ._truffle import InstanceType, QuotaInfo, SpotPrice, TruffleClient -# Module-level convenience: a default client using ambient AWS credentials +# Module-level convenience: a default client using ambient AWS credentials. _default: Client | None = None @@ -21,15 +32,47 @@ def _get_default() -> Client: return _default -def __getattr__(name: str): - # Allow `spore.truffle.find(...)` and `spore.spawn.launch(...)` at module level - default = _get_default() - if name == "truffle": - return default.truffle - if name == "spawn": - return default.spawn - raise AttributeError(f"module 'spore' has no attribute {name!r}") +class _LazySubClient: + """Proxy for `spore.truffle` / `spore.spawn` that forwards to the default + client's sub-client, constructing the default `Client()` on first use (so + `import spore` has no credential-resolution side effect). + + The implementation modules are named ``_truffle`` / ``_spawn`` (private) so + these public proxy attributes never collide with a submodule. An earlier + version kept the modules named ``truffle``/``spawn`` and relied on a + module-level ``__getattr__``, but importing a same-named submodule (which + ``Client.__init__`` does) permanently rebinds ``spore.truffle`` to the module + and shadows the hook — so ``spore.truffle`` resolved to the method-less module + (bug #2). Private module names remove the collision entirely. + """ + + __slots__ = ("_attr",) + + def __init__(self, attr: str): + self._attr = attr + + def _target(self): + return getattr(_get_default(), self._attr) + + def __getattr__(self, name: str): + return getattr(self._target(), name) + + def __repr__(self) -> str: + return repr(self._target()) + +truffle = _LazySubClient("truffle") +spawn = _LazySubClient("spawn") -__version__ = "0.1.2" -__all__ = ["Client", "truffle", "spawn"] +__version__ = "0.1.3" +__all__ = [ + "Client", + "truffle", + "spawn", + "SpawnClient", + "TruffleClient", + "Instance", + "InstanceType", + "SpotPrice", + "QuotaInfo", +] diff --git a/spore/spawn.py b/spore/_spawn.py similarity index 94% rename from spore/spawn.py rename to spore/_spawn.py index 0e8fb2b..1caecfe 100644 --- a/spore/spawn.py +++ b/spore/_spawn.py @@ -3,7 +3,6 @@ from __future__ import annotations import time -import threading from dataclasses import dataclass, field from datetime import datetime from typing import Callable, List, Optional, TYPE_CHECKING @@ -22,6 +21,8 @@ class Instance: state: str region: str public_ip: str = "" + private_ip: str = "" + availability_zone: str = "" dns: str = "" launch_time: Optional[datetime] = None ttl: str = "" @@ -199,17 +200,12 @@ def launch( body["active_processes"] = ",".join(active_processes) data = self._c.post("/v1/instances", body) - inst = Instance( - instance_id=data.get("instance_id", ""), - name=data.get("name", name or ""), - instance_type=instance_type, - state=data.get("state", "pending"), - region=data.get("region", region or self._c._region), - public_ip=data.get("public_ip", ""), - private_ip=data.get("private_ip", ""), - availability_zone=data.get("availability_zone", ""), - ) - inst._client = self + # Build via _parse (single source of truth for API→Instance mapping). The + # launch response echoes name/public_ip/private_ip/availability_zone/state/ + # region but NOT instance_type, so fall back to the requested type. + data.setdefault("instance_type", instance_type) + data.setdefault("state", "pending") + inst = self._parse(data) if wait: inst.wait_running() @@ -252,7 +248,7 @@ def status(self, instance_id_or_name: str) -> Instance: def stop(self, instance_id_or_name: str, hibernate: bool = False) -> Instance: """Stop a running instance.""" action = "hibernate" if hibernate else "stop" - data = self._action(instance_id_or_name, action) + self._action(instance_id_or_name, action) return self.status(instance_id_or_name) def start(self, instance_id_or_name: str) -> Instance: @@ -299,6 +295,8 @@ def _parse(self, d: dict) -> Instance: state=d.get("state", ""), region=d.get("region", ""), public_ip=d.get("public_ip", ""), + private_ip=d.get("private_ip", ""), + availability_zone=d.get("availability_zone", ""), dns=d.get("dns", ""), launch_time=launch_time, ttl=d.get("ttl", ""), diff --git a/spore/truffle.py b/spore/_truffle.py similarity index 89% rename from spore/truffle.py rename to spore/_truffle.py index 3246974..b4dc8c6 100644 --- a/spore/truffle.py +++ b/spore/_truffle.py @@ -146,18 +146,21 @@ def quota( ) def _parse(self, r: dict) -> InstanceType: + # Keys match the REST API's truffleaws.InstanceTypeResult json tags: + # vcpus, memory_mib, gpus, gpu_memory_mib, availability_zones (MiB→GiB + # here). Earlier code guessed mangled keys (memory_mi_b, gpu_memory_mi_b, + # available_a_zs), so memory/GPU-memory/AZs were silently zeroed (#2). return InstanceType( instance_type=r.get("instance_type", ""), region=r.get("region", ""), - vcpus=int(r.get("v_cp_us", r.get("vcpus", 0))), - memory_gib=float(r.get("memory_mi_b", r.get("memory_gib", 0))) / 1024 - if r.get("memory_mi_b") else float(r.get("memory_gib", 0)), + vcpus=int(r.get("vcpus", 0)), + memory_gib=float(r.get("memory_mib", 0)) / 1024, architecture=r.get("architecture", ""), on_demand_price=float(r.get("on_demand_price", 0)), - gpus=int(r.get("gp_us", r.get("gpus", 0))), + gpus=int(r.get("gpus", 0)), gpu_model=r.get("gpu_model", ""), - gpu_memory_gib=float(r.get("gpu_memory_mi_b", 0)) / 1024, - available_azs=r.get("available_a_zs", r.get("available_azs", [])), + gpu_memory_gib=float(r.get("gpu_memory_mib", 0)) / 1024, + available_azs=r.get("availability_zones", []), ) # ── Notebook display ────────────────────────────────────────────────────── diff --git a/spore/client.py b/spore/client.py index a588ba4..f2dba13 100644 --- a/spore/client.py +++ b/spore/client.py @@ -40,8 +40,8 @@ def __init__( self._session: Optional[boto3.Session] = None # Sub-clients - from .truffle import TruffleClient - from .spawn import SpawnClient + from ._truffle import TruffleClient + from ._spawn import SpawnClient self.truffle = TruffleClient(self) self.spawn = SpawnClient(self) diff --git a/tests/test_sdk.py b/tests/test_sdk.py new file mode 100644 index 0000000..b394563 --- /dev/null +++ b/tests/test_sdk.py @@ -0,0 +1,177 @@ +"""Tests for the spore SDK — cover exactly the paths the #2 audit found broken. + +Everything mocks at the Client.get/post boundary (no network, no AWS). The canned +JSON uses the REAL REST API keys (lambda/rest-api/instances.go, search.go, and +truffle's InstanceTypeResult json tags), so a future key drift fails here. +""" + +from __future__ import annotations + +import spore +from spore import Client, Instance, SpawnClient, TruffleClient + + +class FakeClient: + """A Client stand-in that returns canned responses instead of HTTP calls.""" + + def __init__(self, get_return=None, post_return=None): + self._get_return = get_return or {} + self._post_return = post_return or {} + self._region = "us-east-1" + self.get_calls = [] + self.post_calls = [] + + def get(self, path, params=None): + self.get_calls.append((path, params)) + return self._get_return + + def post(self, path, body=None): + self.post_calls.append((path, body)) + return self._post_return + + +# ── Fix #1: module shadowing — the documented quickstart must work ────────── + +def test_top_level_truffle_exposes_find(): + # Regression for #2: `spore.truffle` used to resolve to the method-less module. + # It must forward to a TruffleClient (has .find/.spot/.quota). This must hold + # even though importing spore.spawn/spore.truffle (top of this file) registers + # the submodules — the lazy proxy is a real attribute, so it isn't shadowed. + assert hasattr(spore.truffle, "find") + assert hasattr(spore.truffle, "spot") + assert callable(spore.truffle.find) + + +def test_top_level_spawn_exposes_launch(): + assert hasattr(spore.spawn, "launch") + assert hasattr(spore.spawn, "list") + assert callable(spore.spawn.launch) + + +def test_top_level_proxy_forwards_to_a_real_subclient(): + # The proxy's target is the actual sub-client type. + assert isinstance(spore.truffle._target(), TruffleClient) + assert isinstance(spore.spawn._target(), SpawnClient) + + +def test_private_submodule_import_does_not_shadow_top_level(): + # Importing the (now private) impl module must NOT rebind spore.spawn — the + # original bug's trigger was a same-named submodule; private names remove it. + import spore._spawn # noqa: F401 + assert hasattr(spore.spawn, "launch") # still the proxy, not the module + assert callable(spore.spawn.launch) + + +# ── Fix #3: truffle.find parses the real API keys (was silently zeroing) ──── + +def test_truffle_find_parses_memory_azs_vcpus(): + fake = FakeClient(get_return={ + "results": [{ + "instance_type": "m7i.2xlarge", + "region": "us-east-1", + "vcpus": 8, + "memory_mib": 32768, + "gpus": 0, + "architecture": "x86_64", + "on_demand_price": 0.4032, + "availability_zones": ["us-east-1a", "us-east-1b"], + }] + }) + tc = TruffleClient(fake) + results = tc.find("intel 32gb", region="us-east-1") + + assert len(results) == 1 + r = results[0] + assert r.instance_type == "m7i.2xlarge" + assert r.vcpus == 8 + assert r.memory_gib == 32.0 # was 0.0 (read memory_mi_b, absent) + assert r.available_azs == ["us-east-1a", "us-east-1b"] # was [] (available_a_zs) + assert r.on_demand_price == 0.4032 + assert r.architecture == "x86_64" + + +def test_truffle_find_parses_gpu_memory(): + fake = FakeClient(get_return={ + "results": [{ + "instance_type": "p5.48xlarge", + "region": "us-east-1", + "vcpus": 192, + "memory_mib": 2097152, + "gpus": 8, + "gpu_model": "H100", + "gpu_memory_mib": 655360, + "architecture": "x86_64", + }] + }) + r = TruffleClient(fake).find("h100")[0] + assert r.gpus == 8 + assert r.gpu_model == "H100" + assert r.gpu_memory_gib == 640.0 # was 0.0 (read gpu_memory_mi_b) + assert r.memory_gib == 2048.0 + + +# ── Fix #2: spawn.launch returns an Instance (no TypeError) ────────────────── + +def _launch_response(): + # Mirrors handleLaunch's jsonResp body (instances.go). Note: no instance_type. + return { + "instance_id": "i-0abc123", + "name": "sim-run", + "public_ip": "54.1.2.3", + "private_ip": "10.0.0.5", + "availability_zone": "us-east-1a", + "state": "pending", + "key_name": "spawn-key", + "region": "us-east-1", + } + + +def test_spawn_launch_returns_instance_with_fields(): + fake = FakeClient(post_return=_launch_response()) + sc = SpawnClient(fake) + inst = sc.launch("c7i.2xlarge", name="sim-run", ttl="4h") + + assert isinstance(inst, Instance) # was TypeError + assert inst.instance_id == "i-0abc123" + assert inst.instance_type == "c7i.2xlarge" # fallback: response omits it + assert inst.private_ip == "10.0.0.5" + assert inst.availability_zone == "us-east-1a" + assert inst.state == "pending" + assert inst._client is sc # actions/refresh work + # request body carried the essentials + _, body = fake.post_calls[0] + assert body["instance_type"] == "c7i.2xlarge" + assert body["ttl"] == "4h" + + +# ── spawn status/list parsing against real instances.go keys ──────────────── + +def test_spawn_status_parses_instance(): + fake = FakeClient(get_return={ + "instance_id": "i-0abc123", "name": "sim-run", "instance_type": "c7i.2xlarge", + "state": "running", "region": "us-east-1", "public_ip": "54.1.2.3", + "private_ip": "10.0.0.5", "availability_zone": "us-east-1a", + "ttl": "4h", "idle_timeout": "30m", "launch_time": "2026-07-09T00:00:00Z", + }) + inst = SpawnClient(fake).status("sim-run") + assert inst.state == "running" + assert inst.private_ip == "10.0.0.5" + assert inst.ttl == "4h" + assert inst.launch_time is not None + + +def test_spawn_list_parses_instances(): + fake = FakeClient(get_return={"instances": [ + {"instance_id": "i-1", "name": "a", "state": "running", "region": "us-east-1"}, + {"instance_id": "i-2", "name": "b", "state": "running", "region": "us-east-1"}, + ]}) + insts = SpawnClient(fake).list() + assert [i.instance_id for i in insts] == ["i-1", "i-2"] + + +# ── Client basics ─────────────────────────────────────────────────────────── + +def test_client_repr_masks_api_key(): + c = Client(api_key="sk_secret_value_1234567890") + assert "sk_secre" in repr(c) + assert "secret_value" not in repr(c)