kbendick commented on a change in pull request #3955:
URL: https://github.com/apache/iceberg/pull/3955#discussion_r791416437
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -367,6 +367,90 @@ paths:
- $ref: '#/components/parameters/namespace'
- $ref: '#/components/parameters/table'
+ get:
+ tags:
+ - Catalog API
+ summary: Load a table from the catalog
+ operationId: loadTable
+ description:
+ Load a table from the catalog.
+
+ 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 implemented used for the table.
+
+ The response also contains the table's full metadata.
+ responses:
+ 200:
+ $ref: '#/components/responses/LoadTableResponse'
+ 400:
+ $ref: '#/components/responses/BadRequestErrorResponse'
+ 401:
+ $ref: '#/components/responses/UnauthorizedResponse'
+ 404:
+ description:
+ Not Found - NoSuchTableException, table to load does not exist
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/IcebergErrorResponse'
+ examples:
+ TableToRenameDoesNotExist:
+ $ref: '#/components/examples/NoSuchTableError'
+ 5XX:
+ $ref: '#/components/responses/ServerErrorResponse'
+
+ post:
+ tags:
+ - Catalog API
+ summary: Commit updates to a table
+ operationId: updateTable
+ description:
+ Commit updates to a table.
+
+ Commits have two parts, requirements and updates. Requirements are
assertions that will be validated
+ before attempting to make and commit changes. For example,
assert-ref-snapshot-id will check that a
+ named ref's snapshot ID has a certain value.
+
+ Updates are changes to make to table metadata. For example, after
asserting that the current main ref
+ is at the expected snapshot, a commit may add a new child snapshot and
set the ref to the new
+ snapshot id.
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/CommitTableRequest'
+ responses:
+ 200:
+ description: OK
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/CommitTableResponse'
+ 400:
+ $ref: '#/components/responses/BadRequestErrorResponse'
+ 401:
+ $ref: '#/components/responses/UnauthorizedResponse'
+ 404:
+ description:
+ Not Found - NoSuchTableException, table to load does not exist
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/IcebergErrorResponse'
+ examples:
+ TableToRenameDoesNotExist:
+ $ref: '#/components/examples/NoSuchTableError'
+ 409:
+ description:
+ Conflict - CommitFailedException, one or more requirements failed.
The client may retry.
Review comment:
In the previous PR, we put `50X` everywhere. I believe I also included a
section that mentions 50X can return text/plain (eg some middleware or proxy
server hit an issue and then returned an HTML page, which definitely happens
even once you hit the app sever).
I think we should include them everywhere, but I can understand possibly
adding them last as they’re the same throughout,
50X response would be maybe a good argument for using an external tool to
bundle all of the files together, though we’re likely far from being in that
position currently and should focus on defining the spec first.
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -564,12 +648,21 @@ components:
example: '{ "owner": "Hank Bendickson" }'
default: '{ }'
- Namespace:
Review comment:
Do we want to do this in another PR to reduce clutter? I agree they make
sense together, as there’s no section for `requests`.
I’d alphabetize actual schema elements and then separate them by a block
that does requests. I can open a PR for that if you’d like.
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -367,6 +367,90 @@ paths:
- $ref: '#/components/parameters/namespace'
- $ref: '#/components/parameters/table'
+ get:
+ tags:
+ - Catalog API
+ summary: Load a table from the catalog
+ operationId: loadTable
+ description:
+ Load a table from the catalog.
+
+ 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 implemented used for the table.
+
+ The response also contains the table's full metadata.
+ responses:
+ 200:
+ $ref: '#/components/responses/LoadTableResponse'
+ 400:
+ $ref: '#/components/responses/BadRequestErrorResponse'
+ 401:
+ $ref: '#/components/responses/UnauthorizedResponse'
+ 404:
+ description:
+ Not Found - NoSuchTableException, table to load does not exist
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/IcebergErrorResponse'
+ examples:
+ TableToRenameDoesNotExist:
Review comment:
Yeah this could be changed to simply `TableToLoadDoesNotExist`. The key
here in the examples map doesn’t have many requirements on it.
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -367,6 +367,90 @@ paths:
- $ref: '#/components/parameters/namespace'
- $ref: '#/components/parameters/table'
+ get:
+ tags:
+ - Catalog API
+ summary: Load a table from the catalog
+ operationId: loadTable
+ description:
+ Load a table from the catalog.
+
+ 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 implemented used for the table.
+
+ The response also contains the table's full metadata.
+ responses:
+ 200:
+ $ref: '#/components/responses/LoadTableResponse'
+ 400:
+ $ref: '#/components/responses/BadRequestErrorResponse'
+ 401:
+ $ref: '#/components/responses/UnauthorizedResponse'
+ 404:
+ description:
+ Not Found - NoSuchTableException, table to load does not exist
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/IcebergErrorResponse'
+ examples:
+ TableToRenameDoesNotExist:
+ $ref: '#/components/examples/NoSuchTableError'
+ 5XX:
+ $ref: '#/components/responses/ServerErrorResponse'
+
+ post:
+ tags:
+ - Catalog API
+ summary: Commit updates to a table
+ operationId: updateTable
+ description:
+ Commit updates to a table.
+
+ Commits have two parts, requirements and updates. Requirements are
assertions that will be validated
+ before attempting to make and commit changes. For example,
assert-ref-snapshot-id will check that a
+ named ref's snapshot ID has a certain value.
+
+ Updates are changes to make to table metadata. For example, after
asserting that the current main ref
+ is at the expected snapshot, a commit may add a new child snapshot and
set the ref to the new
+ snapshot id.
+ requestBody:
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/CommitTableRequest'
+ responses:
+ 200:
+ description: OK
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/CommitTableResponse'
+ 400:
+ $ref: '#/components/responses/BadRequestErrorResponse'
+ 401:
+ $ref: '#/components/responses/UnauthorizedResponse'
+ 404:
+ description:
+ Not Found - NoSuchTableException, table to load does not exist
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/responses/IcebergErrorResponse'
+ examples:
+ TableToRenameDoesNotExist:
+ $ref: '#/components/examples/NoSuchTableError'
+ 409:
+ description:
+ Conflict - CommitFailedException, one or more requirements failed.
The client may retry.
Review comment:
In the previous PR, we put `50X` everywhere. I believe I also included a
section that mentions 50X can return text/plain (eg some middleware or proxy
server hit an issue and then returned an HTML page, which definitely happens
even once you hit the app sever).
I think we should include them everywhere, but I can understand possibly
adding them last as they’re the same throughout,
50X response would be maybe a good argument for using an external tool to
bundle all of the files together, though we’re likely far from being in that
position currently and should focus on defining the spec first.
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -579,36 +672,552 @@ components:
destination:
$ref: '#/components/schemas/TableIdentifier'
+ Namespace:
+ description: Reference to one or more levels of a namespace
+ type: array
+ items:
+ type: string
+ example: [ "accounting", "tax" ]
+
TableIdentifier:
type: object
required:
- namespace
- name
properties:
namespace:
+ $ref: '#/components/schemas/Namespace'
+ name:
+ type: string
+ nullable: false
+
+ PrimitiveType:
+ type: string
+ example:
+ - "long"
+ - "string"
+ - "fixed[16]"
+ - "decimal(10,2)"
+
+ StructField:
+ type: object
+ required:
+ - id
+ - name
+ - required
+ - type
+ properties:
+ id:
+ type: integer
+ name:
+ type: string
+ required:
+ type: boolean
+ type:
+ $ref: '#/components/schemas/Type'
+ doc:
+ type: string
+
+ StructType:
+ type: object
+ required:
+ - fields
+ properties:
+ type:
+ type: string
+ enum: ["struct"]
+ fields:
type: array
- description: Individual levels of the namespace
items:
- type: string
+ $ref: '#/components/schemas/StructField'
+
+ ListType:
+ type: object
+ required:
+ - type
+ - element-id
+ - element-required
+ - element
+ properties:
+ type:
+ type: string
+ enum: ["list"]
+ element-id:
+ type: integer
+ element-required:
+ type: boolean
+ element:
+ $ref: '#/components/schemas/Type'
+
+ MapType:
+ type: object
+ required:
+ - type
+ - key-id
+ - key
+ - value-id
+ - value-required
+ - value
+ properties:
+ type:
+ type: string
+ enum: ["map"]
+ key-id:
+ type: integer
+ key:
+ $ref: '#/components/schemas/Type'
+ value-id:
+ type: integer
+ value-required:
+ type: boolean
+ value:
+ $ref: '#/components/schemas/Type'
+
+ Type:
+ anyOf:
+ - $ref: '#/components/schemas/PrimitiveType'
+ - $ref: '#/components/schemas/StructType'
+ - $ref: '#/components/schemas/ListType'
+ - $ref: '#/components/schemas/MapType'
+
+ Schema:
+ allOf:
+ - $ref: '#/components/schemas/StructType'
+ - type: object
+ properties:
+ schema-id:
+ type: integer
+ readOnly: true
+ identifier-field-ids:
+ type: array
+ items:
+ type: integer
+
+ Transform:
+ type: string
+ example:
+ - "identity"
+ - "year"
+ - "month"
+ - "day"
+ - "hour"
+ - "bucket[256]"
+ - "truncate[16]"
+
+ PartitionField:
+ type: object
+ required:
+ - source-id
+ - transform
+ - name
+ properties:
+ field-id:
+ type: integer
+ source-id:
+ type: integer
name:
type: string
- nullable: false
+ transform:
+ $ref: '#/components/schemas/Transform'
- UpdateNamespacePropertiesRequest:
+ PartitionSpec:
type: object
+ required:
+ - fields
properties:
- removals:
+ spec-id:
+ type: integer
+ readOnly: true
+ fields:
type: array
- uniqueItems: true
items:
+ $ref: '#/components/schemas/PartitionField'
+
+ SortDirection:
+ type: string
+ enum: ["asc", "desc"]
+
+ NullOrder:
+ type: string
+ enum: ["nulls-first", "nulls-last"]
+
+ SortField:
+ type: object
+ required:
+ - source-id
+ - transform
+ - direction
+ - null-order
+ properties:
+ source-id:
+ type: integer
+ transform:
+ $ref: '#/components/schemas/Transform'
+ direction:
+ $ref: '#/components/schemas/SortDirection'
+ null-order:
+ $ref: '#/components/schemas/NullOrder'
+
+ SortOrder:
+ type: object
+ required:
+ - fields
+ properties:
+ order-id:
+ type: integer
+ readOnly: true
+ fields:
+ $ref: '#/components/schemas/SortField'
+
+ Snapshot:
+ type: object
+ required:
+ - snapshot-id
+ - timestamp-ms
+ - manifest-list
+ properties:
+ snapshot-id:
+ type: integer
+ timestamp-ms:
+ type: integer
+ manifest-list:
+ type: string
+ description: Location of the snapshot's manifest list file
+ schema-id:
+ type: integer
+ summary:
+ type: object
+ required:
+ - summary
+ properties:
+ summary:
+ type: string
+ enum: ["append", "replace", "overwrite", "delete"]
+ additionalProperties:
+ type: string
+
+ SnapshotReference:
+ type: object
+ required:
+ - type
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: ["tag", "branch"]
+ snapshot-id:
+ type: integer
+ max-ref-age-ms:
+ type: integer
+ max-snapshot-age-ms:
+ type: integer
+ min-snapshots-to-keep:
+ type: integer
+
+ SnapshotReferences:
+ type: object
+ additionalProperties:
+ $ref: '#/components/schemas/SnapshotReference'
+
+ SnapshotLog:
+ type: array
+ items:
+ type: object
+ required:
+ - snapshot-id
+ - timestamp-ms
+ properties:
+ snapshot-id:
+ type: integer
+ timestamp-ms:
+ type: integer
+
+ MetadataLog:
+ type: array
+ items:
+ type: object
+ required:
+ - metadata-file
+ - timestamp-ms
+ properties:
+ metadata-file:
type: string
- example: '[ "department", "access_group" ]'
- updates:
- uniqueItems: true
+ timestamp-ms:
+ type: integer
+
+ TableMetadata:
+ type: object
+ required:
+ - format-version
+ - table-uuid
+ properties:
+ format-version:
+ type: integer
+ table-uuid:
+ type: string
+ location:
+ type: string
+ last-updated-ms:
+ type: integer
+ properties:
type: object
- items:
+ additionalProperties:
Review comment:
+1. This is the proper way to show generic string to string map (though
it looks weird if you’re not used to it).
I’ve considered defining a `StringMap` type as I’ve seen that done in some
projects to make the `additionalProperties` part less weird (especially for
`Map<String, String>` where we know the required fields and other things.
##########
File path: rest_docs/rest-catalog-open-api.yaml
##########
@@ -579,36 +672,552 @@ components:
destination:
$ref: '#/components/schemas/TableIdentifier'
+ Namespace:
+ description: Reference to one or more levels of a namespace
+ type: array
+ items:
+ type: string
+ example: [ "accounting", "tax" ]
+
TableIdentifier:
type: object
required:
- namespace
- name
properties:
namespace:
+ $ref: '#/components/schemas/Namespace'
+ name:
+ type: string
+ nullable: false
+
+ PrimitiveType:
+ type: string
+ example:
+ - "long"
+ - "string"
+ - "fixed[16]"
+ - "decimal(10,2)"
+
+ StructField:
+ type: object
+ required:
+ - id
+ - name
+ - required
+ - type
+ properties:
+ id:
+ type: integer
+ name:
+ type: string
+ required:
+ type: boolean
+ type:
+ $ref: '#/components/schemas/Type'
+ doc:
+ type: string
+
+ StructType:
+ type: object
+ required:
+ - fields
+ properties:
+ type:
+ type: string
+ enum: ["struct"]
+ fields:
type: array
- description: Individual levels of the namespace
items:
- type: string
+ $ref: '#/components/schemas/StructField'
+
+ ListType:
+ type: object
+ required:
+ - type
+ - element-id
+ - element-required
+ - element
+ properties:
+ type:
+ type: string
+ enum: ["list"]
+ element-id:
+ type: integer
+ element-required:
+ type: boolean
+ element:
+ $ref: '#/components/schemas/Type'
+
+ MapType:
+ type: object
+ required:
+ - type
+ - key-id
+ - key
+ - value-id
+ - value-required
+ - value
+ properties:
+ type:
+ type: string
+ enum: ["map"]
+ key-id:
+ type: integer
+ key:
+ $ref: '#/components/schemas/Type'
+ value-id:
+ type: integer
+ value-required:
+ type: boolean
+ value:
+ $ref: '#/components/schemas/Type'
+
+ Type:
+ anyOf:
+ - $ref: '#/components/schemas/PrimitiveType'
+ - $ref: '#/components/schemas/StructType'
+ - $ref: '#/components/schemas/ListType'
+ - $ref: '#/components/schemas/MapType'
+
+ Schema:
+ allOf:
+ - $ref: '#/components/schemas/StructType'
+ - type: object
+ properties:
+ schema-id:
+ type: integer
+ readOnly: true
+ identifier-field-ids:
+ type: array
+ items:
+ type: integer
+
+ Transform:
+ type: string
+ example:
+ - "identity"
+ - "year"
+ - "month"
+ - "day"
+ - "hour"
+ - "bucket[256]"
+ - "truncate[16]"
+
+ PartitionField:
+ type: object
+ required:
+ - source-id
+ - transform
+ - name
+ properties:
+ field-id:
+ type: integer
+ source-id:
+ type: integer
name:
type: string
- nullable: false
+ transform:
+ $ref: '#/components/schemas/Transform'
- UpdateNamespacePropertiesRequest:
+ PartitionSpec:
type: object
+ required:
+ - fields
properties:
- removals:
+ spec-id:
+ type: integer
+ readOnly: true
+ fields:
type: array
- uniqueItems: true
items:
+ $ref: '#/components/schemas/PartitionField'
+
+ SortDirection:
+ type: string
+ enum: ["asc", "desc"]
+
+ NullOrder:
+ type: string
+ enum: ["nulls-first", "nulls-last"]
+
+ SortField:
+ type: object
+ required:
+ - source-id
+ - transform
+ - direction
+ - null-order
+ properties:
+ source-id:
+ type: integer
+ transform:
+ $ref: '#/components/schemas/Transform'
+ direction:
+ $ref: '#/components/schemas/SortDirection'
+ null-order:
+ $ref: '#/components/schemas/NullOrder'
+
+ SortOrder:
+ type: object
+ required:
+ - fields
+ properties:
+ order-id:
+ type: integer
+ readOnly: true
+ fields:
+ $ref: '#/components/schemas/SortField'
+
+ Snapshot:
+ type: object
+ required:
+ - snapshot-id
+ - timestamp-ms
+ - manifest-list
+ properties:
+ snapshot-id:
+ type: integer
+ timestamp-ms:
+ type: integer
+ manifest-list:
+ type: string
+ description: Location of the snapshot's manifest list file
+ schema-id:
+ type: integer
+ summary:
+ type: object
+ required:
+ - summary
+ properties:
+ summary:
+ type: string
+ enum: ["append", "replace", "overwrite", "delete"]
+ additionalProperties:
+ type: string
+
+ SnapshotReference:
+ type: object
+ required:
+ - type
+ - snapshot-id
+ properties:
+ type:
+ type: string
+ enum: ["tag", "branch"]
+ snapshot-id:
+ type: integer
+ max-ref-age-ms:
+ type: integer
+ max-snapshot-age-ms:
+ type: integer
+ min-snapshots-to-keep:
+ type: integer
+
+ SnapshotReferences:
+ type: object
+ additionalProperties:
+ $ref: '#/components/schemas/SnapshotReference'
+
+ SnapshotLog:
+ type: array
+ items:
+ type: object
+ required:
+ - snapshot-id
+ - timestamp-ms
+ properties:
+ snapshot-id:
+ type: integer
+ timestamp-ms:
+ type: integer
+
+ MetadataLog:
+ type: array
+ items:
+ type: object
+ required:
+ - metadata-file
+ - timestamp-ms
+ properties:
+ metadata-file:
type: string
- example: '[ "department", "access_group" ]'
- updates:
- uniqueItems: true
+ timestamp-ms:
+ type: integer
+
+ TableMetadata:
+ type: object
+ required:
+ - format-version
+ - table-uuid
+ properties:
+ format-version:
+ type: integer
+ table-uuid:
+ type: string
+ location:
+ type: string
+ last-updated-ms:
+ type: integer
+ properties:
type: object
- items:
+ additionalProperties:
type: string
- example: { "owner": "Hank Bendickson" }
+ # schema tracking
+ schemas:
+ type: array
+ items:
+ $ref: '#/components/schemas/Schema'
+ current-schema-id:
+ type: integer
+ last-column-id:
+ type: integer
+ # partition spec tracking
+ partition-specs:
+ type: array
+ items:
+ $ref: '#/components/schemas/PartitionSpec'
+ default-spec-id:
+ type: integer
+ last-partition-id:
+ type: integer
+ # sort order tracking
+ sort-orders:
+ type: array
+ items:
+ $ref: '#/components/schemas/SortOrder'
+ default-sort-order-id:
+ type: integer
+ # snapshot tracking
+ snapshots:
+ type: array
+ items:
+ $ref: '#/components/schemas/Snapshot'
+ refs:
+ $ref: '#/components/schemas/SnapshotReferences'
+ current-snapshot-id:
+ type: integer
Review comment:
The integer width is defined in the spec, right? We should define if
here too. I believe the key is `format` and can be `int32` or `int64` (eg long).
You can also use type Number and then use `float` and `double`. `Number` is
the super type basically.
--
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]
