You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2022/04/08 11:23:51 UTC

[GitHub] [shardingsphere] ShiFengCui opened a new pull request, #16667: Add verification of config.yaml configuration to provide user-friendly prompts

ShiFengCui opened a new pull request, #16667:
URL: https://github.com/apache/shardingsphere/pull/16667

   Fixes #16404.
   
   Changes proposed in this pull request:
   - Add verification of config.yaml configuration to provide user-friendly prompts
   


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] totalo merged pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
totalo merged PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] totalo commented on a diff in pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
totalo commented on code in PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#discussion_r846746817


##########
examples/shardingsphere-example-generator/src/main/java/org/apache/shardingsphere/example/generator/core/yaml/config/YamlExampleConfigurationValidator.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.shardingsphere.example.generator.core.yaml.config;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
+
+/**
+ * Example configuration validator.
+ */
+public class YamlExampleConfigurationValidator {

Review Comment:
   plz add final



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] totalo commented on pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
totalo commented on PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#issuecomment-1093044025

   hi @ShiFengCui ,Thanks for your contribution, here are some questions, please check.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] totalo commented on a diff in pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
totalo commented on code in PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#discussion_r846277690


##########
examples/shardingsphere-example-generator/src/main/java/org/apache/shardingsphere/example/generator/core/ExampleGeneratorFactory.java:
##########
@@ -53,13 +54,14 @@ private Configuration createTemplateConfiguration() throws IOException {
     
     /**
      * Generate directories and files by template.
-     * 
+     *
      * @throws TemplateException template exception
      * @throws IOException IO exception
      */
     @SuppressWarnings("unchecked")
     public void generate() throws TemplateException, IOException {
         YamlExampleConfiguration exampleConfiguration = swapConfigToObject();
+        new YamlExampleConfigurationValidator().validate(exampleConfiguration);

Review Comment:
   Maybe the tool class is more suitable, we call the static method for verification



##########
examples/shardingsphere-example-generator/src/main/java/org/apache/shardingsphere/example/generator/core/yaml/config/YamlExampleConfigurationValidator.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.shardingsphere.example.generator.core.yaml.config;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+/**
+ * Example configuration validator.
+ */
+public class YamlExampleConfigurationValidator {
+    
+    /**
+     * Verify the entrance.
+     *
+     * @param configuration configuration
+     */
+    public void validate(final YamlExampleConfiguration configuration) {
+        Map<String, ConfigurationData> valueMap = new HashMap<>();
+        valueMap.put("products", new ConfigurationData(configuration.getProducts(), Sets.newHashSet("jdbc", "proxy")));

Review Comment:
   I think these can be abstracted into enumeration classes, or static properties



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] ShiFengCui commented on a diff in pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
ShiFengCui commented on code in PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#discussion_r846596540


##########
examples/shardingsphere-example-generator/src/main/java/org/apache/shardingsphere/example/generator/core/ExampleGeneratorFactory.java:
##########
@@ -53,13 +54,14 @@ private Configuration createTemplateConfiguration() throws IOException {
     
     /**
      * Generate directories and files by template.
-     * 
+     *
      * @throws TemplateException template exception
      * @throws IOException IO exception
      */
     @SuppressWarnings("unchecked")
     public void generate() throws TemplateException, IOException {
         YamlExampleConfiguration exampleConfiguration = swapConfigToObject();
+        new YamlExampleConfigurationValidator().validate(exampleConfiguration);

Review Comment:
   ok



##########
examples/shardingsphere-example-generator/src/main/java/org/apache/shardingsphere/example/generator/core/yaml/config/YamlExampleConfigurationValidator.java:
##########
@@ -0,0 +1,89 @@
+/*
+ * 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.shardingsphere.example.generator.core.yaml.config;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
+import lombok.Getter;
+import lombok.Setter;
+
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+/**
+ * Example configuration validator.
+ */
+public class YamlExampleConfigurationValidator {
+    
+    /**
+     * Verify the entrance.
+     *
+     * @param configuration configuration
+     */
+    public void validate(final YamlExampleConfiguration configuration) {
+        Map<String, ConfigurationData> valueMap = new HashMap<>();
+        valueMap.put("products", new ConfigurationData(configuration.getProducts(), Sets.newHashSet("jdbc", "proxy")));

Review Comment:
   ok



-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] totalo commented on pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
totalo commented on PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#issuecomment-1094227678

   hi, @ShiFengCui ,Thanks for you nice job! There is one more small problem here, I will merge him after it is fixed.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] ShiFengCui commented on pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
ShiFengCui commented on PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#issuecomment-1093786368

   > 嗨@ShiFengCui,感谢您的贡献,这里有一些问题,请检查。
   
   OK, I got it. I'll adjust it


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

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


[GitHub] [shardingsphere] codecov-commenter commented on pull request #16667: Add verification of config.yaml configuration to provide user-friendly prompts

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #16667:
URL: https://github.com/apache/shardingsphere/pull/16667#issuecomment-1094211867

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/16667?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 [#16667](https://codecov.io/gh/apache/shardingsphere/pull/16667?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b895562) into [master](https://codecov.io/gh/apache/shardingsphere/commit/a7aaf79a82dc61e29a7c89efc5bde52b50d4ef01?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a7aaf79) will **increase** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #16667      +/-   ##
   ============================================
   + Coverage     60.55%   60.57%   +0.01%     
   - Complexity     2099     2112      +13     
   ============================================
     Files          3382     3378       -4     
     Lines         50882    50798      -84     
     Branches       8737     8721      -16     
   ============================================
   - Hits          30814    30772      -42     
   + Misses        17477    17435      -42     
     Partials       2591     2591              
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/16667?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [.../datasource/config/yaml/YamlJdbcConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvYXBpL2RhdGFzb3VyY2UvY29uZmlnL3lhbWwvWWFtbEpkYmNDb25maWd1cmF0aW9uLmphdmE=) | `33.33% <0.00%> (-66.67%)` | :arrow_down: |
   | [.../check/datasource/PostgreSQLDataSourceChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9wb3N0Z3Jlc3FsL2NoZWNrL2RhdGFzb3VyY2UvUG9zdGdyZVNRTERhdGFTb3VyY2VDaGVja2VyLmphdmE=) | `75.00% <0.00%> (-10.72%)` | :arrow_down: |
   | [.../frontend/postgresql/PostgreSQLFrontendEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9Qb3N0Z3JlU1FMRnJvbnRlbmRFbmdpbmUuamF2YQ==) | `57.14% <0.00%> (-7.15%)` | :arrow_down: |
   | [...adata/schema/loader/TableMetaDataLoaderEngine.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9tZXRhZGF0YS9zY2hlbWEvbG9hZGVyL1RhYmxlTWV0YURhdGFMb2FkZXJFbmdpbmUuamF2YQ==) | `38.70% <0.00%> (-5.41%)` | :arrow_down: |
   | [...hardingsphere/spi/type/typed/TypedSPIRegistry.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcGkvdHlwZS90eXBlZC9UeXBlZFNQSVJlZ2lzdHJ5LmphdmE=) | `92.59% <0.00%> (-3.57%)` | :arrow_down: |
   | [...uery/ShowScalingCheckAlgorithmsQueryResultSet.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctZGlzdHNxbC9zaGFyZGluZ3NwaGVyZS1zaGFyZGluZy1kaXN0c3FsLWhhbmRsZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3NjYWxpbmcvZGlzdHNxbC9oYW5kbGVyL3F1ZXJ5L1Nob3dTY2FsaW5nQ2hlY2tBbGdvcml0aG1zUXVlcnlSZXN1bHRTZXQuamF2YQ==) | `86.66% <0.00%> (-0.84%)` | :arrow_down: |
   | [.../pipeline/mysql/ingest/MySQLIncrementalDumper.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvbXlzcWwvaW5nZXN0L015U1FMSW5jcmVtZW50YWxEdW1wZXIuamF2YQ==) | `73.56% <0.00%> (-0.07%)` | :arrow_down: |
   | [...phere/spi/type/singleton/SingletonSPIRegistry.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtc3BpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9zcGkvdHlwZS9zaW5nbGV0b24vU2luZ2xldG9uU1BJUmVnaXN0cnkuamF2YQ==) | `100.00% <0.00%> (ø)` | |
   | [...sphere/proxy/initializer/BootstrapInitializer.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktYm9vdHN0cmFwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9wcm94eS9pbml0aWFsaXplci9Cb290c3RyYXBJbml0aWFsaXplci5qYXZh) | `0.00% <0.00%> (ø)` | |
   | [.../datasource/registry/GlobalDataSourceRegistry.java](https://codecov.io/gh/apache/shardingsphere/pull/16667/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-c2hhcmRpbmdzcGhlcmUtaW5mcmEvc2hhcmRpbmdzcGhlcmUtaW5mcmEtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9pbmZyYS9kYXRhc291cmNlL3JlZ2lzdHJ5L0dsb2JhbERhdGFTb3VyY2VSZWdpc3RyeS5qYXZh) | `0.00% <0.00%> (ø)` | |
   | ... and [50 more](https://codecov.io/gh/apache/shardingsphere/pull/16667/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/shardingsphere/pull/16667?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/shardingsphere/pull/16667?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 [a7aaf79...b895562](https://codecov.io/gh/apache/shardingsphere/pull/16667?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: notifications-unsubscribe@shardingsphere.apache.org

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