Skip to content

Commit 5161036

Browse files
committed
fix(managed): raise on a query reply shape this client cannot read
The last route by which "I do not know" could read as "no rows". If an SDK release adds a third reply model for the query endpoint, neither `isinstance` branch matches, `_query_database_scoped` returns `None`, `fetch_table` returns `None`, and `fetch_table_rows` turns that into `[]` -- the same answer it gives for a table that is not synced. A merge or append load would then write only its new batch over rows it believed were absent. Same collapse the previous commit closed for a succeeded run that saved nothing, by the other path into it. `HotdataClient` already raised here, so this also settles a disagreement between the two clients. A `None` from `fetch_table` now carries exactly one meaning: the table is not synced.
1 parent 19ae3d4 commit 5161036

3 files changed

Lines changed: 53 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3131
table. `fetch_table` answered `None` for it, which `fetch_table_rows` turns
3232
into `[]`, the same answer both give for a table that is not synced. A
3333
read-modify-write load would have read no existing rows and written only its
34-
new batch, dropping every row already there. Arrow stays the only path the
35-
data travels, so column types come from the server's schema rather than being
36-
inferred from JSON.
34+
new batch, dropping every row already there. A reply shape this client does not
35+
recognise raises for the same reason, as `HotdataClient` already did — so a
36+
`None` from `fetch_table` now means one thing only: the table is not synced.
37+
Arrow stays the only path the data travels, so column types come from the
38+
server's schema rather than being inferred from JSON.
3739

3840
Costs one extra round trip on a query that would have answered synchronously,
3941
in exchange for not transferring the result twice.

hotdata_framework/managed_client.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,13 @@ def _query_database_scoped(self, sql: str, *, database_id: str) -> str | None:
162162
# in the background, so it is not the finish line either.
163163
if isinstance(raw, (QueryResponse, AsyncQueryResponse)):
164164
return self._await_query_run(raw.query_run_id, database_id=database_id)
165-
return None
165+
# Returning nothing here would read as an empty table: `fetch_table`
166+
# answers `None`, `fetch_table_rows` turns that into `[]`, and a
167+
# read-modify-write load would write only its new batch over rows it
168+
# believed were not there. A reply shape this client does not know is a
169+
# reason to stop, not to report emptiness. `HotdataClient` raises on the
170+
# same condition.
171+
raise RuntimeError(f"Unexpected query response type: {type(raw)!r}")
166172

167173
def _await_query_run(self, query_run_id: str, *, database_id: str) -> str | None:
168174
"""Wait for a query run to finish; return the result id it produced.

tests/test_managed_client.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -926,3 +926,44 @@ def test_query_run_model_carries_every_field_this_client_reads() -> None:
926926
"""
927927
for field in ("status", "result_id", "error_message", "warning_message"):
928928
assert field in QueryRunInfo.model_fields, field
929+
930+
931+
def test_an_unknown_query_reply_shape_raises_rather_than_reading_empty(
932+
monkeypatch: pytest.MonkeyPatch,
933+
) -> None:
934+
"""The last route by which "I do not know" could have read as "no rows".
935+
936+
If an SDK release adds a third reply model for the query endpoint, neither
937+
`isinstance` branch matches. Falling through to `None` would surface as an
938+
empty table -- and `fetch_table_rows` maps `None` to `[]`, the same answer it
939+
gives for a table that is not synced -- so a merge or append load would drop
940+
every row already there. `HotdataClient` raises on this condition; now both
941+
do.
942+
943+
After this, a `None` from `fetch_table` means one thing only: the table is
944+
not synced.
945+
"""
946+
947+
class FakeQueryApi:
948+
def __init__(self, api: object) -> None:
949+
pass
950+
951+
def query(self, request: object, *, x_database_id: str) -> Any:
952+
# A shape from neither branch -- a future reply model, as far as
953+
# this client is concerned.
954+
return SimpleNamespace(something_new="?")
955+
956+
monkeypatch.setattr(mc, "QueryApi", FakeQueryApi)
957+
monkeypatch.setattr(mc.time, "sleep", lambda _seconds: None)
958+
959+
client = mc.ManagedDatabaseClient(
960+
api_key="k",
961+
workspace_id="w",
962+
api_base_url="https://example.test",
963+
max_retries=1,
964+
retry_backoff_seconds=0.0,
965+
)
966+
client._runtime = _fake_runtime()
967+
968+
with pytest.raises(HotdataTerminalError, match="Unexpected query response type"):
969+
client.fetch_table_rows(database="mydb", schema="public", table="orders")

0 commit comments

Comments
 (0)