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]

Reply via email to