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]