singhpk234 commented on code in PR #17912:
URL: https://github.com/apache/iceberg/pull/17912#discussion_r4503764303
##########
aws/src/main/java/org/apache/iceberg/aws/s3/VendedCredentialsProvider.java:
##########
@@ -56,7 +58,14 @@ public class VendedCredentialsProvider implements
AwsCredentialsProvider, SdkAut
private VendedCredentialsProvider(Map<String, String> properties) {
Preconditions.checkArgument(null != properties, "Invalid properties:
null");
- Preconditions.checkArgument(null != properties.get(URI), "Invalid
credentials endpoint: null");
+ this.credentialsEndpoint =
+ properties.containsKey(RESTCatalogProperties.CREDENTIALS_ENDPOINT)
+ ? properties.get(RESTCatalogProperties.CREDENTIALS_ENDPOINT)
+ : RESTUtil.resolveEndpoint(
+ properties.get(CatalogProperties.URI),
+ properties.getOrDefault(
+ URI,
properties.get(AwsClientProperties.REFRESH_CREDENTIALS_ENDPOINT)));
Review Comment:
can we use PropertyUtils here
##########
gcp/src/main/java/org/apache/iceberg/gcp/gcs/OAuth2RefreshCredentialsHandler.java:
##########
@@ -51,13 +52,15 @@ public class OAuth2RefreshCredentialsHandler
private AuthSession authSession;
private OAuth2RefreshCredentialsHandler(Map<String, String> properties) {
- Preconditions.checkArgument(
- null !=
properties.get(GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT),
- "Invalid credentials endpoint: null");
+ this.credentialsEndpoint =
+ properties.containsKey(RESTCatalogProperties.CREDENTIALS_ENDPOINT)
+ ? properties.get(RESTCatalogProperties.CREDENTIALS_ENDPOINT)
+ : RESTUtil.resolveEndpoint(
+ properties.get(CatalogProperties.URI),
+
properties.get(GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT));
Review Comment:
i wonder if its better we can just have a function in rest util which takes
:
// in REST Util Class
RESTUtil.resolveCredentialEndpoint(properties, endpointPath) {
return
PropertyUtil.propertyAsString(RESTCatalogProperties.CREDENTIALS_ENDPOINT,
RESTUtil.resolveEndpoint(
properties.get(CatalogProperties.URI), endpointPath))
}
// in this class this just becomes
this.credentialEndpoint = RESTUtil.resolveCredentialEndpoint(properties,
properties.get(GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT))
This was all the provider specific property key inspection stays here and
the RESTUtil is generic enough, 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: [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]
