elek commented on a change in pull request #536: HDDS-2988. Use 
getPropertiesByPrefix instead of regex in matching ratis client and server 
properties.
URL: https://github.com/apache/hadoop-ozone/pull/536#discussion_r377105756
 
 

 ##########
 File path: 
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
 ##########
 @@ -54,29 +57,34 @@
 import org.apache.ratis.retry.RetryPolicy;
 import org.apache.ratis.rpc.RpcType;
 import org.apache.ratis.rpc.SupportedRpcType;
+import org.apache.ratis.server.RaftServerConfigKeys;
 import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
 import org.apache.ratis.util.TimeDuration;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import static 
org.apache.hadoop.ozone.conf.DatanodeRatisServerConfig.DATANODE_RATIS_SERVER_CONFIG_PREFIX;
-
 /**
  * Ratis helper methods.
  */
 public interface RatisHelper {
   Logger LOG = LoggerFactory.getLogger(RatisHelper.class);
 
-  // Ratis Client and Grpc header regex filters.
-  String RATIS_CLIENT_HEADER_REGEX = "raft\\.client\\.([a-z\\.]+)";
-  String RATIS_GRPC_CLIENT_HEADER_REGEX = "raft\\.grpc\\.(?!server|tls)" +
-      "([a-z\\.]+)";
+  // Prefix for Ratis Server GRPC and Ratis client conf.
+  String HDDS_DATANODE_RATIS_PREFIX_KEY = "datanode.ratis.";
+  String HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY = "datanode.ratis.server";
+  String HDDS_DATANODE_RATIS_CLIENT_PREFIX_KEY = "datanode.ratis.client";
+  String HDDS_DATANODE_RATIS_GRPC_PREFIX_KEY = "datanode.ratis.grpc";
+
 
-  // Ratis Server header regex filter.
-  String RATIS_SERVER_HEADER_REGEX = "datanode\\.ratis\\.raft\\.server\\" +
-      ".([a-z\\.]+)";
-  String RATIS_SERVER_GRPC_HEADER_REGEX = "datanode\\.ratis\\.raft\\.grpc\\" +
-      ".([a-z\\.]+)";
+  String RATIS_SERVER_PREFIX_KEY =
 
 Review comment:
   Do we really need the `server` and `client` part here? Why don't we use 
`datanode.ratis.raft.client.*` and `datanode.ratis.raft.server.*` instead of 
`datanode.ratis.server.raft.server`? Is there any use case where we need to 
configure the client configs on the server side? 
(`datanode.ratis.server.raft.client.*`)
   
   BTW, this is just a matter of taste but I would prefer to avoid substring. 
This seems to be more readable for me:
   
   ```
     String HDDS_DATANODE_RATIS_SERVER_TAG = "server";
     
   String HDDS_DATANODE_RATIS_PREFIX_KEY = "datanode.ratis.";
   
     String HDDS_DATANODE_RATIS_SERVER_PREFIX_KEY =
         HDDS_DATANODE_RATIS_PREFIX_KEY + "." HDDS_DATANODE_RATIS_SERVER_TAG;
   
   
   String RATIS_SERVER_PREFIX_KEY =  RATIS_SERVER_PREFIX_KEY +"." + 
RaftServerConfigKeys.PREFIX;
   ```
    

----------------------------------------------------------------
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:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: ozone-issues-h...@hadoop.apache.org

Reply via email to