ILuffZhe commented on a change in pull request #2698:
URL: https://github.com/apache/calcite/pull/2698#discussion_r795039946
##########
File path:
elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/ElasticSearchAdapterTest.java
##########
@@ -105,7 +105,7 @@ private static Connection createConnection() throws
SQLException {
connection.unwrap(CalciteConnection.class).getRootSchema();
root.add("elastic",
- new ElasticsearchSchema(NODE.restClient(), NODE.mapper(), ZIPS));
+ new ElasticsearchSchema(NODE.clientBuilder(), NODE.mapper(),
ZIPS));
Review comment:
Please check the indent here, so does somewhere else.
##########
File path:
elasticsearch/src/test/java/org/apache/calcite/adapter/elasticsearch/EmbeddedElasticsearchPolicy.java
##########
@@ -192,19 +201,18 @@ ObjectMapper mapper() {
* Low-level http rest client connected to current embedded elastic search
instance.
* @return http client connected to ES cluster
*/
- RestClient restClient() {
- if (client != null) {
- return client;
+ synchronized RestClientBuilder clientBuilder() {
Review comment:
Do we need a synchronized here?
##########
File path:
elasticsearch/src/main/java/org/apache/calcite/adapter/elasticsearch/ElasticsearchTransport.java
##########
@@ -279,22 +276,25 @@ private Response applyInternal(final HttpRequest request)
request.getRequestLine().getMethod(),
request.getRequestLine().getUri());
r.setEntity(entity);
- final Response response = restClient.performRequest(r);
- final String payload = entity != null && entity.isRepeatable()
- ? EntityUtils.toString(entity) : "<empty>";
+ try (RestClient restClient = clientBuilder.build()) {
Review comment:
If I understand this correctly, for each unit test, we will create a new
RestClient to request, which is what you mentioned.
- In my team, we manage the RestClients by Cache(for lifecycle and something
else). Since it is actual business related, we might don't make it here
- For unit tests, I'm wandering if there is a suitable way to manage the
RestClient in BeforeClass and AfterClass. Just my personal opinion.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
