EdColeman commented on code in PR #2569:
URL: https://github.com/apache/accumulo/pull/2569#discussion_r851267922


##########
server/base/src/main/java/org/apache/accumulo/server/util/TablePropUtil.java:
##########
@@ -18,48 +18,47 @@
  */
 package org.apache.accumulo.server.util;
 
-import static java.nio.charset.StandardCharsets.UTF_8;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
 
-import org.apache.accumulo.core.Constants;
 import org.apache.accumulo.core.conf.Property;
+import org.apache.accumulo.core.data.AbstractId;
 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.zookeeper.KeeperException;
+import org.apache.accumulo.server.conf.store.PropCacheKey;
+import org.apache.accumulo.server.conf.store.PropStoreException;
 
-public class TablePropUtil {
+public class TablePropUtil implements PropUtil {
 
-  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;
+  private TablePropUtil() {}
 
-    // 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);
+  public static TablePropUtil factory() {
+    return new TablePropUtil();
+  }
 
-    // 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);
+  /**
+   * Helper method to set provided properties for the provided table.
+   *
+   * @throws PropStoreException
+   *           if an underlying exception (KeeperException, 
InterruptException) or other failure to
+   *           read properties from the cache / backend store
+   */
+  @Override
+  public void setProperties(ServerContext context, AbstractId<?> tableId,
+      Map<String,String> props) {
+    Map<String,String> tempProps = new HashMap<>(props);
+    // TODO reconcile with NamespacePropUtil on invalid, this ignores, 
namespace throws exception

Review Comment:
   With a TODO - the IDE helps me find it for future reference.  It is 
something that can be done in the future - why is that not a TODO - would it 
help if I created an issue and then referenced that in the todo?



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