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/01/13 11:14:44 UTC
[GitHub] [shardingsphere] lanchengx opened a new pull request #14747: Refactoring the Exportable interface
lanchengx opened a new pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747
Changes proposed in this pull request:
- Adjusted by exporting all the rules that implement this interface to specify the required key for export
--
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] RaigorJiang merged pull request #14747: Refactor the Exportable interface
Posted by GitBox <gi...@apache.org>.
RaigorJiang merged pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747
--
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] RaigorJiang commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r784746775
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/rule/identifier/type/ExportableRule.java
##########
@@ -19,17 +19,61 @@
import org.apache.shardingsphere.infra.rule.ShardingSphereRule;
+import java.util.Collection;
+import java.util.HashMap;
import java.util.Map;
+import java.util.function.Supplier;
/**
* ShardingSphere rule which supports exporting data.
*/
public interface ExportableRule extends ShardingSphereRule {
/**
- * Export data map.
+ * The methods that the rule can supply.
*
+ * @return export method supplier
+ */
+ Map<String, Supplier<Object>> getExportMethods();
Review comment:
getExportedMethods may better?
--
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 commented on pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
terrymanu commented on pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#issuecomment-1015464956
Please resolve conflicting files
--
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 commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
terrymanu commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r786403585
##########
File path: shardingsphere-features/shardingsphere-readwrite-splitting/shardingsphere-readwrite-splitting-core/src/main/java/org/apache/shardingsphere/readwritesplitting/rule/ReadwriteSplittingRule.java
##########
@@ -43,6 +43,8 @@
import java.util.Map;
import java.util.Optional;
import java.util.Map.Entry;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
Review comment:
Please remove unused import
--
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] TeslaCN commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r783887711
##########
File path: shardingsphere-features/shardingsphere-db-discovery/shardingsphere-db-discovery-distsql/shardingsphere-db-discovery-distsql-handler/src/main/java/org/apache/shardingsphere/dbdiscovery/distsql/handler/query/DatabaseDiscoveryRuleQueryResultSet.java
##########
@@ -70,8 +70,9 @@ public void init(final ShardingSphereMetaData metaData, final SQLStatement sqlSt
discoveryTypes = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryTypes).orElse(Collections.emptyMap());
discoveryHeartbeats = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryHeartbeats).orElse(Collections.emptyMap());
Optional<ExportableRule> exportableRule = metaData.getRuleMetaData().getRules()
- .stream().filter(each -> each instanceof ExportableRule).map(each -> (ExportableRule) each).findAny();
- primaryDataSources = (Map<String, String>) exportableRule.map(optional -> optional.export().get(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap());
+ .stream().filter(each -> each instanceof ExportableRule).filter(each -> ((ExportableRule) each).containExportableKey(Collections.singleton(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)))
+ .map(each -> (ExportableRule) each).findAny();
+ primaryDataSources = (Map<String, String>) (exportableRule.map(optional -> optional.export(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap()));
Review comment:
Consider using `orElseGet`.
##########
File path: shardingsphere-features/shardingsphere-readwrite-splitting/shardingsphere-readwrite-splitting-core/src/main/java/org/apache/shardingsphere/readwritesplitting/rule/ReadwriteSplittingRule.java
##########
@@ -118,11 +119,11 @@ public void updateStatus(final DataSourceStatusChangedEvent event) {
}
@Override
- public Map<String, Object> export() {
- Map<String, Object> result = new HashMap<>(1, 1);
- result.put(ExportableConstants.AUTO_AWARE_DATA_SOURCE_KEY, exportAutoAwareDataSourceMap());
- result.put(ExportableConstants.AUTO_AWARE_DATA_SOURCE_NAME, exportAutoAwareDataSourceNames());
- result.put(ExportableConstants.DATA_SOURCE_KEY, exportDataSourceNames());
+ public Map<String, Supplier<Object>> getExportMethods() {
+ Map<String, Supplier<Object>> result = new HashMap<>(2);
Review comment:
Maybe larger size?
--
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] TeslaCN commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r784521454
##########
File path: shardingsphere-features/shardingsphere-db-discovery/shardingsphere-db-discovery-distsql/shardingsphere-db-discovery-distsql-handler/src/main/java/org/apache/shardingsphere/dbdiscovery/distsql/handler/query/DatabaseDiscoveryRuleQueryResultSet.java
##########
@@ -70,8 +70,9 @@ public void init(final ShardingSphereMetaData metaData, final SQLStatement sqlSt
discoveryTypes = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryTypes).orElse(Collections.emptyMap());
discoveryHeartbeats = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryHeartbeats).orElse(Collections.emptyMap());
Optional<ExportableRule> exportableRule = metaData.getRuleMetaData().getRules()
- .stream().filter(each -> each instanceof ExportableRule).map(each -> (ExportableRule) each).findAny();
- primaryDataSources = (Map<String, String>) exportableRule.map(optional -> optional.export().get(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap());
+ .stream().filter(each -> each instanceof ExportableRule).filter(each -> ((ExportableRule) each).containExportableKey(Collections.singleton(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)))
+ .map(each -> (ExportableRule) each).findAny();
+ primaryDataSources = (Map<String, String>) (exportableRule.map(optional -> optional.export(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap()));
Review comment:
Yes, or `orElseGet(Collections::emptyMap)`.
--
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] TeslaCN commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r783887416
##########
File path: shardingsphere-features/shardingsphere-db-discovery/shardingsphere-db-discovery-core/src/main/java/org/apache/shardingsphere/dbdiscovery/rule/DatabaseDiscoveryRule.java
##########
@@ -176,9 +177,9 @@ public void updateStatus(final DataSourceStatusChangedEvent event) {
}
@Override
- public Map<String, Object> export() {
- Map<String, Object> result = new HashMap<>(1, 1);
- result.put(ExportableConstants.PRIMARY_DATA_SOURCE_KEY, exportPrimaryDataSourceMap());
+ public Map<String, Supplier<Object>> getExportMethods() {
+ Map<String, Supplier<Object>> result = new HashMap<>(1);
Review comment:
Missing `loadFactor`?
--
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] lanchengx commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
lanchengx commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r784482644
##########
File path: shardingsphere-features/shardingsphere-db-discovery/shardingsphere-db-discovery-distsql/shardingsphere-db-discovery-distsql-handler/src/main/java/org/apache/shardingsphere/dbdiscovery/distsql/handler/query/DatabaseDiscoveryRuleQueryResultSet.java
##########
@@ -70,8 +70,9 @@ public void init(final ShardingSphereMetaData metaData, final SQLStatement sqlSt
discoveryTypes = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryTypes).orElse(Collections.emptyMap());
discoveryHeartbeats = ruleConfig.map(DatabaseDiscoveryRuleConfiguration::getDiscoveryHeartbeats).orElse(Collections.emptyMap());
Optional<ExportableRule> exportableRule = metaData.getRuleMetaData().getRules()
- .stream().filter(each -> each instanceof ExportableRule).map(each -> (ExportableRule) each).findAny();
- primaryDataSources = (Map<String, String>) exportableRule.map(optional -> optional.export().get(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap());
+ .stream().filter(each -> each instanceof ExportableRule).filter(each -> ((ExportableRule) each).containExportableKey(Collections.singleton(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)))
+ .map(each -> (ExportableRule) each).findAny();
+ primaryDataSources = (Map<String, String>) (exportableRule.map(optional -> optional.export(ExportableConstants.PRIMARY_DATA_SOURCE_KEY)).orElse(Collections.emptyMap()));
Review comment:
Is it .orElseGet(() -> Collections.emptyMap())) ?
--
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] RaigorJiang commented on a change in pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
RaigorJiang commented on a change in pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#discussion_r784743407
##########
File path: shardingsphere-infra/shardingsphere-infra-common/src/main/java/org/apache/shardingsphere/infra/distsql/constant/ExportableConstants.java
##########
@@ -37,4 +37,6 @@
public static final String REPLICA_DATA_SOURCE_NAMES = "replica_data_source_names";
public static final String DATA_SOURCE_KEY = "data_source_key";
+
+ public static final String SINGLE_TABLE_TABLES = "single_table_tables";
Review comment:
single_tables
--
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] lanchengx commented on pull request #14747: Refactoring the Exportable interface
Posted by GitBox <gi...@apache.org>.
lanchengx commented on pull request #14747:
URL: https://github.com/apache/shardingsphere/pull/14747#issuecomment-1015490229
> Please resolve conflicting files
ok, it might have some other modifications
--
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