ctubbsii commented on code in PR #3173:
URL: https://github.com/apache/accumulo/pull/3173#discussion_r1087162094
##########
core/src/main/java/org/apache/accumulo/core/conf/Property.java:
##########
@@ -1702,16 +1702,20 @@ private static <T extends Annotation> boolean
hasPrefixWithAnnotation(String key
/**
* Checks if the given property and value are valid. A property is valid if
the property key is
- * valid see {@link #isValidTablePropertyKey} and that the value is a valid
format for the type
- * see {@link PropertyType#isValidFormat}.
+ * valid see {@link #isValidPropertyKey} and that the value is a valid
format for the type see
+ * {@link PropertyType#isValidFormat}.
*
* @param key property key
* @param value property value
* @return true if key is valid (recognized, or has a recognized prefix)
*/
- public static boolean isTablePropertyValid(final String key, final String
value) {
+ public static boolean isValidProperty(final String key, final String value) {
Review Comment:
I haven't done a full review of this PR, but it seems weird that this method
would have had the name "Table" in it if it weren't specifically constraining
the validity check to properties that can be set scoped to a table (or
namespace). We should be careful about changing this name, and its current
behavior that constrains validity to properties whose keys are appropriate for
a table (or namespace). There's a good chance that this method is expected to
enforce that constraint, and now it no longer does.
--
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]
