[GitHub] [ozone] kerneltime commented on a diff in pull request #3498: HDDS-6841. EC: Validate the server default configuration on Ozone manager startup

[GitBox <gi...@apache.org>]

kerneltime commented on code in PR #3498:
URL: https://github.com/apache/ozone/pull/3498#discussion_r894136806


##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/client/ReplicationConfigValidator.java:
##########
@@ -50,10 +50,11 @@ public void init() {
 
   public ReplicationConfig validate(ReplicationConfig replicationConfig) {
     if (validationRegexp != null) {
-      if (!validationRegexp.matcher(replicationConfig.toString()).matches()) {
+      if (!validationRegexp.matcher(

Review Comment:
   The change looks good. A minor observation: We accept a regular expression as a configuration to limit the replication configurations, it would be useful to validate the regular expression? If we only support a subset of replication configurations would it be better to not validate via string comparison as we already have `ReplicationConfiguration` which has the full fidelity of the configuration already parsed into its constituents, it would be better to validate semantically vs via regular expressions.



-- 
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: issues-unsubscribe@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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