HonahX commented on code in PR #2696:
URL: https://github.com/apache/polaris/pull/2696#discussion_r2383285261


##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java:
##########
@@ -124,6 +125,7 @@ public Response createCatalog(
     Catalog catalog = request.getCatalog();
     validateStorageConfig(catalog.getStorageConfigInfo());
     validateExternalCatalog(catalog);
+    validateCatalogProperties(catalog.getProperties());

Review Comment:
   Great question! Here is some more context: 
https://github.com/apache/polaris/pull/2688#discussion_r2380161311,
   https://github.com/apache/polaris/pull/2688#discussion_r2380161311.
   
   TLDR, we want to support configuring 
`ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` per catalog instead of per 
realm, but with some limitaiton. This PR is the first step that at least the 
owner could disable any changes to 
`ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` on the catalog.
   
   IMHO, more limitation should be imposed on 
`ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS`, for example to prevent it 
from turning off if turned on, to prevent unnecessary complexity if you first 
step up some grants and suddenly lose the power to change it, but existing one 
still take effect. This could be a general enhancement option for our feature 
flag infra : )
   
   > In other words, is it the intended use case that I can start with C as 
true, set ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS on my catalog, then 
set C to false, and later continue to change RBAC settings within my catalog?
   
   I do not think this is the intended use-case. IMHO, 
`ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS` should not be changed 
frequently
   
   



-- 
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