XJDKC commented on code in PR #1026:
URL: https://github.com/apache/polaris/pull/1026#discussion_r1976761570
##########
spec/polaris-management-service.yml:
##########
@@ -850,9 +850,92 @@ components:
- $ref: "#/components/schemas/Catalog"
- type: object
properties:
- remoteUrl:
- type: string
- description: URL to the remote catalog API
+ connectionConfigInfo:
+ $ref: "#/components/schemas/ConnectionConfigInfo"
+
+ ConnectionConfigInfo:
+ type: object
+ description: A connection configuration representing a remote catalog
service
+ properties:
+ connectionType:
+ type: string
+ enum:
+ - ICEBERG_REST
+ description: The type of remote catalog service represented by this
connection
+ remoteUri:
+ type: string
+ description: URI to the remote catalog service
+ required:
+ - connectionType
+ discriminator:
+ propertyName: connectionType
+ mapping:
+ ICEBERG_REST: "#/components/schemas/IcebergRestConnectionConfigInfo"
+
+ IcebergRestConnectionConfigInfo:
+ type: object
+ description: Configuration necessary for connecting to an Iceberg REST
Catalog
+ allOf:
+ - $ref: '#/components/schemas/ConnectionConfigInfo'
+ properties:
+ remoteCatalogName:
+ type: string
+ description: The name of a remote catalog instance within the remote
catalog service; in some older systems
+ this is specified as the 'warehouse' when multiple logical
catalogs are served under the same base
+ remoteUri, and often translates into a 'prefix' added to all REST
resource paths
+ restAuthentication:
+ $ref: "#/components/schemas/RestAuthenticationInfo"
+
+ RestAuthenticationInfo:
+ type: object
+ description: Authentication-specific information for a REST connection
+ properties:
+ restAuthenticationType:
+ type: string
+ enum:
+ - OAUTH
+ - BEARER
+ description: The type of authentication to use when connecting to
the remote rest service
+ required:
+ - restAuthenticationType
+ discriminator:
+ propertyName: restAuthenticationType
+ mapping:
+ OAUTH: "#/components/schemas/OauthRestAuthenticationInfo"
+ BEARER: "#/components/schemas/BearerRestAuthenticationInfo"
+
+ OauthRestAuthenticationInfo:
+ type: object
+ description: OAuth authentication based on client_id/client_secret
+ allOf:
+ - $ref: '#/components/schemas/RestAuthenticationInfo'
+ properties:
+ tokenUri:
+ type: string
+ description: Token server URI
+ clientId:
+ type: string
+ description: oauth client id
+ clientSecret:
+ type: string
+ format: password
+ description: oauth client secret (input-only)
+ scopes:
+ type: array
+ items:
+ type: string
+ description: oauth scopes to specify when exchanging for a
short-lived access token
+
+ BearerRestAuthenticationInfo:
Review Comment:
Agreed! We might need to create an authentication config specifically for
ConnectionConfig. For other connections, like `IcebergHiveConnectionConfig`, we
can still reuse the existing authentication parameters.
Also, using `Parameters` as part of the name is a great approach. Since
we’ll be generating some classes based on the spec, suffixing them with
`Parameters` makes sense as they are part of requests. This also provides more
flexibility in naming our internal classes.
For example, we currently have `AwsStorageConfigInfo` (generated by OpenAPI)
and `AwsStorageConfigurationInfo` (internal representation of the AWS storage
config), which can be quite confusing. Using a clearer naming convention will
help distinguish between generated request models and internal representations.
Maybe we can use `IcebergRestConnectionConfigInfo` in our spec and
`IcebergRestConnectionConfig` as the name of internal class. If we follow the
same pattern as the storage config, the name of the internal name would be very
long, e.g. `IcebergRestConnectionConfigurationInfo`
--
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]
