You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/02/22 02:00:53 UTC

[GitHub] [iceberg] kbendick opened a new pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

kbendick opened a new pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186


   The REST catalog receives some of its configuration from the server.
   
   This updates the REST Catalog spec to show:
   1. type of object for both `overrides` and `properties` is a string-to-string map (the `additionalProperties: { type: string }` indicates this).
   2. that these maps have unique items
   3. Provide a default value and an example value, in-line in the schema spec.
   
   This does not change anything about the spec, just provides more detail.
   
   See also https://github.com/apache/iceberg/pull/4184 which adds in a response object for this type.


-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] kbendick commented on pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#issuecomment-1047432423


   cc @rdblue. I was working on this PR for `RESTCatalogConfiguration` and I noticed that this schema in the rest open-api doc didn't have examples or the `additionalProperties` weirdness that indicates that it's an arbitrary string-to-string map. So I added them. https://github.com/apache/iceberg/pull/4184
   
   This isn't doesn't change the spec at all. I'll also update the response to match the name `RESTCatalogConfigResponse`.


-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] kbendick commented on pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#issuecomment-1047443114


   This passes based on editor.swagger.io and openapi-schema-validator 
   ```bash
   $ openapi-spec-validator docs/rest/rest-catalog-open-api.yaml
   OK
   ```


-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] kbendick commented on a change in pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#discussion_r811593161



##########
File path: docs/rest/rest-catalog-open-api.yaml
##########
@@ -661,12 +647,22 @@ components:
       properties:
         overrides:
           type: object
+          uniqueItems: true

Review comment:
       As this is a `Map` (or the type is `object`), I'm not sure if `uniqueItems` provides much value.
   
   Open to removing it if anybody thinks I should.




-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] rdblue commented on a change in pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#discussion_r815117702



##########
File path: docs/rest/rest-catalog-open-api.yaml
##########
@@ -94,21 +94,7 @@ paths:
         "
       responses:
         200:
-          description: Server specified configuration values.
-          content:
-            application/json:
-              schema:
-                $ref: '#/components/schemas/CatalogConfiguration'
-              example: {
-                "data": {
-                  "overrides": {
-                    "warehouse": "s3://bucket/warehouse/"
-                  },
-                  "defaults": {
-                    "clients": "4"
-                  }
-                }
-              }
+          $ref: '#/components/responses/RESTCatalogConfigResponse'

Review comment:
       I think it is clear that all the request/response objects are for REST, so we can probably omit that from the example and object names.




-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] rdblue commented on a change in pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#discussion_r815117970



##########
File path: docs/rest/rest-catalog-open-api.yaml
##########
@@ -661,12 +647,22 @@ components:
       properties:
         overrides:
           type: object
+          uniqueItems: true

Review comment:
       Yeah, probably not useful. It just makes me wonder if it isn't required for all the other maps.




-- 
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-unsubscribe@iceberg.apache.org

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



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


[GitHub] [iceberg] rdblue commented on a change in pull request #4186: SPEC - Update REST Catalog spec to put examples + type details on catalog config

Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #4186:
URL: https://github.com/apache/iceberg/pull/4186#discussion_r815118090



##########
File path: docs/rest/rest-catalog-open-api.yaml
##########
@@ -652,7 +638,7 @@ components:
           description: HTTP response code
           example: 404
 
-    CatalogConfiguration:
+    RESTCatalogConfig:

Review comment:
       Why this rename?




-- 
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-unsubscribe@iceberg.apache.org

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



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