Skip to content

Commit b1f9d5c

Browse files
committed
refactor: eliminate flag-based side-effect tracking, fix unregister, remove dead code
- table_browser: extract _set_table_pick() replacing duplicate _init/_rebuild methods; _sync_table_catalog returns bool so ui drops _rebuilt_table_pick_this_run flag; standardize _active_connection_id to use 'or None' consistently - sql_engine: unregister now restores original engine_to_data_source_connection and resets sentinel so register/unregister/register round-trip works correctly - sql_editor: remove dead 'or ""' on _cached_sql (already guarded by None check above) - workspace_selector: cache HotdataClient, only reconstruct when workspace_id changes
1 parent f1f6188 commit b1f9d5c

4 files changed

Lines changed: 54 additions & 62 deletions

File tree

hotdata_marimo/sql_editor.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ def _execute_or_cached(self) -> QueryResult | None:
105105
title="Running on Hotdata",
106106
subtitle="Re-running last query and waiting for results…",
107107
):
108-
result = self._client.execute_sql(self._cached_sql or "", database=self._database)
108+
result = self._client.execute_sql(self._cached_sql, database=self._database)
109109
self._result_cache = result
110110
self._last_rerun_n = rerun_n
111111
return result

hotdata_marimo/sql_engine.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,11 @@ def register_hotdata_sql_engine() -> None:
368368

369369
def unregister_hotdata_sql_engine() -> None:
370370
"""Remove :class:`HotdataMarimoEngine` from Marimo's registry (mostly for tests)."""
371+
global _ORIGINAL_ENGINE_TO_CONNECTION
371372
from marimo._sql.get_engines import SUPPORTED_ENGINES
372373

373374
while HotdataMarimoEngine in SUPPORTED_ENGINES:
374375
SUPPORTED_ENGINES.remove(HotdataMarimoEngine)
376+
if _ORIGINAL_ENGINE_TO_CONNECTION is not None:
377+
_set_engine_to_data_source_connection(_ORIGINAL_ENGINE_TO_CONNECTION)
378+
_ORIGINAL_ENGINE_TO_CONNECTION = None

hotdata_marimo/table_browser.py

Lines changed: 37 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
resolve_connection_picker,
1313
)
1414

15+
__all__ = ["TableBrowser", "connection_picker", "table_browser"]
16+
1517

1618
class TableBrowser:
1719
"""Pick a fully qualified `connection.schema.table` and inspect columns.
@@ -43,58 +45,26 @@ def __init__(
4345
)
4446

4547
self._table_pick_ctx: str | None = None
46-
self._rebuilt_table_pick_this_run = False
4748
self._init_table_pick()
4849

49-
def _init_table_pick(self) -> None:
50-
if self._conn_pick is not None:
51-
self.table_pick = empty_dropdown(
52-
label="Table",
53-
message="(select connection above)",
54-
)
55-
self._empty_catalog = True
56-
self._all_names = []
57-
self._table_pick_ctx = ""
58-
return
59-
60-
names = self._names_for_active_connection()
61-
self._all_names = names
62-
if not names:
63-
self.table_pick = empty_dropdown(
64-
label="Table",
65-
message="(no tables in catalog)",
66-
)
67-
self._empty_catalog = True
68-
else:
69-
self._empty_catalog = False
70-
self.table_pick = mo.ui.dropdown(
71-
options={n: n for n in names},
72-
label="Table",
73-
full_width=True,
74-
searchable=True,
75-
)
76-
self._table_pick_ctx = self._active_connection_id()
77-
7850
def _active_connection_id(self) -> str | None:
7951
if self._override_connection_id is not None:
8052
return self._override_connection_id or None
8153
if self._conn_pick is not None:
82-
v = self._conn_pick.value # type: ignore[attr-defined]
83-
return v if v else None
84-
if self._implicit_connection_id is None:
85-
return None
54+
return self._conn_pick.value or None # type: ignore[attr-defined]
8655
return self._implicit_connection_id or None
8756

8857
def _names_for_active_connection(self) -> list[str]:
8958
cid = self._active_connection_id()
90-
if cid is None or cid == "":
59+
if not cid:
9160
return []
9261
return self._client.list_qualified_table_names(
9362
limit=self._table_limit,
9463
connection_id=cid,
9564
)
9665

97-
def _rebuild_table_pick(self, names: list[str]) -> None:
66+
def _set_table_pick(self, names: list[str]) -> None:
67+
"""Create or replace the table dropdown for the given names list."""
9868
self._all_names = names
9969
if not names:
10070
self._empty_catalog = True
@@ -111,7 +81,32 @@ def _rebuild_table_pick(self, names: list[str]) -> None:
11181
searchable=True,
11282
)
11383
self._table_pick_ctx = self._active_connection_id()
114-
self._rebuilt_table_pick_this_run = True
84+
85+
def _init_table_pick(self) -> None:
86+
if self._conn_pick is not None:
87+
self._all_names = []
88+
self._empty_catalog = True
89+
self.table_pick = empty_dropdown(
90+
label="Table",
91+
message="(select connection above)",
92+
)
93+
self._table_pick_ctx = ""
94+
return
95+
self._set_table_pick(self._names_for_active_connection())
96+
97+
def _sync_table_catalog(self) -> bool:
98+
"""Refresh the table dropdown when the active connection changes.
99+
100+
Returns True if the dropdown was rebuilt (so the caller knows not to
101+
read ``.value`` on the new widget in the same Marimo run).
102+
"""
103+
if self._conn_pick is not None:
104+
_ = self._conn_pick.value # type: ignore[attr-defined]
105+
cid = self._active_connection_id()
106+
if not cid or cid == self._table_pick_ctx:
107+
return False
108+
self._set_table_pick(self._names_for_active_connection())
109+
return True
115110

116111
@property
117112
def selected_connection_id(self) -> str | None:
@@ -122,30 +117,17 @@ def selected_table(self) -> str | None:
122117
v = self.table_pick.value
123118
return v if v else None
124119

125-
def _sync_table_catalog(self) -> None:
126-
"""Refresh the table dropdown when the active connection changes."""
127-
if self._conn_pick is not None:
128-
_ = self._conn_pick.value # type: ignore[attr-defined]
129-
cid = self._active_connection_id()
130-
if not cid:
131-
return
132-
if cid == self._table_pick_ctx:
133-
return
134-
self._rebuild_table_pick(self._names_for_active_connection())
135-
136120
@property
137121
def ui(self):
138-
self._rebuilt_table_pick_this_run = False
139-
self._sync_table_catalog()
140-
141-
if not self._rebuilt_table_pick_this_run:
122+
rebuilt = self._sync_table_catalog()
123+
if not rebuilt:
142124
_ = self.table_pick.value
143125

144-
sel = None if self._rebuilt_table_pick_this_run else self.selected_table
126+
sel = None if rebuilt else self.selected_table
145127
cid = self._active_connection_id()
146128
conn_header = (
147-
mo.md(f"**Connection** `{self._active_connection_id()}`")
148-
if self._active_connection_id()
129+
mo.md(f"**Connection** `{cid}`")
130+
if cid
149131
else None
150132
)
151133
if not sel:

hotdata_marimo/workspace_selector.py

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ def __init__(
2626
self._api_key = api_key
2727
self._host = host or default_host()
2828
self._session_id = session_id
29+
self._client_cache: HotdataClient | None = None
30+
self._client_cache_wid: str | None = None
2931
selection = resolve_workspace_selection(api_key, self._host, session_id)
3032
self._explicit = selection.source == "explicit_env"
3133
if self._explicit:
@@ -64,12 +66,16 @@ def workspace_id(self) -> str:
6466

6567
@property
6668
def client(self) -> HotdataClient:
67-
return HotdataClient(
68-
self._api_key,
69-
self.workspace_id,
70-
host=self._host,
71-
session_id=self._session_id,
72-
)
69+
wid = self.workspace_id
70+
if self._client_cache is None or self._client_cache_wid != wid:
71+
self._client_cache = HotdataClient(
72+
self._api_key,
73+
wid,
74+
host=self._host,
75+
session_id=self._session_id,
76+
)
77+
self._client_cache_wid = wid
78+
return self._client_cache
7379

7480
@property
7581
def ui(self):

0 commit comments

Comments
 (0)