refactor: simplify redundant expressions in display and workspace_selector#11
Conversation
…tabase scoping Pass database= to client.execute_sql() so queries are scoped to a managed database via the X-Database-Id header (hotdata-runtime>=0.2.1). - HotdataMarimoEngine: add default_database= constructor param, pass to execute() - SqlEditor: add database= constructor param, pass to both execute_sql calls - ManagedDatabaseWriter: use description= kwarg matching ManagedDatabase v0.2.0 API - Fix test_databases_marimo.py syntax error and update assertions
…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
…t param
When options is a dict {label: value}, Marimo validates value= against the
dict keys (labels), not the values. _rebuild_database_pick was passing a
database ID (dict value) which raised ValueError on startup. Now resolves
the label key corresponding to the previously-selected ID instead.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| return | ||
| options = {db.name: db.name for db in dbs} | ||
| value = current if current in options else next(iter(options)) | ||
| options = {db.description or db.id: db.id for db in dbs} |
There was a problem hiding this comment.
nit: (not blocking) if two managed databases share the same description, this dict collapses and one option is silently dropped. Descriptions are user-supplied free text so collisions are plausible (more so than with the old name field). Consider routing through unique_label_options from _options.py — same helper used by connection_options for exactly this case.
| v = self.table_pick.value | ||
| return v if v else None |
There was a problem hiding this comment.
super nit: (not blocking) this PR collapses several v if v else None patterns into ... or None (e.g. _active_connection_id, selected_result_id). For consistency, this could be return self.table_pick.value or None.
| ) | ||
| rows: list[dict[str, object]] = [ | ||
| {"name": db.name, "id": db.id, "sql_prefix": f"{db.name}.{{schema}}.{{table}}"} | ||
| {"description": db.description or db.id, "id": db.id, "sql_prefix": f"{db.id}.{{schema}}.{{table}}"} |
There was a problem hiding this comment.
super nit: (not blocking) db.description or db.id appears on lines 50, 130, and 215. A small _db_label(db) helper would DRY this up and keep behavior in lockstep if the fallback ever changes.
1 participant
display.py:rid if rid else None→rid or None; removed deadrid or ""guard aftermo.stop(execution never reaches it when rid is None)workspace_selector.py:v if v else x→v or x