Skip to content

Commit 17a32c5

Browse files
committed
refactor: dedupe marimo dropdown and connection helpers
Extract shared option builders into _options.py, reuse resolve_connection_picker in TableBrowser, and resolve SQL engine connection ids via runtime.
1 parent 87deab3 commit 17a32c5

6 files changed

Lines changed: 157 additions & 108 deletions

File tree

hotdata_marimo/_options.py

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
"""Shared dropdown option helpers for Marimo UI widgets."""
2+
3+
from __future__ import annotations
4+
5+
from collections.abc import Callable
6+
from typing import Any
7+
8+
import marimo as mo
9+
10+
from hotdata_runtime import HotdataClient
11+
12+
13+
def unique_label_options(
14+
pairs: list[tuple[str, str]],
15+
*,
16+
disambiguate: Callable[[str, str, int], str] | None = None,
17+
) -> dict[str, str]:
18+
"""Build a label→value map, suffixing repeated labels when needed."""
19+
counts: dict[str, int] = {}
20+
options: dict[str, str] = {}
21+
for label, value in pairs:
22+
count = counts.get(label, 0)
23+
counts[label] = count + 1
24+
if count == 0:
25+
key = label
26+
elif disambiguate is not None:
27+
key = disambiguate(label, value, count)
28+
else:
29+
key = f"{label} ({count + 1})"
30+
options[key] = value
31+
return options
32+
33+
34+
def empty_dropdown(
35+
*,
36+
label: str,
37+
message: str,
38+
full_width: bool = True,
39+
):
40+
return mo.ui.dropdown(
41+
options={message: ""},
42+
label=label,
43+
full_width=full_width,
44+
)
45+
46+
47+
def connection_options(conns: list[Any]) -> dict[str, str]:
48+
pairs = [(str(c.name), str(c.id)) for c in conns]
49+
return unique_label_options(
50+
pairs,
51+
disambiguate=lambda label, value, count: f"{label} ({value})",
52+
)
53+
54+
55+
def connection_picker(
56+
client: HotdataClient,
57+
*,
58+
label: str = "Connection",
59+
full_width: bool = True,
60+
):
61+
listing = client.connections().list_connections()
62+
conns = listing.connections
63+
if not conns:
64+
return empty_dropdown(
65+
label=label,
66+
message="(no connections)",
67+
full_width=full_width,
68+
)
69+
return mo.ui.dropdown(
70+
options=connection_options(conns),
71+
label=label,
72+
full_width=full_width,
73+
)
74+
75+
76+
def resolve_connection_picker(
77+
client: HotdataClient,
78+
*,
79+
label: str = "Connection",
80+
full_width: bool = True,
81+
) -> tuple[Any | None, str | None]:
82+
"""Return ``(dropdown_or_none, implicit_connection_id)`` for table browsers."""
83+
listing = client.connections().list_connections()
84+
conns = listing.connections
85+
if not conns:
86+
return None, ""
87+
if len(conns) == 1:
88+
return None, conns[0].id
89+
return connection_picker(client, label=label, full_width=full_width), None

hotdata_marimo/display.py

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,7 @@
66

77
from hotdata_runtime import HotdataClient, QueryResult, workspace_health_lines
88

9-
10-
def _option_map_with_unique_labels(
11-
pairs: list[tuple[str, str]],
12-
) -> dict[str, str]:
13-
counts: dict[str, int] = {}
14-
options: dict[str, str] = {}
15-
for label, value in pairs:
16-
count = counts.get(label, 0)
17-
counts[label] = count + 1
18-
key = label if count == 0 else f"{label} ({count + 1})"
19-
options[key] = value
20-
return options
9+
from hotdata_marimo._options import empty_dropdown, unique_label_options
2110

2211

2312
def query_result(
@@ -76,11 +65,15 @@ def __init__(self, client: HotdataClient, *, limit: int = 50) -> None:
7665
(f"{r.created_at} · {r.status} · {r.result_id}", r.result_id)
7766
for r in self._results
7867
]
79-
options = _option_map_with_unique_labels(option_pairs)
80-
self.pick = mo.ui.dropdown(
81-
options=options or {"(no results)": ""},
82-
label="Recent results",
83-
full_width=True,
68+
options = unique_label_options(option_pairs)
69+
self.pick = (
70+
empty_dropdown(label="Recent results", message="(no results)")
71+
if not options
72+
else mo.ui.dropdown(
73+
options=options,
74+
label="Recent results",
75+
full_width=True,
76+
)
8477
)
8578

8679
@property

hotdata_marimo/sql_engine.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ def __init__(
4040
) -> None:
4141
super().__init__(connection, engine_name)
4242
self._connections_cache: list[Any] | None = None
43-
self._connection_id_cache: dict[str, str] | None = None
4443

4544
@property
4645
def source(self) -> str:
@@ -71,15 +70,12 @@ def _resolve_should_auto_discover(
7170
return True
7271
return value
7372

74-
def _connection_ids(self) -> dict[str, str]:
75-
if self._connection_id_cache is None:
76-
self._connection_id_cache = {
77-
str(c.name): str(c.id) for c in self._connections()
78-
}
79-
return self._connection_id_cache
80-
8173
def _connection_id(self, connection_name: str) -> str | None:
82-
return self._connection_ids().get(connection_name)
74+
try:
75+
return self._connection.connection_id_by_name().get(connection_name)
76+
except RuntimeError as e:
77+
LOGGER.warning("%s", e)
78+
return None
8379

8480
def _connections(self) -> list[Any]:
8581
if self._connections_cache is None:

hotdata_marimo/table_browser.py

Lines changed: 33 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -6,39 +6,11 @@
66

77
from hotdata_runtime import HotdataClient
88

9-
10-
def _connection_options(conns: list[Any]) -> dict[str, str]:
11-
counts: dict[str, int] = {}
12-
options: dict[str, str] = {}
13-
for c in conns:
14-
label = c.name
15-
count = counts.get(label, 0)
16-
counts[label] = count + 1
17-
key = label if count == 0 else f"{label} ({c.id})"
18-
options[key] = c.id
19-
return options
20-
21-
22-
def connection_picker(
23-
client: HotdataClient,
24-
*,
25-
label: str = "Connection",
26-
full_width: bool = True,
27-
):
28-
listing = client.connections().list_connections()
29-
conns = listing.connections
30-
if not conns:
31-
return mo.ui.dropdown(
32-
options={"(no connections)": ""},
33-
label=label,
34-
full_width=full_width,
35-
)
36-
options = _connection_options(conns)
37-
return mo.ui.dropdown(
38-
options=options,
39-
label=label,
40-
full_width=full_width,
41-
)
9+
from hotdata_marimo._options import (
10+
connection_picker,
11+
empty_dropdown,
12+
resolve_connection_picker,
13+
)
4214

4315

4416
class TableBrowser:
@@ -66,44 +38,40 @@ def __init__(
6638
self._implicit_connection_id: str | None = None
6739

6840
if self._override_connection_id is None:
69-
listing = client.connections().list_connections()
70-
conns = listing.connections
71-
if len(conns) > 1:
72-
self._conn_pick = connection_picker(client)
73-
elif len(conns) == 1:
74-
self._implicit_connection_id = conns[0].id
75-
else:
76-
self._implicit_connection_id = ""
41+
self._conn_pick, self._implicit_connection_id = resolve_connection_picker(
42+
client
43+
)
7744

7845
self._table_pick_ctx: str | None = None
46+
self._init_table_pick()
7947

48+
def _init_table_pick(self) -> None:
8049
if self._conn_pick is not None:
81-
self.table_pick = mo.ui.dropdown(
82-
options={"(select connection above)": ""},
50+
self.table_pick = empty_dropdown(
8351
label="Table",
84-
full_width=True,
52+
message="(select connection above)",
53+
)
54+
self._empty_catalog = True
55+
self._all_names = []
56+
return
57+
58+
names = self._names_for_active_connection()
59+
self._all_names = names
60+
if not names:
61+
self.table_pick = empty_dropdown(
62+
label="Table",
63+
message="(no tables in catalog)",
8564
)
8665
self._empty_catalog = True
87-
self._all_names: list[str] = []
8866
else:
89-
names = self._names_for_active_connection()
90-
self._all_names = names
91-
if not names:
92-
self.table_pick = mo.ui.dropdown(
93-
options={"(no tables in catalog)": ""},
94-
label="Table",
95-
full_width=True,
96-
)
97-
self._empty_catalog = True
98-
else:
99-
self._empty_catalog = False
100-
self.table_pick = mo.ui.dropdown(
101-
options={n: n for n in names},
102-
label="Table",
103-
full_width=True,
104-
searchable=True,
105-
)
106-
self._table_pick_ctx = self._active_connection_id()
67+
self._empty_catalog = False
68+
self.table_pick = mo.ui.dropdown(
69+
options={n: n for n in names},
70+
label="Table",
71+
full_width=True,
72+
searchable=True,
73+
)
74+
self._table_pick_ctx = self._active_connection_id()
10775

10876
def _active_connection_id(self) -> str | None:
10977
if self._override_connection_id is not None:
@@ -128,10 +96,9 @@ def _rebuild_table_pick(self, names: list[str]) -> None:
12896
self._all_names = names
12997
if not names:
13098
self._empty_catalog = True
131-
self.table_pick = mo.ui.dropdown(
132-
options={"(no tables in catalog)": ""},
99+
self.table_pick = empty_dropdown(
133100
label="Table",
134-
full_width=True,
101+
message="(no tables in catalog)",
135102
)
136103
else:
137104
self._empty_catalog = False

hotdata_marimo/workspace_selector.py

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
resolve_workspace_selection,
1010
)
1111

12+
from hotdata_marimo._options import unique_label_options
13+
1214

1315
class WorkspaceSelector:
1416
"""Workspace picker that rebuilds `HotdataClient` as selection changes."""
@@ -37,17 +39,20 @@ def __init__(
3739
self._workspace_id = workspaces[0].public_id
3840
return
3941

40-
labels: list[tuple[str, str]] = []
41-
seen: set[str] = set()
42-
for w in workspaces:
43-
base = w.name
44-
label_text = base if base not in seen else f"{base} ({w.public_id})"
45-
seen.add(base)
46-
labels.append((label_text, w.public_id))
47-
48-
labels.sort(key=lambda t: 0 if t[1] == selection.workspace_id else 1)
49-
options = {k: v for k, v in labels}
50-
self._pick = mo.ui.dropdown(options=options, label=label, full_width=True)
42+
pairs = [(w.name, w.public_id) for w in workspaces]
43+
options = unique_label_options(
44+
pairs,
45+
disambiguate=lambda name, public_id, count: f"{name} ({public_id})",
46+
)
47+
items = sorted(
48+
options.items(),
49+
key=lambda item: 0 if item[1] == selection.workspace_id else 1,
50+
)
51+
self._pick = mo.ui.dropdown(
52+
options=dict(items),
53+
label=label,
54+
full_width=True,
55+
)
5156
self._workspace_id = selection.workspace_id
5257

5358
@property

tests/test_hotdata_marimo.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,8 @@
77

88
import hotdata_marimo as hm
99
from hotdata_runtime import HotdataClient
10-
from hotdata_marimo.display import _option_map_with_unique_labels
10+
from hotdata_marimo._options import connection_options, unique_label_options
1111
from hotdata_marimo.sql_engine import HotdataMarimoEngine
12-
from hotdata_marimo.table_browser import _connection_options
1312
from hotdata_marimo.workspace_selector import WorkspaceSelector, workspace_selector_from_env
1413
from marimo._types.ids import VariableName
1514

@@ -39,8 +38,8 @@ def _workspace_row(name: str, public_id: str, *, active: bool = True):
3938
),
4039
],
4140
)
42-
def test_option_map_with_unique_labels(labels, expected):
43-
assert _option_map_with_unique_labels(labels) == expected
41+
def test_unique_label_options(labels, expected):
42+
assert unique_label_options(labels) == expected
4443

4544

4645
def test_connection_options_disambiguates_duplicate_names():
@@ -49,7 +48,7 @@ def test_connection_options_disambiguates_duplicate_names():
4948
SimpleNamespace(name="Warehouse", id="conn_2"),
5049
SimpleNamespace(name="Analytics", id="conn_3"),
5150
]
52-
assert _connection_options(conns) == {
51+
assert connection_options(conns) == {
5352
"Warehouse": "conn_1",
5453
"Warehouse (conn_2)": "conn_2",
5554
"Analytics": "conn_3",

0 commit comments

Comments
 (0)