Skip to content

feat: add database= parameter to sql engine and editor for managed database scoping#8

Merged
eddietejeda merged 3 commits into
masterfrom
fix/execute-sql-database-scope
May 25, 2026
Merged

feat: add database= parameter to sql engine and editor for managed database scoping#8
eddietejeda merged 3 commits into
masterfrom
fix/execute-sql-database-scope

Conversation

@eddietejeda
Copy link
Copy Markdown
Contributor

Summary

  • Adds default_database: str | None = None to HotdataMarimoEngine.__init__, passed to execute_sql() so mo.sql queries are scoped to a managed database via X-Database-Id header
  • Adds database: str | None = None to SqlEditor.__init__ and sql_editor() factory, passed to both execute_sql calls (run and rerun paths)
  • Updates ManagedDatabaseWriter to use description= kwarg for create_managed_database (matches ManagedDatabase API from hotdata-runtime v0.2.0)
  • Bumps hotdata-runtime dependency to >=0.2.1 to pick up the database= support and X-Database-Id header fix
  • Fixes test_databases_marimo.py syntax error from corrupted duplicate function and updates create_managed_database assertion to use description= kwarg

…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
Comment thread hotdata_marimo/databases.py Outdated
Comment on lines +130 to +131
options = {db.description or db.id: db.id for db in dbs}
value = current if current in options.values() else next(iter(options.values()))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: (not blocking) two subtleties here worth a quick check:

  1. Dropdown value semantics changed from name to id. Previously database.value was the database name; now it's the id. That id is then passed positionally to load_managed_table(database, ...) in _maybe_load (line 189). Worth double-checking that hotdata-runtime 0.2.1's load_managed_table accepts the id as the first arg (vs. the old name) — test_managed_database_writer_loads_parquet still asserts load_managed_table.assert_called_once_with("sales", ...) and only passes because the dropdown is mocked, so it doesn't actually validate the new behavior.

  2. Description collisions silently collapse. Two managed databases with the same description (or both unset, falling back to db.id which is unique — that case is fine) collapse to a single dropdown entry because dict keys must be unique. Probably rare, but consider disambiguating, e.g. {(db.description or db.id) if uniq else f"{db.description} ({db.id})": db.id for ...} or just including the id in the label unconditionally.

claude[bot]
claude Bot previously approved these changes May 25, 2026
…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
claude[bot]
claude Bot previously approved these changes May 25, 2026
…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.
@eddietejeda eddietejeda merged commit f15c942 into master May 25, 2026
2 checks passed
@eddietejeda eddietejeda deleted the fix/execute-sql-database-scope branch May 25, 2026 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant