[Iceberg 1.11] Update REST spec#3687
[Iceberg 1.11] Update REST spec#3687adutra wants to merge 1 commit intoapache:feature/iceberg-1.11from
Conversation
| .createNamespace( | ||
| catalog, | ||
| createNamespaceRequest, | ||
| UUID.randomUUID(), |
There was a problem hiding this comment.
Note: random UUIDs are not UUID v7! This will have to be changed if/when we decide to support idempotency keys.
There was a problem hiding this comment.
That's fine IMO. We can revisit later, when needed. Mind adding a TODO?
There was a problem hiding this comment.
Or you use a
static final UUID NULL_UUID = new UUID(0L, 0L);
snazy
left a comment
snazy
left a comment
There was a problem hiding this comment.
LGTM - I'd advocate for the "NULL_UUID" (or whatever other fancy name)
snazy
left a comment
snazy
left a comment
There was a problem hiding this comment.
The spec-file changes LGTM.
I think there are a couple of changes that deserve feature/configuration toggles/guard-rails and others seem to deserve code changes. But I think those should be done separately.
| type: integer | ||
| minimum: 1 | ||
| maximum: 2 | ||
| maximum: 3 |
There was a problem hiding this comment.
Should we potentially add some safeguards to allow/disallow v3? (in a separate PR, of course)
| type: string | ||
| metadata-location: | ||
| type: string | ||
| overwrite: |
There was a problem hiding this comment.
This one cries for a feature/configuration toggle?
There was a problem hiding this comment.
See: apache/iceberg#15248
It's optional, so I think we are fine.
| message: The given namespace already exists | ||
| type: AlreadyExistsException | ||
| code: 409 | ||
| NamespaceNotEmptyError: |
There was a problem hiding this comment.
This new one seems to demand other code changes.
There was a problem hiding this comment.
Good point, I'll investigate.
This isn't the definitive spec for 1.11, so it will have to be updated again later.
adutra
force-pushed
the
feature/iceberg-1.11-update-rest-spec
branch
from
5a906cc to
c8bf29c
Compare
github-project-automation
bot
moved this from PRs In Progress
to Ready to merge
in Basic Kanban Board
2 participants
This isn't the definitive spec for 1.11, so it will have to be updated again later.
Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)