feat(storage): surface the typed-table definition in table-detail (#621) - #630
Closed
padak wants to merge 1 commit into
Closed
feat(storage): surface the typed-table definition in table-detail (#621)#630padak wants to merge 1 commit into
padak wants to merge 1 commit into
Conversation
`storage create-table` can apply BigQuery `timePartitioning`,
`rangePartitioning` and `clustering`, and `storage swap-tables` promotes the
result into place -- but nothing could read that layout back.
`StorageService.get_table_detail()` built its response from an explicit field
allowlist that dropped `definition`, even though the underlying
`GET /v2/storage/tables/{id}` returns it for typed tables with no `include=`
parameter. The write half of the repartition flow was supported and the verify
half was not.
That gap matters because the table ID is unchanged whether the swap happened or
not, so the layout is the only field that tells a completed repartition from a
failed one -- and on a Keboola-managed BigQuery project without
`bigquery.jobs.create`, the Storage `definition` is the only reachable view of
the registered layout.
- service: `definition` passes through verbatim, `None` for untyped tables.
Deliberately detail-only -- the LIST endpoint does not return it.
- command: human mode gains `Partitioning:` / `Clustering:` rows between the
primary key and last-import lines, emitted only when a layout exists, so
Snowflake and untyped-table output stays byte-identical.
- `commands/storage.py` is past its grandfathered line budget, so the
table-detail human renderer moved to a private `_storage_format` module
alongside the new layout formatter. Net effect is a 22-line shrink.
The formatter type-guards every level: a Storage API deploy once served
`definition` as `[]` (see tests/test_storage_empty_definition.py) and that
crash class must not come back.
Member
Author
|
Closing as a duplicate of #629, which was opened concurrently and is the stronger implementation: it also surfaces Opened by the daily issue-triage run before #629 existed; no reviewer time was spent on it. |
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.
What
storage table-detailnow returns the table'sdefinition— the typed-table layoutcarrying
primaryKeysNamesand, on BigQuery,timePartitioning,rangePartitioningand
clustering.nullfor an untyped table.Why
kbagent could already write that layout (
storage create-table --source-table-id --time-partitioning-field --clustering-field, promoted withstorage swap-tables) buthad no way to read it back.
StorageService.get_table_detail()built its response froman explicit field allowlist that dropped
definition, even thoughGET /v2/storage/tables/{id}returns it for typed tables with noinclude=parameter.The write half of the repartition flow was supported; the verify half was not.
That matters because the table ID is unchanged whether the swap happened or not — the
layout is the only field that distinguishes a completed repartition from a failed one.
On a Keboola-managed BigQuery project where the caller holds no
bigquery.jobs.create,the Storage
definitionis also the only reachable view of the registered layout.Changes
service (
services/storage_service.py::get_table_detail) —definitionpassesthrough verbatim;
Noneon untyped tables, which needs no special-casing.Deliberately detail-only: the LIST endpoint genuinely does not return it, so
storage tableswould mean one detail request per table — a different change with adifferent cost profile (the issue calls this out as out of scope).
command — human mode gains
Partitioning:/Clustering:rows between theprimary key and last-import lines:
Rows are emitted only when a layout exists, so Snowflake and untyped-table output is
byte-identical. Ingestion-time partitioning (no
field) prints just the type.REST —
GET /storage/table-detail/...delegates to the service, so the routepicks the field up with no router change. No new command, so no
OPERATION_REGISTRYentry.
file-size budget —
commands/storage.pyis past its grandfathered ceiling, sorather than golf the diff, the table-detail human renderer moved into a private
commands/_storage_format.py(the same pattern as_storage_snapshots.py) togetherwith the new layout formatter. Net effect on
commands/storage.pyis a 22-lineshrink (2246 → 2224).
Robustness note
The formatter type-guards at every level (
isinstance(..., dict)on the definition andon each sub-object). A Storage API deploy once served
definitionas[]rather than anobject — the incident pinned by
tests/test_storage_empty_definition.py, which broke thelegacy Go CLI's strict decoder. A test asserts the renderer returns nothing for that shape
so the crash class cannot come back through this path.
Version
Bumped to 0.87.1 with a changelog entry. Renumber freely if it should land under a
different version.
Testing
make checkgreen: lint, format, typecheck, skill-check, version-check,command-sync-check, changelog-check, error-codes, sentinel-guards, loc-check, and
5711 tests passed.
tests/test_storage_table_definition.py: service passthrough (typed + untyped),seven formatter cases (no layout, non-dict
definition, time partitioning with andwithout
field, range partitioning with and without bounds, clustering), and three CLIcases (human output shows and orders the rows; human output unchanged without a layout;
--jsoncarriesdefinition).response shape follows the issue's reported v0.86.0 output and the existing
create-tablewrite-side field names.Note for the merge queue
Touches the same
get_table_detailfunction as #628 (issue #624). Whichever lands secondneeds a trivial rebase; there is no semantic overlap — #628 changes how column
descriptions are read, this one adds a field to the return dict.
Fixes #621