feat: generic tables, format-version policy, maintenance summary#194
Merged
Conversation
- regrouped warehouse add/catalog-settings dialog under a single 3-col General Settings block (soft deletion + iceberg format policy) - TaskManager: added per-entity maintenance summary card (last/next per queue + Run now / Reschedule / Cancel via controlTasks) - WarehousesNavigationTree: added pickable mode with `+` row action emitting `pick`; only-full-page tokens drive Load more; strip stale load-more on cache restore - NamespaceTables: brand format icons (Iceberg/Lance/Delta/Vortex) with mdi-alpha-g fallback - TableCreate / TableRegister: roadmap tab for generic table format - regenerated management openapi types Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 35 minutes and 1 second. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds comprehensive generic-table support to the console-components library, including new OpenAPI specifications for data-plane and management APIs, permission infrastructure for authorization and catalog actions, backend API integration, reusable permission composables, new generic-table UI components, updates to existing table and warehouse views, task and navigation enhancements, and public API exports. ChangesGeneric Tables Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/TableRegister.vue (1)
467-473:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
formatTabinresetForm()to avoid sticky tab state.If the dialog is closed on Generic, reopening keeps Generic selected. Reset this to
'iceberg'in Line 467.Suggested patch
function resetForm() { + formatTab.value = 'iceberg'; tableEntries.value = [createEmptyEntry()]; registrationDone.value = false; totalAttempted.value = 0; succeededCount.value = 0; importError.value = ''; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TableRegister.vue` around lines 467 - 473, The resetForm() function currently reinitializes form state but forgets to reset the formatTab reactive value, causing the dialog to reopen with the previously selected tab (e.g., Generic); update resetForm() to set formatTab to 'iceberg' so the tab selection is cleared when the form/dialog is reset—modify the resetForm function (search for resetForm and formatTab in TableRegister.vue) to assign formatTab.value = 'iceberg' alongside the other resets.src/components/TableCreate.vue (1)
334-339:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
formatTabwhen closing the dialog.After closing on the Generic tab, reopening keeps the previous tab selection. Line 334 should also reset
formatTabto'iceberg'so the dialog always opens in its default mode.Suggested patch
function resetForm() { + formatTab.value = 'iceberg'; tableName.value = ''; fields.value = []; error.value = null; success.value = false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/TableCreate.vue` around lines 334 - 339, The resetForm function currently clears tableName, fields, error, and success but doesn't reset the dialog tab state; update resetForm to also reset formatTab to its default by setting formatTab (the reactive holding the tab, e.g., formatTab.value) to 'iceberg' so the dialog always opens on the default Generic tab; locate resetForm in TableCreate.vue and add the assignment alongside the other resets.src/components/WarehousesNavigationTree.vue (1)
557-616:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSearch result clicks ignore
pickablemode and still navigate.In pickable mode, tree row clicks are suppressed, but search-result clicks still emit
navigate. This can break resource-picker flow by routing instead of selecting.Suggested patch
async function navigateToSearchResult(result: { id: string; name: string; namespace: string; type: string; warehouseId: string; namespaceId: string; }) { + if (props.pickable) { + emit('pick', { + type: result.type, + warehouseId: result.warehouseId, + namespaceId: result.namespaceId, + name: result.name, + id: result.id, + }); + dismissSearch(); + return; + } + const apiNamespace = namespacePathToApiFormat(result.namespaceId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/WarehousesNavigationTree.vue` around lines 557 - 616, navigateToSearchResult currently always emits a 'navigate' event and ignores the component's pickable mode, causing search clicks to route instead of selecting; update navigateToSearchResult to check the pickable prop/flag (e.g. this.pickable or props.pickable) and if pickable is true, do not emit 'navigate' — instead emit the appropriate selection event (e.g. 'pick' or 'select') with the result payload (id, type, warehouseId, namespaceId, name) or simply return after setting any UI state, otherwise proceed with the existing permission checks, tree expansion, saveWarehouseSubtreeState, dismissSearch and emit('navigate', ...). Ensure you reference the navigateToSearchResult function and the emit call so the conditional is placed before emitting navigation.
🧹 Nitpick comments (3)
openapi/generic-table-open-api.yaml (1)
279-282: 💤 Low valueConsider adding
maxItemsconstraint to arrays (static analysis hint).Static analysis flagged that the
stackarray lacks amaxItemsconstraint. The same applies to other arrays likeidentifiers(line 349) andstorage-credentials(line 362). Adding bounds can provide defense-in-depth against malformed responses.Since this spec is downloaded from the upstream lakekeeper repo via the
update-openapi-generic-tablerecipe, any changes would need to be coordinated there first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/generic-table-open-api.yaml` around lines 279 - 282, The OpenAPI arrays (the "stack" array and the other array schemas "identifiers" and "storage-credentials") lack a defensive maxItems bound; update each array schema definition to include a reasonable maxItems value (e.g., based on domain expectations) so consumers and linters have an upper bound, by adding a maxItems field alongside the existing type/items in the "stack", "identifiers" and "storage-credentials" definitions; coordinate this change with the upstream lakekeeper spec since this file is generated from the update-openapi-generic-table recipe.Source: Linters/SAST tools
openapi/management-open-api.yaml (2)
4063-4075: ⚡ Quick winModel generic-table identifiers explicitly in the check schemas.
Both new
generic-tablebranches still inherittable-id/tablefromTabularIdentOrUuid. OpenAPI generators will ignore the alias note in the description, so regenerated clients will expose ageneric-tablepayload with table-shaped keys instead ofgeneric-table-id/generic-table. If generic-table checks are meant to be first-class in the generated SDK, add explicit generic-table variants instead of documenting aliases in prose.Also applies to: 4196-4208, 8090-8094
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/management-open-api.yaml` around lines 4063 - 4075, The OpenAPI schema for the "generic-table" branch currently composes from TabularIdentOrUuid, which carries properties named table-id/table that will be exposed by generators despite the prose alias; update the schemas to model explicit generic-table identifier variants instead of inheriting TabularIdentOrUuid. Concretely, replace or augment the allOf usage for the "generic-table" property (the block referencing TabularIdentOrUuid and LakekeeperGenericTableAction) with a dedicated schema that defines a generic-table-id (or generic-table) property name and its type, and ensure corresponding schemas at the other occurrences (the blocks around line ranges 4196-4208 and 8090-8094) are similarly updated so generated clients receive explicit generic-table identifier fields rather than relying on description aliases.
4419-4438: ⚡ Quick winEncode the format-version bounds in the schema.
These fields are only constrained in the description right now. Generated clients will therefore accept
[], duplicates, or unsupported values like999as type-valid input. AddingminItems: 1,uniqueItems: true, andenum: [1, 2, 3]here would make the regenerated contract match the documented rules.Suggested schema tightening
allowed-format-versions: type: array + minItems: 1 + uniqueItems: true items: type: integer format: int32 + enum: [1, 2, 3] default-format-version: type: - integer - 'null' format: int32 + enum: [1, 2, 3, null]Also applies to: 8424-8440
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@openapi/management-open-api.yaml` around lines 4419 - 4438, The schema for allowed-format-versions and default-format-version is only documented but not enforced; update the OpenAPI schema for the property named allowed-format-versions to include minItems: 1, uniqueItems: true, and constrain the items with enum: [1, 2, 3] (rather than only type/format) so clients cannot pass empty lists, duplicates, or unsupported numbers; also add an enum: [1, 2, 3] to default-format-version (keeping its nullable/integer type) so the default value is one of the allowed versions; make the same changes for the other occurrence referenced in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openapi/management-open-api.yaml`:
- Around line 5298-5316: The schema exposes "default-format-version" as nullable
and forces clients to reimplement the fallback; change the API to return the
resolved default by making "default-format-version" non-nullable/required in the
warehouse response and ensure the server computes this value server-side using
the existing policy (resolve to v2 if present in "allowed-format-versions",
otherwise the highest allowed version). Update the OpenAPI schema entry for
allowed-format-versions and default-format-version so default-format-version is
required and documented as the effective, resolved value, and verify the
warehouse read/detail handlers produce that resolved integer rather than leaving
it null.
In `@src/components/GenericTableOverview.vue`:
- Around line 124-126: Replace raw console.error calls in the
GenericTableOverview.vue catch blocks with the shared handleError(...) handler:
call handleError(error, '<context string>', true) so the user gets consistent
snackbar feedback. Specifically, update the catch in the generic-table data
loader to use handleError(error, 'loading generic table data', true) and update
the catch handling protection failures (the other occurrence) to use
handleError(error, 'checking table protection', true); reference the handleError
function and the GenericTableOverview component when locating these catch
blocks.
- Around line 103-124: loadTableData currently assigns response.table into the
existing table object and only updates genericTableId when a match is found,
which can leave stale table fields and an incorrect genericTableId if the lookup
misses; before assigning new table data and before calling listGenericTables,
clear or reset table (e.g., replace with an empty object or reset known keys)
and set genericTableId.value = undefined (or null) to ensure stale IDs are not
reused, then Object.assign(table, response.table) and proceed with the
listGenericTables lookup and setting of genericTableId; ensure
recursiveDeleteProtection.value is set explicitly from response.table.protected
after the table reset.
- Around line 129-142: The catch block in setProtection currently toggles
recursiveDeleteProtection.value on failure which misrepresents the real state;
before calling functions.setGenericTableProtection (inside setProtection)
capture the current state (e.g., const previous =
recursiveDeleteProtection.value), use pendingProtectionValue.value to
optimistically update or set after success, and on error restore
recursiveDeleteProtection.value = previous (instead of assigning
!recursiveDeleteProtection.value); reference setProtection,
recursiveDeleteProtection, pendingProtectionValue, and
functions.setGenericTableProtection to locate and implement this change.
In `@src/components/NamespaceGenericTables.vue`:
- Around line 212-214: Replace the console.error in the catch block for the
delete/drop flow with the shared error handler: call
functions.handleError(error, `Failed to drop generic-table ${item.name}`, true)
from the same method (the delete/drop function in NamespaceGenericTables.vue),
ensuring the local `functions` object is imported or available in the component
setup; this routes the error through the standard handler and shows user
notification instead of logging to console.
- Line 149: The watcher only reacts to props.namespacePath so changes to the
warehouse route param can leave data stale; update the watcher to watch both
props.namespacePath and props.warehouseId (or a computed tuple like () =>
[props.namespacePath, props.warehouseId]) and call loadGenericTables when either
changes, ensuring the existing loadGenericTables function is reused and the
watcher registration (watch(..., loadGenericTables)) is updated accordingly.
In `@src/components/NamespaceTables.vue`:
- Around line 285-287: Replace the direct console.error call in the delete/drop
catch block with the standardized error handler: call handleError(error, `Failed
to drop ${item.source}-${item.name}`, true) so the failure uses the app's
centralized error/context/notification flow; locate the catch in the drop/delete
method where item is available (in NamespaceTables.vue) and swap
console.error(...) for handleError(...) using that message and notify=true.
- Around line 268-273: The paginationCheck uses loadedRows.length (mixing
generic + Iceberg rows) so the Iceberg-only pagination trigger can fire too
early; change the check to use only the count of Iceberg rows (or a dedicated
icebergRows collection) instead of loadedRows.length. In paginationCheck (and
where loadIceberg and paginationToken are used) compute icebergCount (e.g.,
loadedRows.filter(r => r.source === 'iceberg').length) or maintain icebergRows
and then replace the condition `option.page * option.itemsPerPage ==
loadedRows.length && paginationToken.value != ''` with a check against
icebergCount (or icebergRows.length) so fetching more Iceberg items is based
solely on Iceberg rows.
In `@src/components/TaskManager.vue`:
- Around line 675-700: The catch blocks in runQueueNow and cancelQueueTask
currently swallow errors; update each catch to accept the error and call
handleError(error, '<action> queue task', true) (e.g., for runQueueNow use
handleError(error, 'run-now queue task', true) and for cancelQueueTask use
handleError(error, 'cancel queue task', true)); also apply the same pattern to
the other task-control functions in this component (the reschedule/cancel/resume
handlers around the other task-control blocks) so all API errors route through
handleError(error, <context>, true).
- Around line 709-714: The current openReschedule function slices the UTC ISO
string (iso.slice(0,16)) and places that value into rescheduleAt, which treats
it as local time and can shift the intended instant; instead parse the stored
ISO timestamp into a Date and format a local "YYYY-MM-DDTHH:MM" string (using
getFullYear/getMonth/getDate/getHours/getMinutes) and assign that to
rescheduleAt.value so the datetime-local input reflects the correct local wall
time; keep rescheduleTarget.value and rescheduleDialog.value assignments
unchanged and update any helper or inline logic in openReschedule to produce the
local-formatted string.
In `@src/components/WarehouseAddDialog.vue`:
- Around line 358-361: The helper pickDefaultFromAllowed currently calls
Math.max(...allowed) which returns -Infinity for an empty array; change
pickDefaultFromAllowed to guard for an empty allowed (e.g., if allowed.length
=== 0) and return a safe fallback (for example 2) otherwise keep the existing
logic (prefer 2 if present, else Math.max). Apply the same empty-array guard to
the other places in this file where Math.max(...allowed) is used to compute
default format versions so you don't propagate -Infinity into
default-format-version/formatPolicy.default in update/create payloads.
In `@src/components/WarehouseHeader.vue`:
- Around line 170-187: The current batch update using Promise.all([...]) can
leave settings partially applied if one promise rejects and prevents
loadWarehouse() from running; change the pattern in the block that builds calls
(the calls array and invocations of functions.updateWarehouseDeleteProfile and
functions.setWarehouseFormatVersionPolicy) to use Promise.allSettled(calls),
iterate the results to detect which updates succeeded or failed, surface precise
error(s) to the user (or log them) based on the rejected entries, and always
call loadWarehouse() afterwards to reconcile UI state with backend; ensure the
same change is applied to the similar block around lines 195-197.
- Line 196: Replace the raw console.error call in the catalog-settings failure
path with the shared error handler: call handleError(...) instead of
console.error('Failed to update catalog settings:', error), passing the caught
error object plus a clear context string (e.g. "Failed to update catalog
settings") and the notify flag as appropriate so the failure in the
WarehouseHeader component routes through the centralized handler.
In `@src/composables/useAuthorizerPermissions.ts`:
- Around line 690-696: In loadPermissions(), when genericTableIdRef.value or
warehouseIdRef.value are missing the function currently returns early but leaves
permissions populated; update loadPermissions (referencing genericTableIdRef,
warehouseIdRef, permissions, and config.enabledPermissions.value) to explicitly
clear permissions.value = [] before returning for the missing-ID case so stale
grants are not shown; keep the existing branch that clears permissions when
config.enabledPermissions.value is false.
In `@src/composables/useCatalogPermissions.ts`:
- Around line 750-752: In loadPermissions(), when the early-return triggers
because genericTableIdRef.value or warehouseIdRef.value is unset, clear the
cached permissions so stale values don't persist; update the branch that
currently returns early to reset the permissions state (e.g., set
permissions/permissionsRef to an empty value or initial state) before returning.
Target the loadPermissions function and the genericTableIdRef, warehouseIdRef,
and permissions (or permissionsRef) symbols so the change is applied where the
IDs are checked and the function currently exits without clearing permissions.
In `@src/stores/permissions.ts`:
- Around line 97-99: Replace the direct console.error call when whId is missing
with the centralized error handler: call functions.handleError(...) passing a
descriptive Error (or message) mentioning "Failed to load generic table
permissions: warehouseId not available", include relevant context (e.g.,
whId/permissions load action) and set notify=true (or the appropriate flag) so
the snackbar pipeline is used; keep the early return of [] in the same place.
Target the block that checks whId in the permissions loader function in
src/stores/permissions.ts and update it to use functions.handleError instead of
console.error.
---
Outside diff comments:
In `@src/components/TableCreate.vue`:
- Around line 334-339: The resetForm function currently clears tableName,
fields, error, and success but doesn't reset the dialog tab state; update
resetForm to also reset formatTab to its default by setting formatTab (the
reactive holding the tab, e.g., formatTab.value) to 'iceberg' so the dialog
always opens on the default Generic tab; locate resetForm in TableCreate.vue and
add the assignment alongside the other resets.
In `@src/components/TableRegister.vue`:
- Around line 467-473: The resetForm() function currently reinitializes form
state but forgets to reset the formatTab reactive value, causing the dialog to
reopen with the previously selected tab (e.g., Generic); update resetForm() to
set formatTab to 'iceberg' so the tab selection is cleared when the form/dialog
is reset—modify the resetForm function (search for resetForm and formatTab in
TableRegister.vue) to assign formatTab.value = 'iceberg' alongside the other
resets.
In `@src/components/WarehousesNavigationTree.vue`:
- Around line 557-616: navigateToSearchResult currently always emits a
'navigate' event and ignores the component's pickable mode, causing search
clicks to route instead of selecting; update navigateToSearchResult to check the
pickable prop/flag (e.g. this.pickable or props.pickable) and if pickable is
true, do not emit 'navigate' — instead emit the appropriate selection event
(e.g. 'pick' or 'select') with the result payload (id, type, warehouseId,
namespaceId, name) or simply return after setting any UI state, otherwise
proceed with the existing permission checks, tree expansion,
saveWarehouseSubtreeState, dismissSearch and emit('navigate', ...). Ensure you
reference the navigateToSearchResult function and the emit call so the
conditional is placed before emitting navigation.
---
Nitpick comments:
In `@openapi/generic-table-open-api.yaml`:
- Around line 279-282: The OpenAPI arrays (the "stack" array and the other array
schemas "identifiers" and "storage-credentials") lack a defensive maxItems
bound; update each array schema definition to include a reasonable maxItems
value (e.g., based on domain expectations) so consumers and linters have an
upper bound, by adding a maxItems field alongside the existing type/items in the
"stack", "identifiers" and "storage-credentials" definitions; coordinate this
change with the upstream lakekeeper spec since this file is generated from the
update-openapi-generic-table recipe.
In `@openapi/management-open-api.yaml`:
- Around line 4063-4075: The OpenAPI schema for the "generic-table" branch
currently composes from TabularIdentOrUuid, which carries properties named
table-id/table that will be exposed by generators despite the prose alias;
update the schemas to model explicit generic-table identifier variants instead
of inheriting TabularIdentOrUuid. Concretely, replace or augment the allOf usage
for the "generic-table" property (the block referencing TabularIdentOrUuid and
LakekeeperGenericTableAction) with a dedicated schema that defines a
generic-table-id (or generic-table) property name and its type, and ensure
corresponding schemas at the other occurrences (the blocks around line ranges
4196-4208 and 8090-8094) are similarly updated so generated clients receive
explicit generic-table identifier fields rather than relying on description
aliases.
- Around line 4419-4438: The schema for allowed-format-versions and
default-format-version is only documented but not enforced; update the OpenAPI
schema for the property named allowed-format-versions to include minItems: 1,
uniqueItems: true, and constrain the items with enum: [1, 2, 3] (rather than
only type/format) so clients cannot pass empty lists, duplicates, or unsupported
numbers; also add an enum: [1, 2, 3] to default-format-version (keeping its
nullable/integer type) so the default value is one of the allowed versions; make
the same changes for the other occurrence referenced in the comment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84b45e35-023e-4a1a-8fc5-0b1e5b1154ff
⛔ Files ignored due to path filters (31)
src/assets/delta.svgis excluded by!**/*.svgsrc/assets/iceberg.svgis excluded by!**/*.svgsrc/assets/lance.pngis excluded by!**/*.pngsrc/assets/vortex_logo.svgis excluded by!**/*.svgsrc/assets/vortex_logo_dark_theme.svgis excluded by!**/*.svgsrc/gen/generic-table/client.gen.tsis excluded by!**/gen/**src/gen/generic-table/client/client.gen.tsis excluded by!**/gen/**src/gen/generic-table/client/index.tsis excluded by!**/gen/**src/gen/generic-table/client/types.gen.tsis excluded by!**/gen/**src/gen/generic-table/client/utils.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/auth.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/bodySerializer.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/params.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/pathSerializer.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/queryKeySerializer.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/serverSentEvents.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/types.gen.tsis excluded by!**/gen/**src/gen/generic-table/core/utils.gen.tsis excluded by!**/gen/**src/gen/generic-table/index.tsis excluded by!**/gen/**src/gen/generic-table/sdk.gen.tsis excluded by!**/gen/**src/gen/generic-table/types.gen.tsis excluded by!**/gen/**src/gen/iceberg/client/types.gen.tsis excluded by!**/gen/**src/gen/iceberg/core/params.gen.tsis excluded by!**/gen/**src/gen/iceberg/index.tsis excluded by!**/gen/**src/gen/iceberg/sdk.gen.tsis excluded by!**/gen/**src/gen/iceberg/types.gen.tsis excluded by!**/gen/**src/gen/management/client/types.gen.tsis excluded by!**/gen/**src/gen/management/core/params.gen.tsis excluded by!**/gen/**src/gen/management/index.tsis excluded by!**/gen/**src/gen/management/sdk.gen.tsis excluded by!**/gen/**src/gen/management/types.gen.tsis excluded by!**/gen/**
📒 Files selected for processing (28)
justfileopenapi/generic-table-open-api.yamlopenapi/management-open-api.yamlopenapi/rest-catalog-open-api.yamlsrc/common/enums.tssrc/common/interfaces.tssrc/common/permissionActions.tssrc/components/GenericTableHeader.vuesrc/components/GenericTableOverview.vuesrc/components/NamespaceGenericTables.vuesrc/components/NamespaceTables.vuesrc/components/PermissionAssignDialog.vuesrc/components/PermissionManager.vuesrc/components/PermissionMatrixDialog.vuesrc/components/TableCreate.vuesrc/components/TableRegister.vuesrc/components/TaskDetails.vuesrc/components/TaskManager.vuesrc/components/WarehouseActionsMenu.vuesrc/components/WarehouseAddDialog.vuesrc/components/WarehouseDetails.vuesrc/components/WarehouseHeader.vuesrc/components/WarehousesNavigationTree.vuesrc/composables/useAuthorizerPermissions.tssrc/composables/useCatalogPermissions.tssrc/index.tssrc/plugins/functions.tssrc/stores/permissions.ts
Comment on lines
+5298
to
+5316
| - allowed-format-versions | ||
| properties: | ||
| allowed-format-versions: | ||
| type: array | ||
| items: | ||
| type: integer | ||
| format: int32 | ||
| description: |- | ||
| Iceberg table format versions that may be created in, or upgraded to, | ||
| within this warehouse. | ||
| default-format-version: | ||
| type: | ||
| - integer | ||
| - 'null' | ||
| format: int32 | ||
| description: |- | ||
| Default Iceberg table format version applied when a create-table request | ||
| does not specify one. When absent, resolves to v2 if allowed, otherwise | ||
| the highest allowed version. |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Return the resolved default-format-version in warehouse responses.
The response currently makes every consumer reimplement the fallback rules from prose to know the effective default. Since this PR surfaces the policy in read-only/detail flows, that invites drift between clients and server behavior. Prefer making default-format-version required once the warehouse policy has been normalized server-side.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openapi/management-open-api.yaml` around lines 5298 - 5316, The schema
exposes "default-format-version" as nullable and forces clients to reimplement
the fallback; change the API to return the resolved default by making
"default-format-version" non-nullable/required in the warehouse response and
ensure the server computes this value server-side using the existing policy
(resolve to v2 if present in "allowed-format-versions", otherwise the highest
allowed version). Update the OpenAPI schema entry for allowed-format-versions
and default-format-version so default-format-version is required and documented
as the effective, resolved value, and verify the warehouse read/detail handlers
produce that resolved integer rather than leaving it null.
- centralize error handling: replace console.error with functions.handleError in drop flows (NamespaceTables / NamespaceGenericTables), task-control actions (TaskManager runQueueNow / cancelQueueTask / stopTask / confirmCancelTask / confirmRunTaskNow), generic-table load + setProtection, catalog-settings update (WarehouseHeader), and the missing-warehouseId path in stores/permissions.ts - GenericTableOverview: clear stale `table` + `genericTableId` before reload; setProtection captures previous state and restores on failure instead of toggling - NamespaceGenericTables: watcher now reacts to warehouseId too - NamespaceTables: paginationCheck triggers on icebergCount (filtering by source) so generic rows no longer break the Iceberg page boundary - TaskManager: openReschedule formats the existing scheduled-for as a local wall-clock string for `datetime-local`; slicing UTC ISO shifted the value by the user's offset - WarehouseAddDialog: pickDefaultFromAllowed guards empty input so Math.max(...[]) cannot leak -Infinity into the payload - WarehouseHeader: updateCatalogSettings uses Promise.allSettled so a partial failure still reloads the warehouse and reports per-call failures - useAuthorizer/useCatalogPermissions (generic-table): clear permissions before the missing-id early return so stale grants do not leak across route changes - TableCreate / TableRegister: resetForm also resets formatTab to 'iceberg' so the dialog reopens on the default tab - WarehousesNavigationTree: navigateToSearchResult honors `pickable` and emits `pick` instead of `navigate` so Cedar Resolve selects from search Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The detail pages (TableOverview, ViewOverview, generic-table page) already render a friendly not-found / forbidden alert via pageError, so calling loadX before navigating just adds a redundant round-trip and a less informative snackbar — and worst case it blocks the user from even seeing the resource breadcrumbs. Removed the pre-check + snackbar early-return from: - NamespaceTables.routeToRow (unified Iceberg + generic flow) - WarehousesNavigationTree.handleNavigate (table/view/generic-table) - WarehousesNavigationTree.navigateToSearchResult Click → page opens → detail page handles 403/404 inline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
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
1 participant
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
/lakekeeper/v1/.../generic-tablesSDK, plus management-side actions/assignments/protection. Unified list with format icons insideNamespaceTables, detail page, tree integration, brand icons.WarehouseDetails; editable in the create-warehouse flow and the renamed "Catalog Settings" update dialog.TaskManager— per-queue last-run / next-run cards with Run now / Reschedule / Cancel via existingcontrolTasks. Shown when the manager is scoped to a single entity.pickablemode onWarehousesNavigationTree—+button per row for resource picking (used by Cedar Resolve in console-plus-components).generic-table-open-api.yaml; SDKs regenerated.Test plan
Notes
ObjectType.DELETION_PROFILEwas renamed toObjectType.CATALOG_SETTINGS. The new dialog covers both deletion and format-version policy.doc/base-locationare write-once at create time. UI displays them read-only on the detail page;TableCreate/TableRegistershow a "Roadmap" tab for the Generic format.BEGIN_COMMIT_OVERRIDE
feat(ui): add generic-table (Lance/Delta/Vortex/…) support across navigation, list, detail, and search
feat(ui): unify Iceberg + generic tables under one Tables tab with format icons and type filter
feat(ui): add Iceberg format-version policy editor to warehouse create and "Catalog Settings" dialogs
feat(ui): rename "Change Deletion" to "Catalog Settings" combining deletion + format policy in one update
feat(ui): add per-queue maintenance summary (last/next/run-now/reschedule/cancel) inside TaskManager
feat(ui): add `pickable` mode + `+` action button to WarehousesNavigationTree
chore(ui): refresh OSS OpenAPI specs and regenerate management/iceberg/generic-table SDKs
END_COMMIT_OVERRIDE
Summary by CodeRabbit