collado-mike commented on code in PR #465:
URL: https://github.com/apache/polaris/pull/465#discussion_r1854797397
##########
polaris-core/src/main/java/org/apache/polaris/core/persistence/MetaStoreManagerFactory.java:
##########
@@ -50,4 +51,17 @@ public interface MetaStoreManagerFactory extends
Discoverable {
/** Purge all metadata for the realms provided */
void purgeRealms(List<String> realms);
+
+ /**
+ * Default {@link PolarisGrantManager} factory that returns the {@link
PolarisMetaStoreManager} as
+ * the grant manager.
+ *
+ * @param realm the current realm
+ * @return the {@link PolarisMetaStoreManager} returned by {@link
+ * #getOrCreateMetaStoreManager(RealmContext)}
+ */
+ @Override
+ default PolarisGrantManager getGrantManagerForRealm(RealmContext realm) {
Review Comment:
Part of the reasoning for this PR is to move toward dependency injection
without relying on it fully implemented. #417 was, in large part, to encourage
callers to declare only the most specific dependency interface they need (e.g.,
`PolarisGrantManager` to manage grants, `CredentialVendor` to manage
credentials) so that we could use dependency injection to support different
manager implementations. Today, the `PolarisMetaStoreManager` implements all of
the component interfaces, so it's easy to make this interface implement the
`PolarisGrantManager.Factory` interface. In the future, I imagine we might
break the interface inheritance so that a `PolarisMetaStoreManager` doesn't
necessarily implement, e.g., `CredentialVendor`, since the persistence
implementation might not want to know anything about credential vending. But
for now, this factory method seems reasonable.
But we can still take advantage of the different interfaces even if all
roads eventually lead to the `PolarisMetaStoreManager`. The caching
implementation of the `PolarisGrantManager` is one such example. I'd also like
to rewrite the `StorageCredentialsCache` as an implementation of
`CredentialVendor`, even if the `PolarisMetaStoreManager` still implements the
credential vending methods.
I think we should resolve the DI discussion on the mailing list before we
introduce another service-locator approach.
--
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]