Add Open Index API to the high level REST client #27574

olcbean · 2017-11-28T20:28:38Z

Add _open to the high level REST client

Relates to #27205

elasticmachine · 2017-11-28T20:28:39Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

DaveCTurner · 2017-11-28T21:14:28Z

@elasticmachine please test this

javanna

thanks a lot @olcbean I left a few comments, looks good though. I will merge this once you addressed those.

javanna · 2017-12-01T19:56:30Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

+     */
+    public final void openIndexAsync(OpenIndexRequest openIndexRequest, ActionListener<OpenIndexResponse> listener, Header... headers) {
+        performRequestAsyncAndParseEntity(openIndexRequest, Request::openIndex, OpenIndexResponse::fromXContent, listener,
+                Collections.singleton(404), headers);


we need these only in IndicesClient, so that users can do client.indices().openIndex() . No need to add them here too.

javanna · 2017-12-01T19:57:47Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        OpenIndexRequest openIndexRequest = new OpenIndexRequest(indexName);
+        OpenIndexResponse openIndexResponse = execute(openIndexRequest, highLevelClient().indices()::openIndex,
+                highLevelClient().indices()::openIndexAsync);
+        assertTrue(openIndexResponse.isAcknowledged());


maybe it would be good to close the index first using the low-level REST client, then verify that opening it actually does something?

javanna · 2017-12-01T19:59:06Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+
+        ElasticsearchException exception = expectThrows(ElasticsearchException.class,
+                () -> execute(openIndexRequest, highLevelClient().indices()::openIndex, highLevelClient().indices()::openIndexAsync));
+        assertEquals(RestStatus.NOT_FOUND, exception.status());


maybe also test the API against multiple indices?

javanna · 2017-12-01T19:59:48Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

+        Map<String, String> expectedParams = new HashMap<>();
+        setRandomTimeout(openIndexRequest::timeout, AcknowledgedRequest.DEFAULT_ACK_TIMEOUT, expectedParams);
+        setRandomMasterTimeout(openIndexRequest, expectedParams);
+        setRandomIndicesOptions(openIndexRequest::indicesOptions, openIndexRequest::indicesOptions, expectedParams);


can we also add the set wait for active shards ?

javanna · 2017-12-01T20:02:08Z

core/src/test/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponseTests.java

+        boolean humanReadable = randomBoolean();
+        final XContentType xContentType = randomFrom(XContentType.values());
+        BytesReference originalBytes = toShuffledXContent(openIndexResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
+


can you also add here an insertRandomFields step to verify forward compatibility (as we ignore any unknown fields rather than throwing errors)

javanna · 2017-12-01T20:02:45Z

core/src/test/java/org/elasticsearch/action/admin/indices/open/OpenIndexResponseTests.java

+        final XContentType xContentType = randomFrom(XContentType.values());
+        BytesReference originalBytes = toShuffledXContent(openIndexResponse, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
+
+        OpenIndexResponse parsedCreateIndexResponse;


rename this variable?

olcbean · 2017-12-03T21:45:49Z

@javanna thanks for the review. Ready for a second round ;)

javanna · 2017-12-04T11:24:25Z

test this please

javanna

thanks @olcbean I left a few more comments, nothing major. The important bit is removing the new methods from RestHighLevelClient.

javanna · 2017-12-04T11:17:43Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -281,7 +283,7 @@ public final GetResponse get(GetRequest getRequest, Header... headers) throws IO
     *
     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-get.html">Get API on elastic.co</a>
     */
-    public void getAsync(GetRequest getRequest, ActionListener<GetResponse> listener, Header... headers) {
+    public final void getAsync(GetRequest getRequest, ActionListener<GetResponse> listener, Header... headers) {


thank you, seems like I left this out :)

javanna · 2017-12-04T11:18:04Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

+    public final void openIndexAsync(OpenIndexRequest openIndexRequest, ActionListener<OpenIndexResponse> listener, Header... headers) {
+        performRequestAsyncAndParseEntity(openIndexRequest, Request::openIndex, OpenIndexResponse::fromXContent, listener, emptySet(),
+                headers);
+    }


we don't need these two methods, they are already in IndicesClient. Can you remove them?

javanna · 2017-12-04T11:19:50Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        return randomIndices(0, 5);
+    }
+
+    private String[] randomIndices(int minIndicesNum, int maxIndicesNum) {


make this static?

javanna · 2017-12-04T11:20:31Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        assertEquals(RestStatus.NOT_FOUND, exception.status());
+    }
+
+    private String[] randomIndices() {


maybe we can remove this method and always call the other one?

javanna · 2017-12-04T11:20:53Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        assertTrue(openIndexResponse.isAcknowledged());
+
+        for (String index : indices) {
+            client().performRequest("GET", index + "/_search");


check that 200 was returned?

javanna · 2017-12-04T11:22:20Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        OpenIndexRequest openIndexRequest = new OpenIndexRequest(nonExistentIndices);
+        ElasticsearchException exception = expectThrows(ElasticsearchException.class,
+                () -> execute(openIndexRequest, highLevelClient().indices()::openIndex, highLevelClient().indices()::openIndexAsync));
+        assertEquals(RestStatus.NOT_FOUND, exception.status());


would you mind checking also that with IndicesOptions.lenientExpandOpen we don't get back an error?

Clean up

olcbean · 2017-12-04T12:52:30Z

@javanna my bad! Ready for another round ;)

javanna

LGTM thanks @olcbean

javanna · 2017-12-06T11:55:34Z

test this please

javanna · 2017-12-06T13:29:08Z

retest this please

javanna · 2017-12-07T10:57:14Z

heya @olcbean I took the liberty to merge master in as there were conflicts and push to your remote. Running tests one last time and then merging. Thanks a lot!

javanna · 2017-12-07T10:57:29Z

retest this please

olcbean · 2017-12-07T13:51:16Z

Thanks @javanna!
I will continue with _close ( if nobody is working on it )

javanna · 2017-12-07T17:15:48Z

sounds great @olcbean thank you!

* master: (414 commits) Set ACK timeout on indices service test Implement byte array reusage in `NioTransport` (elastic#27696) [TEST] Remove leftover ES temp directories before Vagrant tests (elastic#27722) Cleanup split strings by comma method Remove unused import from AliasResolveRoutingIT Add read timeouts to http module (elastic#27713) Fix routing with leading or trailing whitespace remove await fix from FullClusterRestartIT.testRecovery Add missing 's' to tmpdir name (elastic#27721) [Issue-27716]: CONTRIBUTING.md IntelliJ configurations settings are confusing. (elastic#27717) [TEST] Now actually wait for merges Test out of order delivery of append only index and retry with an intermediate delete [TEST] remove code duplications in RequestTests [Tests] Add test for GeoShapeFieldType#setStrategyName (elastic#27703) Remove unused *Commit* classes (elastic#27714) Add test for writer operation buffer accounting (elastic#27707) [TEST] Wait for merging to complete before testing breaker Add Open Index API to the high level REST client (elastic#27574) Correcting some minor typos in comments Add unreleased v5.6.6 version ...

Add _open to the high level REST client Relates to #27205

Add open index api to the high level rest client

9e25962

DaveCTurner requested a review from javanna November 28, 2017 21:13

DaveCTurner added v6.2.0 v7.0.0 :Java High Level REST Client labels Nov 28, 2017

javanna self-assigned this Dec 1, 2017

javanna added the >enhancement label Dec 1, 2017

javanna requested changes Dec 1, 2017

View reviewed changes

olcbean added 2 commits December 3, 2017 20:44

Addressing reviewers remarks

cbfaf34

Setting getAsync to final

ae7ede9

javanna requested changes Dec 4, 2017

View reviewed changes

Added tests for strictExpandOpen and lenientExpandOpen

c3f85ee

Clean up

javanna approved these changes Dec 6, 2017

View reviewed changes

Merge branch 'master' into hlrc_open_index

f63d4a3

javanna merged commit bcc33f3 into elastic:master Dec 7, 2017

javanna added the backport pending label Dec 7, 2017

javanna pushed a commit that referenced this pull request Dec 10, 2017

Add Open Index API to the high level REST client (#27574)

80be19a

Add _open to the high level REST client Relates to #27205

javanna removed the backport pending label Dec 10, 2017

jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Open Index API to the high level REST client #27574

Add Open Index API to the high level REST client #27574

olcbean commented Nov 28, 2017

elasticmachine commented Nov 28, 2017

DaveCTurner commented Nov 28, 2017

javanna left a comment

javanna Dec 1, 2017

javanna Dec 1, 2017

javanna Dec 1, 2017

javanna Dec 1, 2017

javanna Dec 1, 2017

javanna Dec 1, 2017

olcbean commented Dec 3, 2017

javanna commented Dec 4, 2017

javanna left a comment

javanna Dec 4, 2017

javanna Dec 4, 2017

olcbean Dec 4, 2017

javanna Dec 4, 2017

javanna Dec 4, 2017

javanna Dec 4, 2017

javanna Dec 4, 2017

olcbean commented Dec 4, 2017

javanna left a comment

javanna commented Dec 6, 2017

javanna commented Dec 6, 2017

javanna commented Dec 7, 2017

javanna commented Dec 7, 2017

olcbean commented Dec 7, 2017

javanna commented Dec 7, 2017

Add Open Index API to the high level REST client #27574

Add Open Index API to the high level REST client #27574

Conversation

olcbean commented Nov 28, 2017

elasticmachine commented Nov 28, 2017

DaveCTurner commented Nov 28, 2017

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olcbean commented Dec 3, 2017

javanna commented Dec 4, 2017

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olcbean commented Dec 4, 2017

javanna left a comment

Choose a reason for hiding this comment

javanna commented Dec 6, 2017

javanna commented Dec 6, 2017

javanna commented Dec 7, 2017

javanna commented Dec 7, 2017

olcbean commented Dec 7, 2017

javanna commented Dec 7, 2017