brianloss commented on a change in pull request #1873:
URL: https://github.com/apache/accumulo/pull/1873#discussion_r562845665
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java
##########
@@ -70,6 +81,20 @@ public static void setSystemProperty(ServerContext context,
String property, Str
public static void removeSystemProperty(ServerContext context, String
property)
throws InterruptedException, KeeperException {
+ AtomicBoolean shouldRemove = new AtomicBoolean(true);
+ DeprecatedPropertyUtil.getReplacementName(property, (log, replacement) -> {
+ log.warn("{} was deprecated and will be removed in a future release;"
+ + " no action was taken because it is not set here;"
+ + " did you mean to remove its replacment {} instead?", property,
replacement);
+ shouldRemove.set(false);
+ });
+ if (shouldRemove.get()) {
+ removePropWithoutDeprecationWarning(context, property);
+ }
Review comment:
@keith-turner I'm wondering if you have any opinion on this change in
particular. This change means a user would not be able to use the old property
name when removing a property from zookeeper--they could only do that using the
new property name. You had mentioned some concern about automation updating
zookeeper properties, though setting a property using the old name is
supported. Do you have any concerns here? The thinking is for a property
removal it is more dangerous to assume the user meant to remove the new
property name and used the old name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
