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/07/22 02:02:41 UTC

[GitHub] [shardingsphere] huangdx0726 opened a new pull request, #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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

   … and PrepareTargetTablesParameter
   
   Fixes #19440 .
   
   Changes proposed in 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: notifications-unsubscribe@shardingsphere.apache.org

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

[GitHub] [shardingsphere] huangdx0726 commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter

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

   I see
   
   
   
   ---Original---
   From: "Hongsheng ***@***.***&gt;
   Date: Thu, Jul 28, 2022 09:59 AM
   To: ***@***.***&gt;;
   Cc: "Da Xiang ***@***.******@***.***&gt;;
   Subject: Re: [apache/shardingsphere] Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter (PR #19455)
   
   
   
   
    
   Thanks. You need to comment on the issue firstly, else could not assign to you.
    
   —
   Reply to this email directly, view it on GitHub, or unsubscribe.
   You are receiving this because you were mentioned.Message ID: ***@***.***&gt;


-- 
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] sandynz commented on a diff in pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/rulealtered/RuleAlteredJobPreparer.java:
##########
@@ -142,10 +141,19 @@ private void prepareTarget(final RuleAlteredJobContext jobContext) {
             log.info("dataSourcePreparer null, ignore prepare target");
             return;
         }
+        DatabaseType sourceDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getSourceDatabaseType());
+        DatabaseType targetDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getTargetDatabaseType());
+        if (!sourceDatabaseType.isSchemaAvailable() || !targetDatabaseType.isSchemaAvailable()) {
+            log.info("prepareTargetSchemas, one of source or target database type schema is not available, ignore");
+            return;
+        }

Review Comment:
   For example:
   ```
   private boolean isSourceAndTargetSchemaAvailable(... jobConfig) {
   ...
   }
   ```
   Then it could be used in `prepareTarget`:
   ```
   if (isSourceAndTargetSchemaAvailable(...)) {
       dataSourcePreparer.get().prepareTargetSchemas(prepareTargetSchemasParameter);
   }
   ```
   



-- 
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] sandynz commented on a diff in pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/prepare/datasource/PrepareTargetSchemasParameter.java:
##########
@@ -30,7 +30,7 @@
 @Getter
 public final class PrepareTargetSchemasParameter {
     
-    private final TaskConfiguration taskConfig;
+    private final RuleAlteredJobConfiguration jobConfig;
     

Review Comment:
   It's better not depend on RuleAlteredJobConfiguration, since it's dedicated for a type of job. PrepareTargetSchemasParameter is planned for common usage (it might be used by several types of jobs).
   
   For example:
   - Add logicTableNames in PrepareTargetSchemasParameter to replace parameter.getJobConfig().splitLogicTableNames()
   - Extract sourceDatabaseType.isSchemaAvailable() to RuleAlteredJobPreparer
   
   And also PrepareTargetTablesParameter.



-- 
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] sandynz commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter

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

   Thanks. You need to comment on the issue firstly, else could not assign to you.


-- 
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] sandynz merged pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter

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


-- 
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] sandynz commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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

   And there's github action CI failure, you could try NATIVE-mysql on local machine, refer to [Scaling integration test]( https://shardingsphere.apache.org/document/current/cn/test-manual/scaling-integration-test/ ), and debug on local proxy.
   


-- 
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] huangdx0726 commented on a diff in pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/prepare/datasource/PrepareTargetSchemasParameter.java:
##########
@@ -30,7 +30,7 @@
 @Getter
 public final class PrepareTargetSchemasParameter {
     
-    private final TaskConfiguration taskConfig;
+    private final RuleAlteredJobConfiguration jobConfig;
     

Review Comment:
   get 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] sandynz commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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

   Hi @huangdx0726 , some errors could be checked:
   
   1, There's error in Continuous Integration:
   ```
   [INFO] There is 1 error reported by Checkstyle 8.19 with src/resources/checkstyle_ci.xml ruleset.
   Error:  src/main/java/org/apache/shardingsphere/data/pipeline/scenario/rulealtered/RuleAlteredJobPreparer.java:[156,54] (misc) FinalParameters: Parameter jobConfig should be final.
   ```
   
   2, There's error in Scaling Integration Test, could you merge latest upstream master? It should be fixed on master branch.
   


-- 
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 #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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

   # [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/19455?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 [#19455](https://codecov.io/gh/apache/shardingsphere/pull/19455?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (75d3913) into [master](https://codecov.io/gh/apache/shardingsphere/commit/f1182e80d1c34a6d885cfca7899fc3228bec64b1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f1182e8) will **increase** coverage by `0.00%`.
   > The diff coverage is `63.15%`.
   
   > :exclamation: Current head 75d3913 differs from pull request most recent head 3385728. Consider uploading reports for the commit 3385728 to get more accurate results
   
   ```diff
   @@            Coverage Diff            @@
   ##             master   #19455   +/-   ##
   =========================================
     Coverage     59.87%   59.88%           
     Complexity     2383     2383           
   =========================================
     Files          3822     3822           
     Lines         54444    54464   +20     
     Branches       7622     7627    +5     
   =========================================
   + Hits          32600    32616   +16     
   - Misses        19054    19058    +4     
     Partials       2790     2790           
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/19455?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...prepare/datasource/AbstractDataSourcePreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvcHJlcGFyZS9kYXRhc291cmNlL0Fic3RyYWN0RGF0YVNvdXJjZVByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...epare/datasource/PrepareTargetTablesParameter.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL2NvcmUvcHJlcGFyZS9kYXRhc291cmNlL1ByZXBhcmVUYXJnZXRUYWJsZXNQYXJhbWV0ZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...e/scenario/rulealtered/RuleAlteredJobPreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL3NjZW5hcmlvL3J1bGVhbHRlcmVkL1J1bGVBbHRlcmVkSm9iUHJlcGFyZXIuamF2YQ==) | `0.00% <0.00%> (ø)` | |
   | [...ql/prepare/datasource/MySQLDataSourcePreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtbXlzcWwvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2RhdGEvcGlwZWxpbmUvbXlzcWwvcHJlcGFyZS9kYXRhc291cmNlL015U1FMRGF0YVNvdXJjZVByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...repare/datasource/OpenGaussDataSourcePreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtb3BlbmdhdXNzL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9zaGFyZGluZ3NwaGVyZS9kYXRhL3BpcGVsaW5lL29wZW5nYXVzcy9wcmVwYXJlL2RhdGFzb3VyY2UvT3BlbkdhdXNzRGF0YVNvdXJjZVByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [...epare/datasource/PostgreSQLDataSourcePreparer.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUta2VybmVsL3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUvc2hhcmRpbmdzcGhlcmUtZGF0YS1waXBlbGluZS1kaWFsZWN0L3NoYXJkaW5nc3BoZXJlLWRhdGEtcGlwZWxpbmUtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZGF0YS9waXBlbGluZS9wb3N0Z3Jlc3FsL3ByZXBhcmUvZGF0YXNvdXJjZS9Qb3N0Z3JlU1FMRGF0YVNvdXJjZVByZXBhcmVyLmphdmE=) | `0.00% <0.00%> (ø)` | |
   | [.../postgresql/command/query/extended/JDBCPortal.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUtcHJveHkvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQvc2hhcmRpbmdzcGhlcmUtcHJveHktZnJvbnRlbmQtcG9zdGdyZXNxbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvcHJveHkvZnJvbnRlbmQvcG9zdGdyZXNxbC9jb21tYW5kL3F1ZXJ5L2V4dGVuZGVkL0pEQkNQb3J0YWwuamF2YQ==) | `59.30% <50.00%> (+0.68%)` | :arrow_up: |
   | [...actReadwriteSplittingRuleConfigurationChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtcmVhZHdyaXRlLXNwbGl0dGluZy9zaGFyZGluZ3NwaGVyZS1yZWFkd3JpdGUtc3BsaXR0aW5nLWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL3JlYWR3cml0ZXNwbGl0dGluZy9jaGVja2VyL0Fic3RyYWN0UmVhZHdyaXRlU3BsaXR0aW5nUnVsZUNvbmZpZ3VyYXRpb25DaGVja2VyLmphdmE=) | `100.00% <100.00%> (+20.00%)` | :arrow_up: |
   | [...cker/AbstractShardingRuleConfigurationChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvY2hlY2tlci9BYnN0cmFjdFNoYXJkaW5nUnVsZUNvbmZpZ3VyYXRpb25DaGVja2VyLmphdmE=) | `73.68% <100.00%> (+5.94%)` | :arrow_up: |
   | [...rithmProvidedShardingRuleConfigurationChecker.java](https://codecov.io/gh/apache/shardingsphere/pull/19455/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-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmcvc2hhcmRpbmdzcGhlcmUtc2hhcmRpbmctY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvc2hhcmRpbmcvY2hlY2tlci9BbGdvcml0aG1Qcm92aWRlZFNoYXJkaW5nUnVsZUNvbmZpZ3VyYXRpb25DaGVja2VyLmphdmE=) | `100.00% <100.00%> (ø)` | |
   | ... and [7 more](https://codecov.io/gh/apache/shardingsphere/pull/19455/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) | |
   
   Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?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

[GitHub] [shardingsphere] huangdx0726 commented on a diff in pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/rulealtered/RuleAlteredJobPreparer.java:
##########
@@ -142,10 +141,19 @@ private void prepareTarget(final RuleAlteredJobContext jobContext) {
             log.info("dataSourcePreparer null, ignore prepare target");
             return;
         }
+        DatabaseType sourceDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getSourceDatabaseType());
+        DatabaseType targetDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getTargetDatabaseType());
+        if (!sourceDatabaseType.isSchemaAvailable() || !targetDatabaseType.isSchemaAvailable()) {
+            log.info("prepareTargetSchemas, one of source or target database type schema is not available, ignore");
+            return;
+        }

Review Comment:
   Could you tell me the idea for the method？



-- 
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] sandynz commented on a diff in pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter…

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


##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/prepare/datasource/PrepareTargetSchemasParameter.java:
##########
@@ -30,7 +32,13 @@
 @Getter
 public final class PrepareTargetSchemasParameter {
     
-    private final TaskConfiguration taskConfig;
+    private final List<String> logicTableNames;
+    
+    private final DatabaseType targetDatabaseType;
+    
+    private final String databaseName;
+    
+    private final PipelineDataSourceConfiguration pipelineDataSourceConfiguration;

Review Comment:
   `pipelineDataSourceConfiguration` could be `dataSourceConfig`



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/scenario/rulealtered/RuleAlteredJobPreparer.java:
##########
@@ -142,10 +141,19 @@ private void prepareTarget(final RuleAlteredJobContext jobContext) {
             log.info("dataSourcePreparer null, ignore prepare target");
             return;
         }
+        DatabaseType sourceDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getSourceDatabaseType());
+        DatabaseType targetDatabaseType = DatabaseTypeFactory.getInstance(jobConfig.getTargetDatabaseType());
+        if (!sourceDatabaseType.isSchemaAvailable() || !targetDatabaseType.isSchemaAvailable()) {
+            log.info("prepareTargetSchemas, one of source or target database type schema is not available, ignore");
+            return;
+        }

Review Comment:
   Looks it could not return, since it just could ignore creating schema but not creating table.
   
   Could we add an judgement method to check whether it need to create schema or not? And then use it to precheck for `prepareTargetSchemas`.



##########
shardingsphere-kernel/shardingsphere-data-pipeline/shardingsphere-data-pipeline-core/src/main/java/org/apache/shardingsphere/data/pipeline/core/prepare/datasource/PrepareTargetTablesParameter.java:
##########
@@ -31,28 +30,23 @@
 @Getter
 public final class PrepareTargetTablesParameter {
     
-    private final TaskConfiguration taskConfig;
+    private final String databaseName;
     
     private final JobDataNodeLine tablesFirstDataNodes;
     
+    private final PipelineDataSourceConfiguration pipelineDataSourceConfiguration;
+    

Review Comment:
   `pipelineDataSourceConfiguration` could be `dataSourceConfig`



-- 
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] sandynz commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter

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

   @huangdx0726 , thanks for your contribution.
   
   I'll create more Scaling issues recently, ones labeled as `good first issue` and `volunteer wanted` might be good for you.
   Welcome for more contributions.
   


-- 
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] huangdx0726 commented on pull request #19455: Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter

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

   my pleasure.&nbsp;Can be assigned directly to me.
   
   
   
   该邮件从移动设备发送
   
   
   &nbsp;
   
   
   
   
   ------------------&nbsp;原始邮件&nbsp;------------------
   发件人:                                                                                                                        "apache/shardingsphere"                                                                                    ***@***.***&gt;;
   发送时间:&nbsp;2022年7月27日(星期三) 下午5:16
   ***@***.***&gt;;
   ***@***.******@***.***&gt;;
   主题:&nbsp;Re: [apache/shardingsphere] Remove TaskConfiguration reference from PrepareTargetSchemasParameter and PrepareTargetTablesParameter (PR #19455)
   
   
   
   
   
    
   @huangdx0726 , thanks for your contribution.
    
   I'll create more Scaling issues recently, ones labeled as good first issue and volunteer wanted might be good for you.
    Welcome for more contributions.
    
   —
   Reply to this email directly, view it on GitHub, or unsubscribe.
   You are receiving this because you were mentioned.Message ID: ***@***.***&gt;


-- 
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