Skip to content

Commit 19ae3d4

Browse files
committed
fix(managed): read the run's warning defensively, and pin the fields
`warning_message` is read while building an error, which is the worst place to assume an attribute: if an SDK release dropped it, the `AttributeError` would replace a message naming the problem with one naming nothing. `getattr` keeps the raise intact and loses only the explanation. Pinning the assumption is the better half of the fix, though. Every fake in the managed-client tests is a `SimpleNamespace`, so nothing there would notice a field being renamed or dropped -- the fakes would keep answering and the suite would keep passing, with the cost landing at runtime. A test now asserts the generated `QueryRunInfo` carries all four fields this client reads off a run, not just the one that prompted this.
1 parent b017161 commit 19ae3d4

2 files changed

Lines changed: 20 additions & 1 deletion

File tree

‎hotdata_framework/managed_client.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,10 +196,14 @@ def _await_query_run(self, query_run_id: str, *, database_id: str) -> str | None
196196
# and write only its new batch, dropping what was there.
197197
# Terminal rather than transient: re-running the query
198198
# cannot save a result that was already discarded.
199+
# `getattr` because this runs while building an error: if
200+
# the field ever goes away, losing the explanation is a far
201+
# better outcome than an AttributeError replacing the raise.
202+
warning = getattr(run, "warning_message", None)
199203
raise RuntimeError(
200204
f"Query run {query_run_id} succeeded but its result was not "
201205
f"saved, so the table cannot be read"
202-
+ (f": {run.warning_message}" if run.warning_message else "")
206+
+ (f": {warning}" if warning else "")
203207
)
204208
return run.result_id
205209
if run.status == "interrupted":

‎tests/test_managed_client.py‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from hotdata.arrow import ResultNotReadyError
1111
from hotdata.models.async_query_response import AsyncQueryResponse
1212
from hotdata.models.query_response import QueryResponse
13+
from hotdata.models.query_run_info import QueryRunInfo
1314
from hotdata.rest import ApiException
1415

1516
import hotdata_framework.managed_client as mc
@@ -911,3 +912,17 @@ def get_result_arrow(self, result_id: str, **kwargs: Any) -> pa.Table:
911912
assert table is not None
912913
assert table.num_rows == 2
913914
assert len(attempts) == 3
915+
916+
917+
def test_query_run_model_carries_every_field_this_client_reads() -> None:
918+
"""Pins the attributes read off a query run against the generated model.
919+
920+
Every fake in this file is a `SimpleNamespace`, so nothing else here would
921+
notice if one of these fields were renamed or dropped by an SDK release --
922+
the fakes would keep answering and the tests would keep passing. The cost
923+
lands at runtime, and worst on `warning_message`, which is read while
924+
building an error: losing it turns a message naming the problem into an
925+
`AttributeError` naming nothing.
926+
"""
927+
for field in ("status", "result_id", "error_message", "warning_message"):
928+
assert field in QueryRunInfo.model_fields, field

0 commit comments

Comments
 (0)