You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/07/04 10:07:18 UTC

[GitHub] [iotdb] liujiajun opened a new pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

liujiajun opened a new pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502


   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663738886



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Correct me if I am wrong: `host` and `rpcPort` are compulsory fields for session and has no default values, so they are placed in the constructor. Same goes for sessionpool. If they are not put in constructor, users might forget to set values for them.

##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Correct me if I am wrong: `host` and `rpcPort` are compulsory fields for session and has no default values, so they are placed in the constructor. Same goes for sessionpool. If they are not put in constructor, users might forget to set values for them. Does this make sense 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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663942520



##########
File path: session/src/main/java/org/apache/iotdb/session/pool/SessionPool.java
##########
@@ -1161,4 +1161,121 @@ public SessionDataSetWrapper executeRawDataQuery(List<String> paths, long startT
     // never go here
     return null;
   }
+
+  public int getMaxSize() {
+    return maxSize;
+  }
+
+  public String getHost() {
+    return host;
+  }
+
+  public int getPort() {
+    return port;
+  }
+
+  public String getUser() {
+    return user;
+  }
+
+  public String getPassword() {
+    return password;
+  }
+
+  public int getFetchSize() {
+    return fetchSize;
+  }
+
+  public long getTimeout() {
+    return timeout;
+  }
+
+  public boolean isEnableCompression() {
+    return enableCompression;
+  }
+
+  public boolean isEnableCacheLeader() {
+    return enableCacheLeader;
+  }
+
+  public ZoneId getZoneId() {

Review comment:
       Currently, the setup of zondId needs a rpc `setTimeZone` which you can refer to [PR3457](https://github.com/apache/iotdb/pull/3457/files), so maybe we can not initialized zoneId here, what do you think?

##########
File path: session/src/main/java/org/apache/iotdb/session/pool/SessionPool.java
##########
@@ -1161,4 +1161,121 @@ public SessionDataSetWrapper executeRawDataQuery(List<String> paths, long startT
     // never go here
     return null;
   }
+
+  public int getMaxSize() {
+    return maxSize;
+  }
+
+  public String getHost() {
+    return host;
+  }
+
+  public int getPort() {
+    return port;
+  }
+
+  public String getUser() {
+    return user;
+  }
+
+  public String getPassword() {
+    return password;
+  }
+
+  public int getFetchSize() {
+    return fetchSize;
+  }
+
+  public long getTimeout() {
+    return timeout;
+  }
+
+  public boolean isEnableCompression() {
+    return enableCompression;
+  }
+
+  public boolean isEnableCacheLeader() {
+    return enableCacheLeader;
+  }
+
+  public ZoneId getZoneId() {

Review comment:
       Currently, the setup of zondId needs a rpc `setTimeZone` which you can refer to [PR3457](https://github.com/apache/iotdb/pull/3457/files), so maybe we can not initialize zoneId here, what do you think?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663686225



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       default of constructors is no parameter.
   // default
   Session session = new SessionBuilder().build()
   SessionPool sessionpool = new SessionPoolBuilder().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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41142346/badge)](https://coveralls.io/builds/41142346)
   
   Coverage increased (+0.05%) to 67.911% when pulling **d119dfff6e7bd5158810aa45d9ed7f89e5070231 on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663586657



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       hi
   I found this implementation to be inconsistent with the use cases on jira.
   We need to return a session through sessionbuild.
   Session session = new SessionBuilder().build()

##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -43,6 +43,7 @@
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 public class SessionUT {

Review comment:
       generate, Generally, UT ends with Test,
   IT ends with 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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663846700



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       ok




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663593585



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Hi, thanks for the review! I think the implementation is consistent (see https://dzone.com/articles/design-patterns-the-builder-pattern). When using the pattern, however, we usually do not keep the interim builder object. I have changed the test case to make this more explicit. 




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#issuecomment-873990757


   Thanks for the comments! Have changed accordingly. 
   
   I removed the old construction method in the docs and also changed examples, so that new users can use builder as a preferred way.
   
   Will blast out an email after this is merged~


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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






-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#issuecomment-873990757


   Thanks for the comments! Have changed accordingly. 
   
   I removed the old construction method in the docs and also changed examples, so that new users can use builder as a preferred way.
   
   Will blast out an email after this is merged~


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41106341/badge)](https://coveralls.io/builds/41106341)
   
   Coverage increased (+0.06%) to 67.918% when pulling **7b0ea65ec180e05cf23a9bdb29955e9bd003e04b on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663738886



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Correct me if I am wrong: `host` and `rpcPort` are compulsory fields for session and has no default values, so they are placed in the constructor. Same goes for sessionpool. If they are not put in constructor, users might forget to set values for them. Does this make sense 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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41116685/badge)](https://coveralls.io/builds/41116685)
   
   Coverage increased (+0.04%) to 67.899% when pulling **c9b466540482ac9c50647d6b5b7d5bdb06af13ee on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41116850/badge)](https://coveralls.io/builds/41116850)
   
   Coverage increased (+0.05%) to 67.909% when pulling **c9b466540482ac9c50647d6b5b7d5bdb06af13ee on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] wangchao316 merged pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
wangchao316 merged pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502


   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663942520



##########
File path: session/src/main/java/org/apache/iotdb/session/pool/SessionPool.java
##########
@@ -1161,4 +1161,121 @@ public SessionDataSetWrapper executeRawDataQuery(List<String> paths, long startT
     // never go here
     return null;
   }
+
+  public int getMaxSize() {
+    return maxSize;
+  }
+
+  public String getHost() {
+    return host;
+  }
+
+  public int getPort() {
+    return port;
+  }
+
+  public String getUser() {
+    return user;
+  }
+
+  public String getPassword() {
+    return password;
+  }
+
+  public int getFetchSize() {
+    return fetchSize;
+  }
+
+  public long getTimeout() {
+    return timeout;
+  }
+
+  public boolean isEnableCompression() {
+    return enableCompression;
+  }
+
+  public boolean isEnableCacheLeader() {
+    return enableCacheLeader;
+  }
+
+  public ZoneId getZoneId() {

Review comment:
       Currently, the setup of zondId needs a rpc `setTimeZone` which you can refer to [PR3457](https://github.com/apache/iotdb/pull/3457/files), so maybe we can not initialized zoneId here, what do you think?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663738886



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Correct me if I am wrong: `host` and `rpcPort` are compulsory fields for session and has no default values, so they are placed in the constructor. Same goes for sessionpool. If they are not put in constructor, users might forget to set values for them.




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41100791/badge)](https://coveralls.io/builds/41100791)
   
   Coverage increased (+0.01%) to 67.873% when pulling **2e0e2f68092833b675edb3b849816eb3a2cd4ea4 on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] wangchao316 commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
wangchao316 commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663846700



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       ok




-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] liujiajun commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
liujiajun commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663593585



##########
File path: session/src/test/java/org/apache/iotdb/session/SessionUT.java
##########
@@ -244,4 +245,25 @@ public void createDeviceTemplate() throws IoTDBConnectionException, StatementExe
         "template1", schemaNames, measurementList, dataTypeList, encodingList, compressionTypes);
     session.setSchemaTemplate("template1", "root.sg.1");
   }
+
+  @Test
+  public void testBuilder() {
+    Session.Builder builder = new Session.Builder("localhost", 1234);

Review comment:
       Hi, thanks for the review! 
   
   I think the implementation is consistent (see https://dzone.com/articles/design-patterns-the-builder-pattern). When using the pattern, however, we usually do not keep the interim builder object. I have changed the test case to make this more explicit. 
   
   Or are you referring to that we prefer move builder class out of session class, so `SessionBuilder.build()` instead of `Session.Builder.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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls commented on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41100310/badge)](https://coveralls.io/builds/41100310)
   
   Coverage decreased (-0.02%) to 67.838% when pulling **203615f4ae10c3fc42efc185627e1a2f01e6372a on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41134172/badge)](https://coveralls.io/builds/41134172)
   
   Coverage increased (+0.02%) to 67.879% when pulling **1afba7d3ad3ffc7f7ac43b2010bf227b3c25bf40 on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] coveralls edited a comment on pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

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


   
   [![Coverage Status](https://coveralls.io/builds/41114131/badge)](https://coveralls.io/builds/41114131)
   
   Coverage increased (+0.06%) to 67.918% when pulling **96ee9fca977ca572d895e36a1a3e547c9eebd1eb on liujiajun:builder** into **dbfb5648ddf4585c6009038e6080493353e7feb4 on apache:master**.
   


-- 
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: reviews-unsubscribe@iotdb.apache.org

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



[GitHub] [iotdb] LebronAl commented on a change in pull request #3502: [IOTDB-1463] Implement builder pattern for Session and SessionPool

Posted by GitBox <gi...@apache.org>.
LebronAl commented on a change in pull request #3502:
URL: https://github.com/apache/iotdb/pull/3502#discussion_r663942520



##########
File path: session/src/main/java/org/apache/iotdb/session/pool/SessionPool.java
##########
@@ -1161,4 +1161,121 @@ public SessionDataSetWrapper executeRawDataQuery(List<String> paths, long startT
     // never go here
     return null;
   }
+
+  public int getMaxSize() {
+    return maxSize;
+  }
+
+  public String getHost() {
+    return host;
+  }
+
+  public int getPort() {
+    return port;
+  }
+
+  public String getUser() {
+    return user;
+  }
+
+  public String getPassword() {
+    return password;
+  }
+
+  public int getFetchSize() {
+    return fetchSize;
+  }
+
+  public long getTimeout() {
+    return timeout;
+  }
+
+  public boolean isEnableCompression() {
+    return enableCompression;
+  }
+
+  public boolean isEnableCacheLeader() {
+    return enableCacheLeader;
+  }
+
+  public ZoneId getZoneId() {

Review comment:
       Currently, the setup of zondId needs a rpc `setTimeZone` which you can refer to [PR3457](https://github.com/apache/iotdb/pull/3457/files), so maybe we can not initialize zoneId here, what do you think?




-- 
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: reviews-unsubscribe@iotdb.apache.org

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