Feat/workspace health#1
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| lines.append(f"**sandbox** `{client.session_id}`") | ||
| return True, lines | ||
| except ApiException as e: | ||
| return False, [e.reason or str(e)] |
There was a problem hiding this comment.
nit: only ApiException is caught here, so transport-level failures (e.g., urllib3.exceptions.MaxRetryError from a DNS/connection failure, a TLS error, or a request timeout) will propagate as uncaught exceptions and defeat the (ok, lines) contract. For a "health" probe the most useful behavior is usually to also return (False, [msg]) on those. Consider catching a broader exception type (e.g., Exception with a friendly str(e), or ApiException/OSError/URLError explicitly). (not blocking)
| from hotdata_runtime.client import HotdataClient, from_env | ||
| from hotdata_runtime.health import workspace_health_lines | ||
| from hotdata_runtime.env import ( | ||
| default_api_key, | ||
| default_host, | ||
| default_session_id, | ||
| explicit_workspace_id, | ||
| list_workspaces, | ||
| normalize_host, | ||
| pick_workspace, | ||
| ) | ||
| from hotdata_runtime.result import QueryResult |
There was a problem hiding this comment.
super nit: imports aren't alphabetical — env is after health. A ruff/isort run would reorder these to client, env, health, result. (not blocking)
|
|
||
|
|
||
| def test_version_is_pep440_core(): | ||
| assert re.fullmatch(r"\d+\.\d+\.\d+(\+.*)?", hr.__version__) |
There was a problem hiding this comment.
super nit: the regex only accepts X.Y.Z with an optional local segment; PEP 440 prereleases like 0.1.0a1, 0.1.0rc1, or dev/post releases would fail this test even though they're valid. Fine for the current 0.1.0 but it'll bite the first time someone tags a prerelease. Consider packaging.version.Version(hr.__version__) instead. (not blocking)
| with patch.object(client, "connections", return_value=Boom()): | ||
| ok, parts = workspace_health_lines(client) | ||
| assert ok is False | ||
| assert parts == ["nope"] |
There was a problem hiding this comment.
nit: no test exercises the session_id branch in workspace_health_lines that appends the **sandbox** line. A small case constructing the client with session_id="sb" and asserting that line appears would cover it. (not blocking)
…ckaging version test; sandbox and transport tests.
1 participant
No description provided.