dlmarion commented on a change in pull request #2569:
URL: https://github.com/apache/accumulo/pull/2569#discussion_r828238435



##########
File path: 
server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java
##########
@@ -18,48 +18,40 @@
  */
 package org.apache.accumulo.server.util;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 
-import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.conf.Property;
 import org.apache.accumulo.core.data.TableId;
-import org.apache.accumulo.fate.zookeeper.ZooReaderWriter;
-import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeExistsPolicy;
-import org.apache.accumulo.fate.zookeeper.ZooUtil.NodeMissingPolicy;
 import org.apache.accumulo.server.ServerContext;
+import org.apache.accumulo.server.conf.store.PropCacheId;
 import org.apache.zookeeper.KeeperException;
 
 public class TablePropUtil {
 
-  public static boolean setTableProperty(ServerContext context, TableId 
tableId, String property,
-      String value) throws KeeperException, InterruptedException {
-    return setTableProperty(context.getZooReaderWriter(), 
context.getZooKeeperRoot(), tableId,
-        property, value);
-  }
-
-  public static boolean setTableProperty(ZooReaderWriter zoo, String zkRoot, 
TableId tableId,
-      String property, String value) throws KeeperException, 
InterruptedException {
-    if (!Property.isTablePropertyValid(property, value))
-      return false;
-
-    // create the zk node for per-table properties for this table if it 
doesn't already exist
-    String zkTablePath = getTablePath(zkRoot, tableId);
-    zoo.putPersistentData(zkTablePath, new byte[0], NodeExistsPolicy.SKIP);
-
-    // create the zk node for this property and set it's data to the specified 
value
-    String zPath = zkTablePath + "/" + property;
-    zoo.putPersistentData(zPath, value.getBytes(UTF_8), 
NodeExistsPolicy.OVERWRITE);
+  public static boolean setTableProperties(ServerContext context, TableId 
tableId,
+      final Map<String,String> props) throws KeeperException, 
InterruptedException {
+    Map<String,String> tempProps = new HashMap<>(props);
+    tempProps.entrySet().removeIf(e -> 
!Property.isTablePropertyValid(e.getKey(), e.getValue()));
 
+    context.getPropStore().putAll(PropCacheId.forTable(context, tableId), 
props);
     return true;

Review comment:
       This used to return `false` if not a valid table property. 
`NamespacePropUtil.setNamespaceProperty` returns false. 
SystemPropUtil.setSystemProperty throws IllegalArgumentException. Suggest 
making these act the same. It may make sense to have a common interface to 
ensure that the behavior is the same. Something like:
   
   ```
   public interface PropUtil {
   
     void setProperty()
     
     void removeProperty()
   
   }
   ```




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