feat: LlamaIndex tools for Hotdata managed databases with database= scoping#2
Conversation
Require the latest SDK directly alongside hotdata-runtime for clearer version alignment.
Add scripts/release.sh for version bumps, changelog updates, tagging, and GitHub Release creation via CI. Enforce changelog checks on version PRs.
Run locked uv sync and pytest on pull requests. Validate examples/demo.ipynb structure and optional live smoke when HOTDATA_API_KEY is set.
Avoid duplicate [Unreleased] headings and keep the preamble intact when preparing releases. Add unit tests and harden release workflow output.
…ping 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). Also updates ManagedDatabase constructor calls to use description= and default_connection_id= fields introduced in hotdata-runtime v0.2.0.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| requires-python = ">=3.10" | ||
| license = { text = "MIT" } | ||
| dependencies = [ | ||
| "hotdata-runtime>=0.2.0", |
There was a problem hiding this comment.
The PR description states that database= support requires hotdata-runtime >= 0.2.1, but this pin is >=0.2.0. A user resolving against this constraint can land on 0.2.0, where client.execute_sql(sql, database=...) will fail at runtime (the kwarg doesn't exist there). The uv.lock pins 0.2.1, but downstream installs use the pyproject constraint.
Bump to hotdata-runtime>=0.2.1 to match the actual minimum required for the new code path.
| "hotdata-runtime>=0.2.0", | |
| "hotdata-runtime>=0.2.1", |
| "hotdata_list_managed_databases", | ||
| "hotdata_create_managed_database", | ||
| "hotdata_load_managed_table", | ||
| } |
There was a problem hiding this comment.
nit: (not blocking) make_hotdata_tools gained a database= parameter, but there's no test that the bound database actually flows through hotdata_execute_sql when the tool is invoked. The existing test_execute_sql_json_with_database covers the direct function call but not the closure inside make_hotdata_tools. A one-liner asserting mock_client.execute_sql.assert_called_with("select 1", database="my_db") after make_hotdata_tools(mock_client, database="my_db") would close that gap.
| rows = [ | ||
| { | ||
| "description": db.description, | ||
| "id": db.id, | ||
| "sql_prefix": f"{db.id}.{{schema}}.{{table}}", | ||
| } | ||
| for db in client.list_managed_databases() | ||
| ] |
There was a problem hiding this comment.
nit: (not blocking) db.description can be None for managed databases, which would emit "description": null here. managed_database_summary already falls back to db.id for this case — consider applying the same fallback so the LLM-facing JSON is consistent across both endpoints.
| base = "origin/main" | ||
| for candidate in ("origin/main", "origin/master"): | ||
| if subprocess.call(["git", "rev-parse", "--verify", candidate], stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL) == 0: | ||
| base = candidate | ||
| break |
There was a problem hiding this comment.
super nit: (not blocking) The initial base = "origin/main" is redundant — the loop immediately re-evaluates the same candidate as the first iteration. You can drop the initial assignment, or initialize to None and raise SystemExit(...) if neither remote exists (instead of silently falling through to a non-existent ref and triggering the "skip" branch).
There was a problem hiding this comment.
Review
Blocking Issues
pyproject.toml:13—hotdata-runtime>=0.2.0doesn't actually guaranteedatabase=support. Per the PR description, that kwarg landed in 0.2.1, so any consumer resolving to 0.2.0 will hit aTypeErrorwhen callingexecute_sql_jsonor anyhotdata_execute_sqltool. The lockfile pinning 0.2.1 only protects this repo's CI — downstream installs use the project constraint.
Action Required
- Bump the
hotdata-runtimeconstraint inpyproject.tomlto>=0.2.1.
Nits left inline (non-blocking).
eddietejeda
changed the title
1 participant
Summary
This PR consolidates two lines of work (previously split across #1 and this branch):
Managed database tools (was #1):
Database scoping fix: