Skip to content

feat(0.22.0): storage describe-bucket/table/column/batch - #205

Closed
ottomansky wants to merge 9 commits into
keboola:release/0.22.0from
ottomansky:fiia/pr7-storage-descriptions
Closed

feat(0.22.0): storage describe-bucket/table/column/batch#205
ottomansky wants to merge 9 commits into
keboola:release/0.22.0from
ottomansky:fiia/pr7-storage-descriptions

Conversation

@ottomansky

Copy link
Copy Markdown
Contributor

Summary

  • storage describe-bucket — sets KBC.description on a bucket via POST /v2/storage/buckets/{id}/metadata (upsert-by-key, provider=user)
  • storage describe-table — sets KBC.description on a table via POST /v2/storage/tables/{id}/metadata
  • storage describe-column — sets per-column descriptions using the KBC.column.{name}.description key convention in table metadata (Keboola Storage has no user-writable column-level metadata endpoint; this is the supported workaround)
  • storage describe-batch --from-file YAML — batch-applies bucket, table, and column descriptions from a YAML file; item-level failures are collected without aborting remaining work

Round-trip: bucket-detail and table-detail now surface a description field extracted from the metadata array, and table-detail surfaces column_details[].description for annotated columns.

Architecture

  • Client (client.py): set_bucket_metadata + set_table_metadata using PHP-array form encoding (metadata[i][key/value]), branch-aware URL prefix
  • Service (storage_service.py): describe_bucket, describe_table, describe_columns, describe_batch; updated get_bucket_detail + get_table_detail to extract descriptions from the metadata array
  • Commands (commands/storage.py): 4 CLI commands with --text|--file|--stdin (bucket/table) or --column NAME=DESC (column) input; all branch-aware
  • Tests: 27 new unit tests (13 CLI via CliRunner + 14 service-layer) + E2E step 14.2

Test plan

  • ruff check src/ tests/ + ruff format --check — clean
  • pytest tests/test_storage_describe_cli.py — 13/13 passed
  • pytest tests/test_storage_describe_service.py — 14/14 passed
  • pytest tests/test_client.py -k "Metadata" — 6/6 passed
  • All 17 failures in full suite are pre-existing Windows file-permission / POSIX-locking issues (verified on main before this branch)
  • Manual E2E against connection.europe-west3.gcp.keboola.com project 1143: all 4 commands + round-trips verified

Add native description writes for Storage assets via the Keboola
metadata API (upsert-by-key, provider=user):

- storage describe-bucket: sets KBC.description on a bucket
- storage describe-table: sets KBC.description on a table
- storage describe-column: sets KBC.column.{name}.description keys
  in table metadata (workaround for missing user-writable column
  metadata endpoint)
- storage describe-batch --from-file YAML: apply all three in one
  shot; item failures are collected without aborting remaining work

Round-trip: bucket-detail and table-detail now surface the description
field (extracted from the metadata array) and table-detail exposes
column_details[].description for annotated columns.

Adds: client.set_bucket_metadata + set_table_metadata (PHP-array
form encoding, branch-aware), 4 service methods, 4 CLI commands,
permissions registry entries, hint definitions, context docs, and
27 new unit tests (13 CLI + 14 service) + E2E steps.
The describe-bucket hint note said descriptions only appear in the
metadata array. After this PR, get_bucket_detail extracts KBC.description
and surfaces it as the top-level description field, so the note is
wrong and could mislead an AI agent.
- Remove _patch_services() helper in test_storage_describe_cli.py
  (defined but never called -- every test has its own with-block)
- Fix describe_batch() docstring: Returns says 'skipped/results'
  but actual keys are applied_count/error_count
- Simplify except (KeboolaApiError, Exception) -> except Exception
  (KeboolaApiError is already an Exception subclass; isinstance check
  inside the handler still distinguishes the two cases)
padak
padak previously requested changes Apr 22, 2026
@padak
padak dismissed their stale review April 22, 2026 21:41

Replacing with English version for broader team visibility.

@padak padak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Changes requested — one required addition per CONTRIBUTING.md

The PR is architecturally clean, test coverage is excellent (1884 passed), version sync OK, make skill-gen clean. Only a required hand-maintained doc is missing:

Required fix

plugins/kbagent/skills/kbagent/references/commands-reference.md — 4 lines missing (CONTRIBUTING.md line 204: "this is a hand-maintained file, NOT auto-generated"). Add to the ## Storage section after delete-bucket:

kbagent storage describe-bucket --project NAME --bucket-id ID --text STR|--file PATH|--stdin [--branch ID]
kbagent storage describe-table --project NAME --table-id ID --text STR|--file PATH|--stdin [--branch ID]
kbagent storage describe-column --project NAME --table-id ID --column NAME=DESCRIPTION [--branch ID]
kbagent storage describe-batch --project NAME --file PATH [--branch ID]

(Verify signatures against your actual @typer.Option definitions.)

Nice-to-have (follow-up)

  • Unit test gap: The enhancement in get_bucket_detail/get_table_detail (extracting description from metadata + the new metadata field) is only covered via E2E. Add 2-3 tests to test_services.py for raw_metadata + description precedence.
  • gotchas.md: A short paragraph about:
    • Column description workaround — KBC.column.{name}.description convention (the Keboola API has no user-writable column-metadata endpoint).
    • describe-batch partial-failure semantics (it doesn't abort, it collects errors).
    • Precedence between the native API description field and the KBC.description metadata entry.
  • Progress indicator: src/keboola_agent_cli/commands/storage.py:1402-1418describe-batch prints nothing during the iteration in human mode. With 100+ items it looks frozen. A Rich status/spinner would help.
  • Dedicated storage-describe-workflow.md: a YAML schema example + batch workflow would deserve its own doc.

Positives

  • Architecturally elegant approach: 4 sugar commands over the /metadata endpoint, upsert-by-key with provider=user, branch-aware.
  • urllib.parse.quote(id, safe="") on bucket_id and table_id (path traversal protection).
  • describe-batch partial-failure collects errors instead of aborting — the right pattern for batch ops.
  • Complete test matrix (14 service + 13 CLI + E2E round-trip with read-back via bucket-detail/table-detail).

Document the four storage describe commands (bucket, table, column, batch)
in the hand-maintained commands-reference.md after delete-bucket.  Signatures
match the @typer.Option definitions in storage.py.
…le_detail

Adds six unit tests that exercise the read-back side of the describe-*
feature (previously covered only via E2E):

- get_bucket_detail: extracts 'description' from the KBC.description
  (provider=user) metadata entry; exposes raw metadata[]; ignores non-user
  providers; falls back to the native 'description' field when absent.
- get_table_detail: extracts table 'description' from KBC.description and
  per-column descriptions from KBC.column.{name}.description into
  column_details[].description; omits 'description' for columns with no
  matching metadata entry.

Precedence is pinned: when both the native 'description' field and a
user-provided KBC.description metadata entry are present, the metadata
entry wins (the native field is only settable at bucket-create time via
the Storage API; user updates flow through the metadata endpoint).
Adds a Storage descriptions subsection to gotchas.md covering:

- Column descriptions use the KBC.column.{name}.description metadata-key
  convention because the Storage API has no user-writable column-metadata
  endpoint; read-back via table-detail uses the same key.
- describe-batch is partial-failure-tolerant: per-item errors are collected
  into result.errors[] but the batch does not abort; the CLI exits non-zero
  only when error_count > 0.
- Description-field precedence: when both the native API description and a
  user-provided KBC.description metadata entry are present, the metadata
  value wins.  Non-user providers (e.g. system) are ignored on read-back.
Large batches (100+ items) previously looked frozen because describe-batch
printed nothing between kicking off and the final summary.  Wire a Rich
Progress spinner that updates per item in human mode; JSON mode is
unchanged (no Rich output on stdout/stderr).

- StorageService.describe_batch() takes an optional progress_callback
  that is invoked once per item with (obj_type, obj_id, current, total)
  before each API call.  JSON mode passes None; the service path stays
  identical for JSON callers.
- CLI builds a Progress with SpinnerColumn + BarColumn + MofNCompleteColumn
  + TimeElapsedColumn, uses it as a transient display, and wires the
  callback to update the current task.  The final completed value is set
  from applied_count + error_count after the batch returns.
- Test coverage: new test_describe_batch_human_mode_wires_progress_callback
  asserts the callable is passed in human mode; the existing JSON test
  now asserts progress_callback=None to prevent regressions.
Dedicated workflow doc mirroring the structure of storage-files-workflow.md:

- Overview and when to use.
- Storage model: bucket/table descriptions via KBC.description, column
  descriptions via KBC.column.{name}.description on the table's metadata.
- Single-item examples for describe-bucket / describe-table / describe-column
  with --text, --file, --stdin and read-back via bucket-detail / table-detail.
- Batch YAML schema with a full example file covering all three sections.
- describe-batch partial-failure semantics (non-zero exit only when
  error_count > 0; always inspect errors[]).
- Precedence between the native description field and the KBC.description
  metadata entry (metadata wins; system providers ignored).
- End-to-end onboarding example.
Post-commit ruff format reordered 'from collections.abc import Callable'
to come before 'from pathlib import Path' (stdlib import ordering).
No functional change.
@ottomansky

Copy link
Copy Markdown
Contributor Author

Required fix + all four nice-to-haves:

  • docs(references): add storage describe-* commands to commands-reference33ad4be.
  • test(storage): cover metadata-extraction in get_bucket_detail/get_table_detail6bcdfb7 — 3 new tests. Pinned precedence: KBC.description metadata (provider=user) wins over native description for buckets; non-user providers (e.g. system) ignored on read-back. Native field is fallback when no user metadata entry exists. get_table_detail consults only KBC.description in metadata.
  • docs(gotchas): document storage description workarounds and precedence0e9078aKBC.column.{name}.description convention + describe-batch partial-failure semantics + precedence rule.
  • feat(storage): add progress spinner to describe-batch human mode2002b47 — Rich Progress with bar/spinner/time. StorageService.describe_batch gained an optional progress_callback: Callable[[str, str, int, int], None] | None = None; JSON mode unchanged.
  • docs(references): add storage-describe-workflow.md0955c1b — overview, single-item examples, batch YAML schema, read-back via bucket-detail/table-detail, partial-failure semantics.
  • style(storage): reorder imports in storage_service per ruffe0a8824 — ruff auto-fix for import ordering.

One signature note: describe-batch uses --from-file PATH (not --file PATH as in your template); updated the docs to match the real @typer.Option.

Targeted tests: 139/139 pass (126 across test_storage_describe_* + 13 client metadata tests). make lint format-check skill-check version-check changelog-check all green.

@ottomansky
ottomansky requested a review from padak April 23, 2026 10:39
@padak
padak changed the base branch from main to release/0.22.0 April 23, 2026 11:56
padak added a commit that referenced this pull request Apr 23, 2026
New: storage describe-bucket/table/column -- set descriptions via KBC
metadata (provider=user); readable via bucket-detail / table-detail.

New: storage describe-batch --from-file YAML -- apply bucket/table/column
descriptions in one shot; failures collected, remaining items continue.

Fix: bucket-detail / table-detail now return description + metadata fields;
KBC.description metadata wins over native bucket description field.

Max's review fixes: commands-reference docs, metadata-extraction tests
pinning precedence, gotchas for description conventions, progress spinner
for describe-batch human mode, storage-describe-workflow.md.
@padak

padak commented Apr 23, 2026

Copy link
Copy Markdown
Member

Integrated into release/0.22.0 as squash commit 105f74b (rebased locally on top of v0.21.2 to resolve merge conflicts with main; all Max's review fixes preserved). Original branch fiia/... unchanged.

Full integration PR coming as release/0.22.0 -> main.

@padak padak closed this Apr 23, 2026
ottomansky pushed a commit to ottomansky/keboola-agent-cli that referenced this pull request Apr 30, 2026
…tion

Three PRs' changelog entries were dropped during the git merge --squash
conflict resolution of release/0.22.0:

- keboola#201 ErrorCode enum + sync init --adopt-existing (3 entries)
- keboola#202 queue polling parity (5 entries)
- keboola#205 storage describe-bucket/table/column/batch (6 entries)

Without this fix, 'kbagent changelog' on installed 0.22.0 would show
only 18 of the 32 features shipped in this release. Adding the missing
14 entries before tagging v0.22.0.
@ottomansky
ottomansky deleted the fiia/pr7-storage-descriptions branch May 11, 2026 15:15
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.

2 participants