fix(databases): use name not description for API alignment#112
Merged
Conversation
The Hotdata API renamed the database identifier field from `description` to `name`. This updates: - `try_resolve_database`: filter by `d.name` instead of `d.description` so that `hotdata databases get <name>` works against the new API - `databases set`: accept the database id directly rather than resolving by name/description — the id is unambiguous and always available - Error messages and help text: "id or description" → "id or name" - Tests: update mock payloads to return `name` (not `description`)
Contributor
There was a problem hiding this comment.
Review
Blocking Issues
src/command.rs:577-585—--tablehelp text claimsschema.tabledot notation indatabases createlands tables in the named schema (e.g.--table raw.raw_orders→rawschema), butcreate_database_request(src/databases.rs:164-173) doesn't parse the dot at all — it sends the literal string as a single table name inside the--schemavalue. The doc and behavior disagree, and there's no test for the claim.
Action Required
- Either implement schema parsing in
create_database_request(split on., group tables by resolved schema) and add a test covering--table raw.raw_orders --table customers, or drop the dot-notation claim from the two--schema/--tabledoc comments incommand.rs.
The remaining inline comments are nits and non-blocking.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ase_request The --table flag doc comments claimed that `schema.table` entries would land in the named schema, but the implementation was sending literal strings as table names inside a single schema. This implements the promised behavior: entries with a dot are split into (schema, table) and grouped accordingly, bare names fall back to the --schema default. Adds a test covering the multi-schema case. Also removes an extra blank line between resolve_database and schema_name.
- Drop `description` field from DatabaseSummary, Database, and CreateDatabaseResponse structs — the API now returns `name` only - Remove `--description` flag from `databases create` and the corresponding `"description"` JSON key from create_database_request - Drop DESCRIPTION column from `databases list` table output - Remove the always-blank `description: -` row from `databases show` - `databases set` now resolves the input to a real db.id before saving, validating the database exists and ensuring the config always holds an ID - Rename id_or_description params to id_or_name in get/delete/set - Update all tests and the databases_cli integration test
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Hotdata API renamed the database identifier field from
descriptiontoname(SDK 0.2.4). This PR updates the CLI to match:try_resolve_database: now filters the list byd.nameinstead ofd.description— fixeshotdata databases get <name>against the new APIdatabases set: now accepts the database id directly rather than resolving by name/description (id is unambiguous and always available afterdatabases create)namefield (notdescription)Test plan
cargo test databases— all 24 tests passhotdata databases get <name>resolves correctlyhotdata databases set <id>saves config without a network round-trip