imbajin commented on code in PR #140:
URL:
https://github.com/apache/incubator-hugegraph-commons/pull/140#discussion_r1522432813
##########
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java:
##########
@@ -30,13 +33,21 @@ public class RestClientConfig {
private String user;
private String password;
private String token;
- // unit in milliseconds
+ /**
+ * @deprecated use connectTimeout and readTimeout instead
+ */
+ @Deprecated
private Integer timeout;
+ // unit in milliseconds
Review Comment:
```suggestion
/** unit in milliseconds */
```
##########
hugegraph-common/src/main/java/org/apache/hugegraph/rest/RestClientConfig.java:
##########
@@ -30,13 +33,21 @@ public class RestClientConfig {
private String user;
private String password;
private String token;
- // unit in milliseconds
+ /**
+ * @deprecated use connectTimeout and readTimeout instead
+ */
+ @Deprecated
private Integer timeout;
+ // unit in milliseconds
+ private Integer connectTimeout;
+ // unit in milliseconds
Review Comment:
```suggestion
/** unit in milliseconds */
```
##########
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java:
##########
@@ -320,6 +321,28 @@ public void testAuthContext() {
Assert.assertNull(client.getAuthContext());
}
+ @SneakyThrows
+ @Test
+ public void testBuilderCallback() {
+ //default config
+ MockRestClientImpl restClient = new MockRestClientImpl(TEST_URL,
+
RestClientConfig.builder().build());
+ OkHttpClient okHttpClient = Whitebox.getInternalState(restClient,
"client");
+ Assert.assertEquals(okHttpClient.connectTimeoutMillis(), 10000);
+ Assert.assertEquals(okHttpClient.readTimeoutMillis(), 10000);
+
+ //set config by builderCallback
Review Comment:
```suggestion
// set config by (user)builderCallback
```
##########
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java:
##########
@@ -320,6 +321,28 @@ public void testAuthContext() {
Assert.assertNull(client.getAuthContext());
}
+ @SneakyThrows
+ @Test
+ public void testBuilderCallback() {
+ //default config
Review Comment:
```suggestion
// default configs
```
##########
hugegraph-common/src/main/java/org/apache/hugegraph/rest/AbstractRestClient.java:
##########
@@ -205,6 +211,11 @@ private OkHttpClient buildOkHttpClient(RestClientConfig
config) {
configSsl(builder, this.baseUrl, config.getTrustStoreFile(),
config.getTrustStorePassword());
+ //execute builder callback before builder.build()
Review Comment:
```suggestion
// Execute builder callback before builder.build() for user configs
```
##########
hugegraph-common/src/test/java/org/apache/hugegraph/unit/rest/RestClientTest.java:
##########
@@ -320,6 +321,28 @@ public void testAuthContext() {
Assert.assertNull(client.getAuthContext());
}
+ @SneakyThrows
+ @Test
+ public void testBuilderCallback() {
+ //default config
+ MockRestClientImpl restClient = new MockRestClientImpl(TEST_URL,
+
RestClientConfig.builder().build());
+ OkHttpClient okHttpClient = Whitebox.getInternalState(restClient,
"client");
+ Assert.assertEquals(okHttpClient.connectTimeoutMillis(), 10000);
+ Assert.assertEquals(okHttpClient.readTimeoutMillis(), 10000);
+
+ //set config by builderCallback
+ RestClientConfig config = RestClientConfig.builder().builderCallback(
+ builder -> builder.connectTimeout(5, TimeUnit.SECONDS)
+ .readTimeout(30, TimeUnit.SECONDS)).build();
Review Comment:
```suggestion
.readTimeout(30, TimeUnit.SECONDS))
.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: dev-unsubscr...@hugegraph.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
