danielcweeks commented on code in PR #17642:
URL: https://github.com/apache/iceberg/pull/17642#discussion_r4543305079
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1039,6 +1039,11 @@ paths:
Load a table from the catalog.
+ This endpoint must not return a format version 4 table.
+ A server must not serve a format version 4 table through this
endpoint, so that a client that does not support format version 4 is never
handed a response it cannot read.
+ Format version 4 tables are loaded through the `/v2` endpoint instead.
Review Comment:
The first two sentences also say the same thing, and the text doesn't say
what a server should return instead (400?).
Can we remove these lines?
```suggestion
Servers MUST NOT return format version 4 tables from this endpoint.
```
<!-- lenses: weeks-review, blue-review, russell-review -->
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -1299,6 +1304,91 @@ paths:
5XX:
$ref: '#/components/responses/ServerErrorResponse'
+ /v2/{prefix}/namespaces/{namespace}/tables/{table}:
+ parameters:
+ - $ref: '#/components/parameters/prefix'
+ - $ref: '#/components/parameters/namespace'
+ - $ref: '#/components/parameters/table'
+
+ get:
+ tags:
+ - Catalog API
+ summary: Load a table from the catalog
+ operationId: loadTableV2
+ description:
+ Load a table from the catalog.
+
+
+ This endpoint returns the same response as the v1 `loadTable` endpoint
and may return a table at any format version, including format version 4.
+ Clients that support format version 4 should load tables through this
endpoint.
+
+
+ The response contains both configuration and table metadata. The
configuration, if non-empty is used
+ as additional configuration for the table that overrides catalog
configuration. For example, this
+ configuration may change the FileIO implementation to be used for the
table.
+
+
+ The response also contains the table's full metadata, matching the
table metadata JSON file.
+
+
+ The catalog configuration may contain credentials that should be used
for subsequent requests for the
+ table. The configuration key "token" is used to pass an access token
to be used as a bearer token
+ for table requests. Otherwise, a token may be passed using a RFC 8693
token type as a configuration
+ key. For example, "urn:ietf:params:oauth:token-type:jwt=<JWT-token>".
+ parameters:
+ - $ref: '#/components/parameters/data-access'
+ - name: If-None-Match
+ in: header
+ description:
+ An optional header that allows the server to return 304 (Not
Modified) if the metadata
+ is current. The content is the value of the ETag received in a
CreateTableResponse,
+ LoadTableResponse or CommitTableResponse.
+ required: false
+ schema:
+ type: string
+ - in: query
+ name: snapshots
+ description:
+ The snapshots to return in the body of the metadata via the
`snapshots` field. Setting
+ the value to `all` would return the full set of snapshots
currently valid for the table.
+ Setting the value to `refs` would load all snapshots referenced by
branches or tags.
+
Review Comment:
nit: if this endpoint stays, the `If-None-Match` and `snapshots` parameters
and the description block are copied from v1 and will drift as soon as someone
edits the v1 text. Can we define the parameters once in
`components/parameters`, `$ref` them from both load operations, and point to
the v1 description instead of repeating it?
This should be a compatible change even if we touch the older v1 endpoints.
<!-- lenses: weeks-review, blue-review -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]