collado-mike commented on code in PR #1765:
URL: https://github.com/apache/polaris/pull/1765#discussion_r2155147142
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/LocalPolarisMetaStoreManagerFactory.java:
##########
@@ -100,7 +101,12 @@ private void initializeForRealm(
final StoreType backingStore = createBackingStore(diagnostics);
sessionSupplierMap.put(
realmContext.getRealmIdentifier(),
- () -> createMetaStoreSession(backingStore, realmContext,
rootCredentialsSet, diagnostics));
+ new CachedSupplier<>(
Review Comment:
This is a good question and it's not as straightforward as it might seem.
The `RealmContext` interface only defines a `getName` method, but there are
concrete implementations that may contain extra information about the realm (we
have our own custom impl). Simply materializing the `RealmContext` in this way
could break functionality if the underlying Session/MetaStoreManager depend on
the concrete implementation.
I think the proper long-term fix is to make the `BasePersistence` itself a
CDI-managed bean so that the `RealmContext` can be injected by the context
rather than us materializing it manually. It also means we have to make the
task execution framework CDI-managed, which is a bigger task that we've been
putting off for a while
--
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]
