nastra commented on code in PR #10722:
URL: https://github.com/apache/iceberg/pull/10722#discussion_r1773432500


##########
open-api/rest-catalog-open-api.yaml:
##########
@@ -3103,6 +3103,81 @@ components:
         uuid:
           type: string
 
+    ADLSCredentials:
+      type: object
+      allOf:
+        - $ref: '#/components/schemas/Credentials'
+      required:
+        - type
+      properties:
+        type:
+          type: string
+          enum: [ "adls" ]
+        account-name:
+          type: string
+        account-key:
+          type: string
+        sas-token:
+          type: string
+        expires-at-ms:
+          type: integer
+          format: int64
+
+    GCSCredentials:
+      type: object
+      allOf:
+        - $ref: '#/components/schemas/Credentials'
+      required:
+        - type
+        - token
+        - expires-at-ms
+      properties:
+        type:
+          type: string
+          enum: [ "gcs" ]
+        token:
+          type: string
+        expires-at-ms:
+          type: integer
+          format: int64
+
+    S3Credentials:
+      type: object
+      allOf:
+        - $ref: '#/components/schemas/Credentials'
+      required:

Review Comment:
   > I didn’t quite get that. Clients in other languages should stick to the 
names defined in the spec. I can’t imagine the server responding with different 
config keys, like the S3 region, just because the client is implemented in a 
different language.
   
   I remember that this was the case for pyiceberg a while ago, but looks like 
this was addressed by https://github.com/apache/iceberg-python/pull/922.
   
   > I'm OK with non-stardadrized schema-less for configs as they may change, 
but I still think it is more organized with one more layer like the following. 
As I said, this is a minor point to make the overall schema a bit cleaner, esp. 
for new storage type adding in the future.
   
   @flyrain do we have a concrete use case for this or is this rather a 
nice-to-have? I'm thinking that we most likely don't need this right now. Also 
in the last Iceberg sync we were talking about v2 version of loadTable, so 
maybe that fits better with a v2 API when we have a concrete use case for 
introducing this?
   



-- 
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