Skip to content

Commit 80c82ed

Browse files
committed
refactor: clean up code quality issues
- backend.py: replace walrus-then-overwrite pattern with plain assignments in _to_catalog_db_tuple - backend.py: getattr(self, '_http', None) is not None -> hasattr(self, '_http') - backend.py: add explanatory comments to the silent pass in _resolve_database_connection_id and the no-op _register_in_memory_table - http.py: replace magic HTTP status integers with http.HTTPStatus constants - http.py: replace range(len(columns)) index loop with direct field iteration - types.py: cache m.group(2) to avoid calling it twice - Remove managed.py (single-constant module); inline 'public' directly at the two call sites in backend.py and http.py
1 parent f25b43b commit 80c82ed

4 files changed

Lines changed: 19 additions & 23 deletions

File tree

src/ibis_hotdata/backend.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
from ibis.backends.sql import SQLBackend
4444

4545
from ibis_hotdata.http import HotdataAPIError, HotdataClient
46-
from ibis_hotdata.managed import DEFAULT_SCHEMA
4746
from ibis_hotdata.types import dtype_from_hotdata_sql_type
4847

4948
_INFORMATION_SCHEMA_PAGE_SIZE = 500
@@ -203,7 +202,7 @@ def do_connect(
203202
)
204203

205204
def disconnect(self) -> None:
206-
if getattr(self, "_http", None) is not None:
205+
if hasattr(self, "_http"):
207206
self._http.close()
208207

209208
# --- hierarchy ---------------------------------------------------------
@@ -253,10 +252,12 @@ def _to_catalog_db_tuple(self, table_loc: sge.Table):
253252
"""Use the compiler SQL dialect when stringifying qualifiers (backend name is not a dialect)."""
254253

255254
dialect = self.dialect
256-
if (sg_cat := table_loc.args["catalog"]) is not None:
255+
sg_cat = table_loc.args["catalog"]
256+
if sg_cat is not None:
257257
sg_cat.args["quoted"] = False
258258
sg_cat = sg_cat.sql(dialect=dialect)
259-
if (sg_db := table_loc.args["db"]) is not None:
259+
sg_db = table_loc.args["db"]
260+
if sg_db is not None:
260261
sg_db.args["quoted"] = False
261262
sg_db = sg_db.sql(dialect=dialect)
262263

@@ -429,7 +430,7 @@ def _resolve_database_connection_id(self) -> str | None:
429430
db = self._http.get_database(self._database_id)
430431
self._database_connection_id = db.get("default_connection_id")
431432
except HotdataAPIError:
432-
pass
433+
pass # best-effort: if the lookup fails, callers fall back to the catalog name
433434
return self._database_connection_id
434435

435436
# --- schema / sql execution --------------------------------------------
@@ -575,7 +576,7 @@ def create_database(
575576
/,
576577
*,
577578
catalog: str | None = None,
578-
schema: str = DEFAULT_SCHEMA,
579+
schema: str = "public",
579580
tables: Sequence[str] | None = None,
580581
force: bool = False,
581582
) -> None:
@@ -722,7 +723,7 @@ def drop_table(
722723
raise _ibis_err_from_hotdata(exc) from exc
723724

724725
def _register_in_memory_table(self, _op: ops.InMemoryTable) -> None:
725-
return
726+
pass # Hotdata has no local in-memory table concept; Ibis calls this hook before execute
726727

727728
@cached_property
728729
def version(self) -> str:

src/ibis_hotdata/http.py

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import http
56
import io
67
import json
78
import time
@@ -30,8 +31,6 @@
3031
from hotdata.models.database_default_table_decl import DatabaseDefaultTableDecl
3132
from hotdata.models.load_managed_table_request import LoadManagedTableRequest
3233

33-
from ibis_hotdata.managed import DEFAULT_SCHEMA
34-
3534
T = TypeVar("T")
3635

3736
# Matches Hotdata / runtimedb ``GET /v1/results/{{id}}`` Arrow responses.
@@ -197,7 +196,7 @@ def create_managed_database(
197196
self,
198197
description: str | None = None,
199198
*,
200-
schema: str = DEFAULT_SCHEMA,
199+
schema: str = "public",
201200
tables: Sequence[str] = (),
202201
) -> dict[str, Any]:
203202
"""POST ``/v1/databases`` — creates a managed database with an auto-provisioned default catalog."""
@@ -264,27 +263,27 @@ def _poll_result_arrow(
264263
status = raw.status
265264
ctype = (raw.headers.get("Content-Type") or "").split(";")[0].strip().lower()
266265

267-
if status == 200 and ctype == APPLICATION_ARROW_STREAM.lower():
266+
if status == http.HTTPStatus.OK and ctype == APPLICATION_ARROW_STREAM.lower():
268267
table = _ipc_stream_bytes_to_table(body)
269268
return self._arrow_payload_from_table(table, result_id=result_id)
270269

271-
if status == 202:
270+
if status == http.HTTPStatus.ACCEPTED:
272271
_sleep_until(deadline, poll_interval_s)
273272
continue
274273

275-
if status == 409:
274+
if status == http.HTTPStatus.CONFLICT:
276275
d = _json_utf8(body) if body else {}
277276
raise HotdataAPIError(
278277
d.get("error_message") or "Result failed",
279-
status_code=409,
278+
status_code=http.HTTPStatus.CONFLICT,
280279
body=d,
281280
)
282281

283-
if status == 404:
282+
if status == http.HTTPStatus.NOT_FOUND:
284283
d = _json_utf8(body) if body else {}
285284
raise HotdataAPIError(
286285
d.get("detail") or f"Result {result_id!r} not found",
287-
status_code=404,
286+
status_code=http.HTTPStatus.NOT_FOUND,
288287
body=d,
289288
)
290289

@@ -304,7 +303,7 @@ def _arrow_payload_from_table(
304303
) -> dict[str, Any]:
305304
sch = table.schema
306305
columns = sch.names
307-
nullable = [sch.field(i).nullable for i in range(len(columns))]
306+
nullable = [field.nullable for field in sch]
308307
return {
309308
"format": "arrow",
310309
"pa_table": table,

src/ibis_hotdata/managed.py

Lines changed: 0 additions & 5 deletions
This file was deleted.

src/ibis_hotdata/types.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@ def _pa_type_from_arrow_str(raw: str) -> pa.DataType | None:
8080
m = _TIMESTAMP_RE.match(s)
8181
if m:
8282
unit = m.group(1).lower()
83-
tz: str | None = m.group(2).strip() if m.group(2) else None
83+
tz_raw = m.group(2)
84+
tz: str | None = tz_raw.strip() if tz_raw else None
8485
try:
8586
return pa.timestamp(unit, tz=tz)
8687
except Exception:

0 commit comments

Comments
 (0)