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 2021/09/14 08:58:02 UTC

[GitHub] [shardingsphere] menghaoranss opened a new pull request #12423: Use port instead of pid in ClusterInstance for proxy

menghaoranss opened a new pull request #12423:
URL: https://github.com/apache/shardingsphere/pull/12423


   Fixes #12396 .
   


-- 
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] taojintianxia commented on a change in pull request #12423: Use port instead of pid in ClusterInstance for proxy

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



##########
File path: shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/datasource/ShardingSphereDataSource.java
##########
@@ -74,7 +74,7 @@ private ContextManager createContextManager(final String schemaName, final ModeC
         Map<String, Collection<RuleConfiguration>> schemaRuleConfigs = Collections.singletonMap(
                 schemaName, ruleConfigs.stream().filter(each -> each instanceof SchemaRuleConfiguration).collect(Collectors.toList()));
         Collection<RuleConfiguration> globalRuleConfigs = ruleConfigs.stream().filter(each -> each instanceof GlobalRuleConfiguration).collect(Collectors.toList());
-        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite);
+        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite, null);

Review comment:
       there are 7 parameters, looks like not that graceful . Haoran, this method is build, why not refactor it as builder ? so you could build the parameter like ContextManagerBuilderFactory.newInstance(modeConfig).builder().modeConfig(modeConfig).overwrite(isOverwrite).xxx(xxx).build();




-- 
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] taojintianxia commented on a change in pull request #12423: Use port instead of pid in ClusterInstance for proxy

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



##########
File path: shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/datasource/ShardingSphereDataSource.java
##########
@@ -74,7 +74,7 @@ private ContextManager createContextManager(final String schemaName, final ModeC
         Map<String, Collection<RuleConfiguration>> schemaRuleConfigs = Collections.singletonMap(
                 schemaName, ruleConfigs.stream().filter(each -> each instanceof SchemaRuleConfiguration).collect(Collectors.toList()));
         Collection<RuleConfiguration> globalRuleConfigs = ruleConfigs.stream().filter(each -> each instanceof GlobalRuleConfiguration).collect(Collectors.toList());
-        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite);
+        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite, null);

Review comment:
       and, again, null is not a good choice.




-- 
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] terrymanu merged pull request #12423: Use port instead of pid in ClusterInstance for proxy

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


   


-- 
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] taojintianxia commented on a change in pull request #12423: Use port instead of pid in ClusterInstance for proxy

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



##########
File path: shardingsphere-jdbc/shardingsphere-jdbc-core/src/main/java/org/apache/shardingsphere/driver/jdbc/core/datasource/ShardingSphereDataSource.java
##########
@@ -74,7 +74,7 @@ private ContextManager createContextManager(final String schemaName, final ModeC
         Map<String, Collection<RuleConfiguration>> schemaRuleConfigs = Collections.singletonMap(
                 schemaName, ruleConfigs.stream().filter(each -> each instanceof SchemaRuleConfiguration).collect(Collectors.toList()));
         Collection<RuleConfiguration> globalRuleConfigs = ruleConfigs.stream().filter(each -> each instanceof GlobalRuleConfiguration).collect(Collectors.toList());
-        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite);
+        return ContextManagerBuilderFactory.newInstance(modeConfig).build(modeConfig, dataSourcesMap, schemaRuleConfigs, globalRuleConfigs, props, isOverwrite, null);

Review comment:
       BTW, there is an anotation may help you ,  `@Builder` in lombok




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