You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "snazy (via GitHub)" <gi...@apache.org> on 2025/08/01 14:20:03 UTC
[PR] Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` [polaris]
snazy opened a new pull request, #2235:
URL: https://github.com/apache/polaris/pull/2235
Instances of `PolarisStorageIntegration` are created for a particular `PolarisStorageConfigurationInfo`, the same value is then passed into `PSI.getSubscopedCreds()`.
This change removes the config parameter, as it's already known when `PolarisStorageIntegration` instances are created.
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on code in PR #2235:
URL: https://github.com/apache/polaris/pull/2235#discussion_r2248277534
##########
service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java:
##########
@@ -81,27 +85,32 @@ public PolarisStorageIntegrationProviderImpl(
case S3:
storageIntegration =
(PolarisStorageIntegration<T>)
- new AwsCredentialsStorageIntegration(stsClientProvider, stsCredentials);
+ new AwsCredentialsStorageIntegration(
+ (AwsStorageConfigurationInfo) polarisStorageConfigurationInfo,
+ stsClientProvider,
+ stsCredentials);
break;
case GCS:
storageIntegration =
(PolarisStorageIntegration<T>)
new GcpCredentialsStorageIntegration(
+ (GcpStorageConfigurationInfo) polarisStorageConfigurationInfo,
gcpCredsProvider.get(),
ServiceOptions.getFromServiceLoader(
HttpTransportFactory.class, NetHttpTransport::new));
break;
case AZURE:
storageIntegration =
- (PolarisStorageIntegration<T>) new AzureCredentialsStorageIntegration();
+ (PolarisStorageIntegration<T>)
+ new AzureCredentialsStorageIntegration(
+ (AzureStorageConfigurationInfo) polarisStorageConfigurationInfo);
break;
case FILE:
storageIntegration =
- new PolarisStorageIntegration<>("file") {
+ new PolarisStorageIntegration<>((T) polarisStorageConfigurationInfo, "file") {
Review Comment:
Yea, but javac is a bit opinionated here ...
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy merged PR #2235:
URL: https://github.com/apache/polaris/pull/2235
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #2235:
URL: https://github.com/apache/polaris/pull/2235#discussion_r2248262082
##########
service/common/src/main/java/org/apache/polaris/service/storage/PolarisStorageIntegrationProviderImpl.java:
##########
@@ -81,27 +85,32 @@ public PolarisStorageIntegrationProviderImpl(
case S3:
storageIntegration =
(PolarisStorageIntegration<T>)
- new AwsCredentialsStorageIntegration(stsClientProvider, stsCredentials);
+ new AwsCredentialsStorageIntegration(
+ (AwsStorageConfigurationInfo) polarisStorageConfigurationInfo,
+ stsClientProvider,
+ stsCredentials);
break;
case GCS:
storageIntegration =
(PolarisStorageIntegration<T>)
new GcpCredentialsStorageIntegration(
+ (GcpStorageConfigurationInfo) polarisStorageConfigurationInfo,
gcpCredsProvider.get(),
ServiceOptions.getFromServiceLoader(
HttpTransportFactory.class, NetHttpTransport::new));
break;
case AZURE:
storageIntegration =
- (PolarisStorageIntegration<T>) new AzureCredentialsStorageIntegration();
+ (PolarisStorageIntegration<T>)
+ new AzureCredentialsStorageIntegration(
+ (AzureStorageConfigurationInfo) polarisStorageConfigurationInfo);
break;
case FILE:
storageIntegration =
- new PolarisStorageIntegration<>("file") {
+ new PolarisStorageIntegration<>((T) polarisStorageConfigurationInfo, "file") {
Review Comment:
`T` is `FileStorageConfigurationInfo` here, right?
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Remove config parameter from `PolarisStorageIntegration#getSubscopedCreds` [polaris]
Posted by "snazy (via GitHub)" <gi...@apache.org>.
snazy commented on PR #2235:
URL: https://github.com/apache/polaris/pull/2235#issuecomment-3154207231
Rebased after merge conflict w/ #2234
--
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@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org