ctubbsii commented on code in PR #5865:
URL: https://github.com/apache/accumulo/pull/5865#discussion_r2342548740
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -1684,7 +1684,8 @@ public static boolean
isValidResourceGroupPropertyKey(String property) {
return property.startsWith(Property.GENERAL_PREFIX.getKey())
|| property.startsWith(COMPACTION_PREFIX.getKey())
|| property.startsWith(COMPACTOR_PREFIX.getKey())
- || property.startsWith(SSERV_PREFIX.getKey()) ||
property.startsWith(TSERV_PREFIX.getKey());
+ || property.startsWith(SSERV_PREFIX.getKey()) ||
property.startsWith(TSERV_PREFIX.getKey())
+ || isFixedZooPropertyKey(getPropertyByKey(property));
Review Comment:
It's not clear to me why the entire set of isFixedZooPropertyKey needs to be
added here. The fixed zoo properties should already be a subset of the system
properties that can be configured in ZK, all of which should already be allowed
in RG properties.
I also think we need to refactor this a bit to have a set of valid
system-level properties (for resource groups and regular system-wide
properties), and keep that separate from what's valid for table/namespace
properties.
Right now, the set of valid Zoo properties seems to allow table properties,
which I thought was removed. Is that still pending in another PR?
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
