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/13 11:24:12 UTC

[GitHub] [shardingsphere] sandynz commented on a diff in pull request #19096: Improve stop scaling when running increment task.

sandynz commented on code in PR #19096:
URL: https://github.com/apache/shardingsphere/pull/19096#discussion_r919961872


##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/env/IntegrationTestEnvironment.java:
##########
@@ -175,4 +176,17 @@ public String getActualDataSourcePassword(final DatabaseType databaseType) {
     public static IntegrationTestEnvironment getInstance() {
         return INSTANCE;
     }
+    
+    /**
+     * Just get native database version. 
+     * @param databaseType database type.
+     * @return if matched return version, otherwise return empty.
+     */
+    public List<String> getNativeDatabaseVersions(final String databaseType) {
+        if (StringUtils.equalsIgnoreCase(databaseType, getNativeDatabaseType())) {
+            // Just to occupy the space, will not really use.
+            return Collections.singletonList("keep");
+        }
+        return Collections.emptyList();
+    }

Review Comment:
   For `getNativeDatabaseVersions`, it's better to rename it.
   
   And return `Collections.singletonList("keep")` is not readable enough.
   
   Is it possible to return `boolean`?
   



##########
shardingsphere-test/shardingsphere-integration-test/shardingsphere-integration-test-scaling/src/test/java/org/apache/shardingsphere/integration/data/pipeline/framework/container/database/OpenGaussContainer.java:
##########
@@ -54,6 +54,7 @@ protected void configure() {
         withExposedPorts(port);
         if (ScalingITEnvTypeEnum.NATIVE == IntegrationTestEnvironment.getInstance().getItEnvType()) {
             addFixedExposedPort(port, port);
+            addFixedExposedPort(port + 1, port + 1);
         }

Review Comment:
   If `addFixedExposedPort(port + 1, port + 1);` is just for openGauss 3.0, maybe it's not necessary enable it always.



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