dimas-b commented on code in PR #1817:
URL: https://github.com/apache/polaris/pull/1817#discussion_r2130583920
##########
service/common/src/main/java/org/apache/polaris/service/config/DefaultConfigurationStore.java:
##########
@@ -51,7 +54,10 @@ public DefaultConfigurationStore(
@Override
public <T> @Nullable T getConfiguration(@Nonnull RealmContext realmContext,
String configName) {
- String realm = realmContext.getRealmIdentifier();
+ String realm = realmContextInstance.get().getRealmIdentifier();
Review Comment:
My thinking goes in the other direction. I'd like to make
`DeafaultConfigurationStore` a proper CDI bean getting realm ID from the
request context. Ideally `Instance<RealmContext>` simply becomes `RealmContext`.
Eventually, I'd like to remove the `RealmContext` parameter.
I believe refactoring the code and internal interfaces to be free from
`RealmContext` parameters in methods would be beneficial. Most of the code is
written to operate within a realm. Managing various objects (e.g. meta store
instances) for multiple realms feels like an infrastructure concern. 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: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
