sungwy commented on code in PR #18254:
URL: https://github.com/apache/iceberg/pull/18254#discussion_r4262009449
##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -5854,6 +5513,104 @@ components:
headers:
$ref: '#/components/schemas/MultiValuedMap'
+ RemoteSignBatchRequest:
+ description: >
+ A batch of requests to be signed remotely. `region`, `method`,
`provider` and
+ `properties` apply to every element of `requests`.
+ type: object
+ required:
+ - region
+ - method
+ - requests
+ properties:
+ region:
+ type: string
+ method:
+ type: string
+ enum: ["PUT", "GET", "HEAD", "POST", "DELETE", "PATCH", "OPTIONS"]
+ provider:
+ type: string
+ description: The storage provider for every element of the batch. If
this is not specified, the
+ provider is assumed to be `s3`, as in `RemoteSignRequest`.
+ properties:
+ type: object
+ additionalProperties:
+ type: string
+ requests:
+ type: array
+ minItems: 1
+ items:
+ $ref: '#/components/schemas/RemoteSignBatchElement'
+
+ RemoteSignBatchElement:
+ description: The fields of one request in a batch that are not repeated
with the other requests.
+ type: object
+ required:
+ - uri
+ - headers
+ properties:
+ uri:
+ type: string
+ headers:
+ $ref: '#/components/schemas/MultiValuedMap'
+ body:
+ type: string
+ description: Optional body of the request to send to the signing
API, as in `RemoteSignRequest`.
+
+ SignStatus:
+ description: Status of signing one element of a batch
+ type: string
+ enum: ["completed", "failed"]
+
+ CompletedSignResult:
+ description: A successfully signed batch element
+ allOf:
+ - $ref: '#/components/schemas/RemoteSignResult'
+ - type: object
+ required:
+ - status
+ properties:
+ status:
+ $ref: '#/components/schemas/SignStatus'
+ const: "completed"
+
+ FailedSignResult:
+ description: A batch element that could not be signed
+ type: object
+ required:
+ - status
+ - error
+ properties:
+ status:
+ $ref: '#/components/schemas/SignStatus'
+ const: "failed"
+ error:
+ $ref: '#/components/schemas/ErrorModel'
+
+ RemoteSignBatchResult:
+ type: object
+ description: The result of signing one element of a batch
+ discriminator:
+ propertyName: status
+ mapping:
+ completed: '#/components/schemas/CompletedSignResult'
+ failed: '#/components/schemas/FailedSignResult'
+ oneOf:
+ - $ref: '#/components/schemas/CompletedSignResult'
+ - $ref: '#/components/schemas/FailedSignResult'
+
+ RemoteSignBatchResponse:
+ description: The result of a batch remote request signing operation.
+ type: object
+ required:
+ - results
+ properties:
+ results:
+ description: Results for each element of the request, in request
order
+ type: array
+ items:
+ $ref: '#/components/schemas/RemoteSignBatchResult'
Review Comment:
Thanks for the thoughtful review Dan.
> Do we need to be able to allow partial results? What does it mean to be
able to access some of the files? I think we need to decide if that's a
reasonable model.
I have the [prior art
analysis](https://docs.google.com/document/d/1A1mSy-c79ZYQ1zcEcVlagev1wwQOaiLo_uRobmqZSOU/edit?tab=t.0#heading=h.10s5qmutq23m)
on the proposal doc, and the dominant pattern I saw was to allow partial
failure handling. I observed that all-or-none semantic was mostly used for a
batch request that needed an atomic guarantee (e.g. create a batch of items
together).
I think handling partial failures would technically allow the client to grab
the failed ones, and retry them in the next batch rather than retrying the
entire batch.
> If we do allow this, I would prefer that we separate the successful and
failed requests into two separate lists. It's easier to distinguish than a
single list that's of mixed types. (Separate is more similar to how S3 do
operations like DeleteObjects
I initially thought keeping it in the same list would work best because we
dont have an identifier (a whole `RemoteSignRequest` uniquely identifies the
singular entry), so we are relying on the ordered response to match the
requested individual entry to its response.
But I think we can do the same by including the index number in the response
- and require that the client map the index in the response to the position of
the `RemoteSignBatchElement` from the original batch request.
Let me update the PR with this change
--
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]
