-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GCP: Use catalog endpoint as base when refreshing OAuth2 token #12638
Conversation
50deccc
to
f29db13
Compare
f29db13
to
73308cf
Compare
@@ -49,6 +53,11 @@ private OAuth2RefreshCredentialsHandler(Map<String, String> properties) { | |||
Preconditions.checkArgument( | |||
null != properties.get(GCPProperties.GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT), | |||
"Invalid credentials endpoint: null"); | |||
Preconditions.checkArgument( | |||
null != properties.get(CatalogProperties.URI), "Invalid catalog endpoint: null"); | |||
this.credentialsEndpoint = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why aren't we doing the same thing here that we do in ADLS?
RESTUtil.resolveEndpoint(
properties.get(CatalogProperties.URI),
properties.get(ADLS_REFRESH_CREDENTIALS_ENDPOINT));
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we're doing it for GCP in GCPProperties
:
iceberg/gcp/src/main/java/org/apache/iceberg/gcp/GCPProperties.java
Lines 109 to 112 in 978189c
gcsOauth2RefreshCredentialsEndpoint = | |
RESTUtil.resolveEndpoint( | |
properties.get(CatalogProperties.URI), | |
properties.get(GCS_OAUTH2_REFRESH_CREDENTIALS_ENDPOINT)); |
For ADLS we're also doing the same in
AzureProperties
: this.adlsRefreshCredentialsEndpoint = | |
RESTUtil.resolveEndpoint( | |
properties.get(CatalogProperties.URI), | |
properties.get(ADLS_REFRESH_CREDENTIALS_ENDPOINT)); |
For S3 it happens in
AwsClientProperties
: iceberg/aws/src/main/java/org/apache/iceberg/aws/AwsClientProperties.java
Lines 109 to 111 in 7d0395d
this.refreshCredentialsEndpoint = | |
RESTUtil.resolveEndpoint( | |
properties.get(CatalogProperties.URI), properties.get(REFRESH_CREDENTIALS_ENDPOINT)); |
No description provided.