dimas-b commented on code in PR #10722:
URL: https://github.com/apache/iceberg/pull/10722#discussion_r1796920466


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3129,6 +3145,11 @@ components:
          - `s3.secret-access-key`: secret for credentials that provide access 
to data in S3 
          - `s3.session-token`: if present, this value should be used for as 
the session token 
          - `s3.remote-signing-enabled`: if `true` remote signing should be 
performed as described in the `s3-signer-open-api.yaml` specification
+
+        ## Storage Credentials
+
+        Credentials for ADLS / GCS / S3 / ... are provided through the 
`storage-credentials` field.

Review Comment:
   Servers have to provide concrete properties for vendor-specific credentials. 
Since this spec does not define them, how will servers know what properties to 
provide?
   
   If servesr adapt to the java REST client impl. then the java client will 
become the "spec"... I'd prefer for Iceberg to provide additional guidelines 
(which do not have to be part of the REST Catalog spec) to clarify credential 
properties for known use cases. It might be necessary for this spec to define a 
sub-type property since some vendors support multiple auth methods for the same 
URI scheme.



##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3103,6 +3103,22 @@ components:
         uuid:
           type: string
 
+    StorageCredential:
+      type: object
+      required:
+        - prefix
+        - config
+      properties:
+        prefix:
+          type: string
+          description: Indicates a storage location prefix where the 
credential is relevant. Clients should choose the most
+            specific prefix if several credentials of the same type are 
available.
+        config:
+          type: object
+          additionalProperties:

Review Comment:
   I understand there was a general agreement to treat vendor-specific 
credentials as simple property bags. However, would it make sense to define 
some common attributes (like expiry time) in this spec? I suppose those would 
be relevant to the follow-up effort of refreshing credentials on the fly. WDYT?



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to