Add managed databases CLI#82
Conversation
Introduce `hotdata databases` as the UX for managed connections: create catalogs, declare tables, load parquet uploads, and query via `name.schema.table`. Includes unit and integration tests plus README docs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| fn upload_parquet_file(api: &ApiClient, path: &str) -> String { | ||
| let f = match std::fs::File::open(path) { | ||
| Ok(f) => f, | ||
| Err(e) => { | ||
| eprintln!("error opening file '{path}': {e}"); | ||
| std::process::exit(1); | ||
| } | ||
| }; | ||
|
|
||
| if !is_parquet_path(path) { | ||
| eprintln!( | ||
| "error: managed table loads require a parquet file (got '{}'). \ | ||
| Convert your data to parquet or use `hotdata datasets create` for CSV/JSON.", | ||
| path | ||
| ); | ||
| std::process::exit(1); | ||
| } |
There was a problem hiding this comment.
nit: is_parquet_path is checked after opening the file (not blocking). If a user passes --file ./orders.csv, the file is opened and then the extension check rejects it — better to validate the extension first so the wrong-format error doesn't depend on whether the file exists/is readable. Swap the order so the cheap check runs before the syscall.
| if detail.source_type != MANAGED_SOURCE_TYPE { | ||
| fail_not_managed(&detail.name, &detail.source_type); | ||
| } |
There was a problem hiding this comment.
nit: this branch is unreachable (not blocking). resolve_database above already enforces is_managed, so by the time we get here db.source_type == "managed" and db.id == detail.id. The DatabaseDetail re-check + fail_not_managed call can be dropped, along with the fail_not_managed helper which has no other callers.
| if file.is_some() && upload_id.is_some() { | ||
| eprintln!("error: pass either --file or --upload-id, not both"); | ||
| std::process::exit(1); | ||
| } |
There was a problem hiding this comment.
super nit: this runtime check is dead code (not blocking). The #[arg(long, conflicts_with = "upload_id")] on the file field makes clap reject the combination at parse time — databases_tables_load_rejects_both_file_and_upload_id_at_parse_time already covers it. The (Some(_), Some(_)) => unreachable!() arm at line 473 confirms the invariant downstream. Safe to delete these four lines.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Validate parquet extension before opening files, remove redundant managed-type check in get(), and drop dead runtime guard for conflicting load flags (clap already enforces).
Add databases command reference, activation triggers, chain workflow, and parquet load workflow to the bundled hotdata skill.
1 participant
Summary
hotdata databasescommands for managed catalogs (source_type: managed): list, create, inspect, delete, and table operations.--table(required by the current API beforetables load).POST .../loads(replacemode); query asdatabase.schema.table.Test plan
cargo test(147 unit + 5 databases_cli + 5 sandbox_env)databases create --table gdp_real→tables load(parquet) →queryreturns 50 rows