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 2020/09/23 05:31:03 UTC

[GitHub] [shardingsphere] xbkaishui opened a new pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

xbkaishui opened a new pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574


   Fixes #7559.
   
   Changes proposed in this pull request:
   - change YamlRuleSchemaMetaData same with  RuleSchemaMetaData


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



[GitHub] [shardingsphere] coveralls edited a comment on pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#issuecomment-697181953


   ## Pull Request Test Coverage Report for [Build 15007](https://coveralls.io/builds/33661976)
   
   * **4** of **4**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
   * **2** unchanged lines in **1** file lost coverage.
   * Overall coverage decreased (**-0.005%**) to **35.109%**
   
   ---
   
   
   |  Files with Coverage Reduction | New Missed Lines | % |
   | :-----|--------------|--: |
   | [shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/metadata/table/TableMetaData.java](https://coveralls.io/builds/33661976/source?filename=shardingsphere-sql-parser%2Fshardingsphere-sql-parser-binder%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fshardingsphere%2Fsql%2Fparser%2Fbinder%2Fmetadata%2Ftable%2FTableMetaData.java#L52) | 2 | 92.86% |
   <!-- | **Total:** | **2** |  | -->
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/33661976/badge)](https://coveralls.io/builds/33661976) |
   | :-- | --: |
   | Change from base [Build 15003](https://coveralls.io/builds/33654393): |  -0.005% |
   | Covered Lines: | 36628 |
   | Relevant Lines: | 104326 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


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



[GitHub] [shardingsphere] xbkaishui commented on a change in pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#discussion_r493374607



##########
File path: shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/resources/yaml/metadata.yaml
##########
@@ -30,15 +30,4 @@ configuredSchemaMetaData:
             name: PRIMARY          
 unconfiguredSchemaMetaDataMap:
   ds_0:
-    tables:
-      t_user:
-        columns:
-          id:
-            caseSensitive: false
-            dataType: 0
-            generated: false
-            name: id
-            primaryKey: true
-        indexes:
-          primary:
-            name: PRIMARY
+    - t_user

Review comment:
       done

##########
File path: shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-common/src/main/java/org/apache/shardingsphere/governance/core/yaml/config/metadata/YamlRuleSchemaMetaData.java
##########
@@ -32,5 +33,5 @@
 
     private YamlSchemaMetaData configuredSchemaMetaData;
 
-    private Map<String, YamlSchemaMetaData> unconfiguredSchemaMetaDataMap;
+    private Map<String, Collection<String>> unconfiguredSchemaMetaDataMap;

Review comment:
       done




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



[GitHub] [shardingsphere] coveralls edited a comment on pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#issuecomment-697181953


   ## Pull Request Test Coverage Report for [Build 15016](https://coveralls.io/builds/33683143)
   
   * **4** of **4**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage decreased (**-0.006%**) to **35.105%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/33683143/badge)](https://coveralls.io/builds/33683143) |
   | :-- | --: |
   | Change from base [Build 15013](https://coveralls.io/builds/33680530): |  -0.006% |
   | Covered Lines: | 36621 |
   | Relevant Lines: | 104317 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


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



[GitHub] [shardingsphere] coveralls commented on pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#issuecomment-697181953


   ## Pull Request Test Coverage Report for [Build 15005](https://coveralls.io/builds/33656465)
   
   * **2** of **2**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage decreased (**-0.004%**) to **35.11%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/33656465/badge)](https://coveralls.io/builds/33656465) |
   | :-- | --: |
   | Change from base [Build 15003](https://coveralls.io/builds/33654393): |  -0.004% |
   | Covered Lines: | 36632 |
   | Relevant Lines: | 104334 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


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



[GitHub] [shardingsphere] tristaZero commented on a change in pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
tristaZero commented on a change in pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#discussion_r493332797



##########
File path: shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-common/src/main/java/org/apache/shardingsphere/governance/core/yaml/config/metadata/YamlRuleSchemaMetaData.java
##########
@@ -32,5 +33,5 @@
 
     private YamlSchemaMetaData configuredSchemaMetaData;
 
-    private Map<String, YamlSchemaMetaData> unconfiguredSchemaMetaDataMap;
+    private Map<String, Collection<String>> unconfiguredSchemaMetaDataMap;

Review comment:
       This PR looks great. However, I found another issue relevant to what you modified. 
   
   ```java
   // CreateTableStatementMetaDataRefreshStrategy
   
   private void refreshUnconfiguredMetaData(final ShardingSphereMetaData metaData, 
                                                final DatabaseType databaseType, final Map<String, DataSource> dataSourceMap, final String tableName) throws SQLException {
           for (Entry<String, DataSource> entry : dataSourceMap.entrySet()) {
               Optional<TableMetaData> tableMetaData = TableMetaDataLoader.loadWithoutColumnMetaData(entry.getValue(), tableName, databaseType.getName());
               if (tableMetaData.isPresent()) {
                   refreshUnconfiguredMetaData(metaData, tableName, entry.getKey());
                   return;
               }
           }
       }
   
   ```
   `loadWithoutColumnMetaData` is supposed to be simplified, IMO. What do you think?

##########
File path: shardingsphere-governance/shardingsphere-governance-core/shardingsphere-governance-core-config/src/test/resources/yaml/metadata.yaml
##########
@@ -30,15 +30,4 @@ configuredSchemaMetaData:
             name: PRIMARY          
 unconfiguredSchemaMetaDataMap:
   ds_0:
-    tables:
-      t_user:
-        columns:
-          id:
-            caseSensitive: false
-            dataType: 0
-            generated: false
-            name: id
-            primaryKey: true
-        indexes:
-          primary:
-            name: PRIMARY
+    - t_user

Review comment:
       Please add a blank line under this line.




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



[GitHub] [shardingsphere] coveralls edited a comment on pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#issuecomment-697181953


   ## Pull Request Test Coverage Report for [Build 15016](https://coveralls.io/builds/33683143)
   
   * **4** of **4**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage decreased (**-0.006%**) to **35.105%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/33683143/badge)](https://coveralls.io/builds/33683143) |
   | :-- | --: |
   | Change from base [Build 15013](https://coveralls.io/builds/33680530): |  -0.006% |
   | Covered Lines: | 36621 |
   | Relevant Lines: | 104317 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


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



[GitHub] [shardingsphere] coveralls edited a comment on pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574#issuecomment-697181953


   ## Pull Request Test Coverage Report for [Build 15015](https://coveralls.io/builds/33681562)
   
   * **4** of **4**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
   * No unchanged relevant lines lost coverage.
   * Overall coverage decreased (**-0.009%**) to **35.105%**
   
   ---
   
   
   
   |  Totals | [![Coverage Status](https://coveralls.io/builds/33681562/badge)](https://coveralls.io/builds/33681562) |
   | :-- | --: |
   | Change from base [Build 15003](https://coveralls.io/builds/33654393): |  -0.009% |
   | Covered Lines: | 36621 |
   | Relevant Lines: | 104317 |
   
   ---
   ##### 💛  - [Coveralls](https://coveralls.io)
   


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



[GitHub] [shardingsphere] tristaZero merged pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574


   


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



[GitHub] [shardingsphere] tristaZero merged pull request #7574: Refactor change YamlRuleSchemaMetaData same with RuleSchemaMetaData

Posted by GitBox <gi...@apache.org>.
tristaZero merged pull request #7574:
URL: https://github.com/apache/shardingsphere/pull/7574


   


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