XJDKC commented on code in PR #2523:
URL: https://github.com/apache/polaris/pull/2523#discussion_r2331463806
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -402,6 +414,20 @@ public OidcTenantResolver oidcTenantResolver(
return
resolvers.select(Identifier.Literal.of(config.tenantResolver())).get();
}
+ @Produces
+ @RequestScoped
+ public RealmServiceIdentityConfiguration realmServiceIdentityConfig(
+ ServiceIdentityConfiguration config, RealmContext realmContext) {
+ return config.forRealm(realmContext);
+ }
+
+ @Produces
+ @RequestScoped
+ public ServiceIdentityRegistry serviceIdentityRegistry(
+ ServiceIdentityRegistryFactory serviceIdentityRegistryFactory,
RealmContext realmContext) {
+ return
serviceIdentityRegistryFactory.getOrCreateServiceIdentityRegistry(realmContext);
Review Comment:
If we want to cache the service identity, does it make sense to add this
factory? This way, we don't need to create a new `ServiceIdentityRegistry` and
resolve the identity for each request. I think Service Identity Resolution can
be very expensive: e.g., if we want to retrieve the iam user creds from remote
Secret Manager, it may introduce many overheads.
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ServiceProducers.java:
##########
@@ -402,6 +414,20 @@ public OidcTenantResolver oidcTenantResolver(
return
resolvers.select(Identifier.Literal.of(config.tenantResolver())).get();
}
+ @Produces
+ @RequestScoped
+ public RealmServiceIdentityConfiguration realmServiceIdentityConfig(
+ ServiceIdentityConfiguration config, RealmContext realmContext) {
+ return config.forRealm(realmContext);
+ }
+
+ @Produces
+ @RequestScoped
+ public ServiceIdentityRegistry serviceIdentityRegistry(
+ ServiceIdentityRegistryFactory serviceIdentityRegistryFactory,
RealmContext realmContext) {
+ return
serviceIdentityRegistryFactory.getOrCreateServiceIdentityRegistry(realmContext);
Review Comment:
If we want to cache the service identity, does it make sense to add this
factory? This way, we don't need to create a new `ServiceIdentityRegistry` and
resolve the identity for each request. I think Service Identity Resolution can
be very expensive: e.g., if we want to retrieve the iam user creds from remote
Secret Manager, it may introduce many overheads.
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]
