You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dolphinscheduler.apache.org by GitBox <gi...@apache.org> on 2021/08/23 12:42:41 UTC

[GitHub] [dolphinscheduler] rumesh-perera opened a new pull request #6027: [COMDEV-412] [WIP] Configuration coverage

rumesh-perera opened a new pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027


   ## Purpose of the pull request
   
   This PR addresses https://github.com/apache/dolphinscheduler/issues/5164
   
   ## Brief change log
   
   <!--*(for example:)*
     - *Add maven-checkstyle-plugin to root pom.xml*
   -->
   ## Verify this pull request
   
   <!--*(Please pick either of the following options)*-->
   
   This pull request is code cleanup without any test coverage.
   
   *(or)*
   
   This pull request is already covered by existing tests, such as *(please describe tests)*.
   
   (or)
   
   This change added tests and can be verified as follows:
   
   <!--*(example:)*
     - *Added dolphinscheduler-dao tests for end-to-end.*
     - *Added CronUtilsTest to verify the change.*
     - *Manually verified the change by testing locally.* -->
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] codecov-commenter commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-903762640


   # [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#6027](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (dc7073f) into [dev](https://codecov.io/gh/apache/dolphinscheduler/commit/16986c3c651af38469c6d4cb03a587fd174c9a9b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16986c3) will **increase** coverage by `0.87%`.
   > The diff coverage is `45.49%`.
   
   > :exclamation: Current head dc7073f differs from pull request most recent head ad814c9. Consider uploading reports for the commit ad814c9 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/graphs/tree.svg?width=650&height=150&src=pr&token=bv9iXXRLi9&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##                dev    #6027      +/-   ##
   ============================================
   + Coverage     45.42%   46.30%   +0.87%     
   - Complexity     3694     3716      +22     
   ============================================
     Files           607      610       +3     
     Lines         24872    24880       +8     
     Branches       2834     2838       +4     
   ============================================
   + Hits          11299    11521     +222     
   + Misses        12484    12230     -254     
   - Partials       1089     1129      +40     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...cheduler/api/controller/AccessTokenController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWNjZXNzVG9rZW5Db250cm9sbGVyLmphdmE=) | `6.66% <0.00%> (ø)` | |
   | [...scheduler/api/controller/AlertGroupController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWxlcnRHcm91cENvbnRyb2xsZXIuamF2YQ==) | `8.33% <0.00%> (+0.33%)` | :arrow_up: |
   | [.../api/controller/AlertPluginInstanceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvQWxlcnRQbHVnaW5JbnN0YW5jZUNvbnRyb2xsZXIuamF2YQ==) | `8.00% <0.00%> (+0.30%)` | :arrow_up: |
   | [...heduler/api/controller/DataAnalysisController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YUFuYWx5c2lzQ29udHJvbGxlci5qYXZh) | `9.09% <ø> (ø)` | |
   | [...scheduler/api/controller/DataSourceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRGF0YVNvdXJjZUNvbnRyb2xsZXIuamF2YQ==) | `4.34% <0.00%> (+0.18%)` | :arrow_up: |
   | [...inscheduler/api/controller/ExecutorController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvRXhlY3V0b3JDb250cm9sbGVyLmphdmE=) | `8.33% <ø> (ø)` | |
   | [...uler/api/controller/ProcessInstanceController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUHJvY2Vzc0luc3RhbmNlQ29udHJvbGxlci5qYXZh) | `4.54% <0.00%> (ø)` | |
   | [...nscheduler/api/controller/ResourcesController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvUmVzb3VyY2VzQ29udHJvbGxlci5qYXZh) | `3.57% <0.00%> (ø)` | |
   | [...nscheduler/api/controller/SchedulerController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvU2NoZWR1bGVyQ29udHJvbGxlci5qYXZh) | `4.76% <0.00%> (ø)` | |
   | [...lphinscheduler/api/controller/UsersController.java](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZG9scGhpbnNjaGVkdWxlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2RvbHBoaW5zY2hlZHVsZXIvYXBpL2NvbnRyb2xsZXIvVXNlcnNDb250cm9sbGVyLmphdmE=) | `4.16% <0.00%> (ø)` | |
   | ... and [94 more](https://codecov.io/gh/apache/dolphinscheduler/pull/6027/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [16986c3...ad814c9](https://codecov.io/gh/apache/dolphinscheduler/pull/6027?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] github-actions[bot] commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-1008453578


   This pull request has been closed because it has not had recent activity. You could reopen it if you try to continue your work, and anyone who are interested in it are encouraged to continue work on this pull request.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] CalvinKirs commented on a change in pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on a change in pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#discussion_r694107027



##########
File path: dolphinscheduler-server/src/test/java/org/apache/dolphinscheduler/server/config/ConfigProviderTests.java
##########
@@ -0,0 +1,70 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.server.config;
+
+import org.apache.dolphinscheduler.common.config.ConfigProvider;
+import org.apache.dolphinscheduler.common.config.validators.ValidationConstants;
+import org.apache.dolphinscheduler.common.exception.ValidationException;
+import org.junit.Test;
+
+import static org.junit.Assert.assertTrue;
+
+public class ConfigProviderTests {

Review comment:
       UT needs to show the declaration in the root pom file




-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] github-actions[bot] closed pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027


   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] CalvinKirs commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-903904500


   You can refer to this document to solve the problem of code style 
   http://dolphinscheduler.apache.org/en-us/community/development/pull-request.html


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] github-actions[bot] commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-1003800695


   This pull request has been automatically marked as stale because it has not had recent activity for 120 days. It will be closed in 7 days if no further activity occurs.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] CalvinKirs commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-903902442


   Hi, as we talked about before, if the parameters have a certain scope of action, such as some parameters only accept jvm system parameters injected, you know, such parameters usually only operations and maintenance personnel have access, and developers even if set will not take effect. I'm wondering if this will be considered in this PR or subsequent PRs.


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] sonarcloud[bot] commented on pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#issuecomment-903767244


   SonarCloud Quality Gate failed.&nbsp; &nbsp; ![Quality Gate failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/failed-16px.png 'Quality Gate failed')
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=BUG) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=CODE_SMELL) [11 Code Smells](https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&pullRequest=6027&resolved=false&types=CODE_SMELL)
   
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/0-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6027&metric=new_coverage&view=list) [0.0% Coverage](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6027&metric=new_coverage&view=list)  
   [![0.0%](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3-16px.png '0.0%')](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6027&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache-dolphinscheduler&pullRequest=6027&metric=new_duplicated_lines_density&view=list)
   
   


-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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

[GitHub] [dolphinscheduler] ruanwenjun commented on a change in pull request #6027: [COMDEV-412] [WIP] Configuration coverage

Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on a change in pull request #6027:
URL: https://github.com/apache/dolphinscheduler/pull/6027#discussion_r702287679



##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/config/ConfigValidationProvider.java
##########
@@ -0,0 +1,57 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.common.config;
+
+import org.apache.dolphinscheduler.common.config.validators.PropertyValidator;
+import org.apache.dolphinscheduler.common.config.validators.impl.worker.ExecThreadsValidator;
+import org.apache.dolphinscheduler.common.config.validators.impl.worker.ListenPortValidator;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.apache.dolphinscheduler.common.config.validators.ValidationConstants.WORKER_EXEC_THREADS_KEY;
+import static org.apache.dolphinscheduler.common.config.validators.ValidationConstants.WORKER_LISTEN_PORT_KEY;
+
+/**
+ * Configuration Validation provider class.
+ */
+public class ConfigValidationProvider {
+
+  private static ConfigValidationProvider instance;
+  private static Map<String, PropertyValidator> validatorMapper = new HashMap<>();
+
+  static {
+    validatorMapper.put(WORKER_LISTEN_PORT_KEY, new ListenPortValidator());
+    validatorMapper.put(WORKER_EXEC_THREADS_KEY, new ExecThreadsValidator());
+  }

Review comment:
       Your design is ok, but can we use the common validator to validate the value? For example, provide IntegerValidator, StringValidator, BooleanValidator..., since the validate rule is similarity.
   
   In the current design, we need to write a Validator for each config key.

##########
File path: dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/config/validators/impl/worker/ExecThreadsValidator.java
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.common.config.validators.impl.worker;
+
+import org.apache.dolphinscheduler.common.config.validators.PropertyValidator;
+import org.apache.dolphinscheduler.common.config.validators.ValidationConstants;
+import org.apache.dolphinscheduler.common.exception.ValidationException;
+
+/**
+ * Worker execution thread validator.
+ */
+public class ExecThreadsValidator implements PropertyValidator<Integer> {
+
+  public String getPropertyKey() {
+    return ValidationConstants.WORKER_EXEC_THREADS_KEY;
+  }
+
+  public Integer getPropertyValue(String value) {
+    return Integer.valueOf(value);
+  }
+
+  public void validate(String value) {
+    int port = Integer.valueOf(value);

Review comment:
       This method will throw NumberFormatException, should we need wrapper this exception to ValidationException?




-- 
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: commits-unsubscribe@dolphinscheduler.apache.org

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