HLRest: Add get index templates API #31161

dnhatn · 2018-06-06T22:37:53Z

elasticmachine · 2018-06-06T22:37:55Z

Pinging @elastic/es-core-infra

cbuescher

Hi @dnhatn, thanks for the PR. I took a quick look and didn't find anything major to change, only left a couple of minor comments which I'm sure are sorted out quickly.

cbuescher · 2018-06-07T15:41:38Z

docs/java-rest/high-level/indices/get_templates.asciidoc

+[[java-rest-high-get-templates-request]]
+==== Get Index Templates Request
+
+A `GetIndexTemplatesRequest` specifies one or several names of the getting index templates.


maybe better "of the index templates to get"

cbuescher · 2018-06-07T15:42:11Z

docs/java-rest/high-level/indices/get_templates.asciidoc

+include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[get-templates-request]
+--------------------------------------------------
+<1> A single index template name
+<2> Multiple index templates' names


I would also just use singular "template" here

cbuescher · 2018-06-07T15:42:25Z

docs/java-rest/high-level/indices/get_templates.asciidoc

+--------------------------------------------------
+<1> A single index template name
+<2> Multiple index templates' names
+<3> An index template's name using wildcard


cbuescher · 2018-06-07T15:44:51Z

...main/java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesResponse.java

@@ -37,6 +39,7 @@
    private List<IndexTemplateMetaData> indexTemplates;

    GetIndexTemplatesResponse() {
+        indexTemplates = new ArrayList<>();


Does this enable us to make the indexTemplates list final?

cbuescher · 2018-06-07T15:54:48Z

...main/java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesResponse.java

+                templates.add(templateMetaData);
+            }
+        }
+        templates.sort(Comparator.comparing(IndexTemplateMetaData::name));


Why is sorting necessary here? From the way the templates are rendered in toXContent it looks to me we could just read them back the same way?

cbuescher · 2018-06-07T15:57:30Z

...java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesResponseTests.java

+    @Override
+    protected GetIndexTemplatesResponse createTestInstance() {
+        List<IndexTemplateMetaData> templates = new ArrayList<>();
+        int numTemplates = between(1, 10);


Does it make sense to also test with 0 templates? Not sure if this throws an error somewhere.

cbuescher · 2018-06-07T16:01:15Z

...java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesResponseTests.java

+            templateBuilder.patterns(IntStream.range(0, between(1, 5)).mapToObj(i -> "pattern-" + i).collect(Collectors.toList()));
+            int numAlias = between(0, 5);
+            for (int i = 0; i < numAlias; i++) {
+                AliasMetaData.Builder alias = AliasMetaData.builder(randomAlphaOfLengthBetween(1, 10));


Maybe its enough to randomize the name here, the full alias parsing is tested more thouroughly somewhere else I think.

dnhatn · 2018-06-07T19:20:02Z

@cbuescher Thanks for taking a look. I've addressed all your comments.

cbuescher

Thanks @dnhatn, LGTM.

javanna

left a couple of comments, LGTM otherwise

javanna · 2018-06-08T09:21:54Z

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

+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-templates.html"> Index Templates API
+     * on elastic.co</a>
+     */
+    public GetIndexTemplatesResponse getTemplates(GetIndexTemplatesRequest getIndexTemplatesRequest,


nit: can we call it getTemplate? That's what our SPEC have, without the final 's'

I pushed 7117a3d

javanna · 2018-06-08T09:28:30Z

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

+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-templates.html"> Index Templates API
+     * on elastic.co</a>
+     */
+    public void getTemplatesAsync(GetIndexTemplatesRequest getIndexTemplatesRequest, RequestOptions options,


can you improve the javadocs like we have it in the other methods?

I pushed 7117a3d

javanna · 2018-06-08T09:29:54Z

...java/org/elasticsearch/action/admin/indices/template/get/GetIndexTemplatesResponseTests.java

+
+    @Override
+    protected boolean supportsUnknownFields() {
+        return false;


I wonder if it's worth to return true here and exclude some specific paths. for instance at the top-level we can't inject anything arbitrary, within a template it should be possible, within mappings it is not... etc. this is important to verify that we our parsing is forward compatible.

For what its worth, should those kind of test be part of the IndexTemplateMetaDataTests instead? The response here just renders a list of IndexTemplateMetaData instances with arbitrary names, so in this level are trying to parse every field name as a IndexTemplateMetaData. The parser in this class will indead fail on anything that it encounters thats not parseable as IndexTemplateMetaData, and at the moment it should I think.

IndexTemplateMetaDataTests already covers these situations.

ok can you add a comment here then?

Yes, I added a comment for this.

dnhatn · 2018-06-11T15:06:12Z

Thanks @cbuescher and @javanna for reviewing!

Relates #27205

* master: Remove RestGetAllAliasesAction (#31308) Temporary fix for broken build Reenable Checkstyle's unused import rule (#31270) Remove remaining unused imports before merging #31270 Fix non-REST doc snippet [DOC] Extend SQL docs Immediately flush channel after writing to buffer (#31301) [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) Add 5.6.11 version constant. Fix version detection. SQL: Whitelist SQL utility class for better scripting (#30681) [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) CCS: don't proxy requests for already connected node (#31273) Mute ScriptedMetricAggregatorTests testSelfReferencingAggStateAfterMap [test] opensuse packaging turn up debug logging Add unreleased version 6.3.1 Removes experimental tag from scripted_metric aggregation (#31298) [Rollup] Metric config parser must use builder so validation runs (#31159) [ML] Check licence when datafeeds use cross cluster search (#31247) Add notion of internal index settings (#31286) Test: Remove broken yml test feature (#31255) REST hl client: cluster health to default to cluster level (#31268) [ML] Update test thresholds to account for changes to memory control (#31289) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) Ignore numeric shard count if waiting for ALL (#31265) [ML] Implement new rules design (#31110) index_prefixes back-compat should test 6.3 (#30951) Core: Remove plain execute method on TransportAction (#30998) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Modify pipelining handlers to require full requests (#31280) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) Fix Netty 4 Server Transport tests. Again. REST hl client: adjust wait_for_active_shards param in cluster health (#31266) REST high-level Client: remove deprecated API methods (#31200) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) Delete typos in SAML docs (#31199) REST high-level client: add Cluster Health API (#29331) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) Remove some line length supressions (#31209) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) LLClient: Support host selection (#30523) Upgrade to Netty 4.1.25.Final (#31232) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) HLRest: Add get index templates API (#31161) Remove all unused imports and fix CRLF (#31207) [Tests] Fix self-referencing tests [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Limit the number of concurrent requests per node (#31206) Call ensureNoSelfReferences() on _agg state variable after scripted metric agg script executions (#31044) Move java version checker back to its own jar (#30708) [test] add fix for rare virtualbox error (#31212)

* 6.x: SQL: Fix build on Java 10 [Tests] Mutualize fixtures code in BaseHttpFixture (#31210) [TEST] Fix RemoteClusterClientTests#testEnsureWeReconnect [ML] Update test thresholds to account for changes to memory control (#31289) Reenable Checkstyle's unused import rule (#31270) [ML] Check licence when datafeeds use cross cluster search (#31247) Fix non-REST doc snippet [DOC] Extend SQL docs [DOCS] Shortens ML API intros Use quotes in the call invocation (#31249) move security ingest processors to a sub ingest directory (#31306) SQL: Whitelist SQL utility class for better scripting (#30681) Add 5.6.11 version constant. Fix version detection. [Docs] All Rollup docs experimental, agg limitations, clarify DeleteJob (#31299) Add missing release notes. Security: fix token bwc with pre 6.0.0-beta2 (#31254) Fix compilation error in UpdateSettingsIT (#31304) Test: Remove broken yml test feature (#31255) Add unreleased version 6.3.1 [Rollup] Metric config parser must use builder so validation runs (#31159) Removes experimental tag from scripted_metric aggregation (#31298) [DOCS] Removes coming tag from 6.3.0 release notes 6.3 release notes. Add notion of internal index settings (#31286) REST high-level client: add Cluster Health API (#29331) Remove leftover usage of deprecated client API SyncedFlushResponse to implement ToXContentObject (#31155) Add Get Aliases API to the high-level REST client (#28799) HLRest: Add get index templates API (#31161) Log warnings when cluster state publication failed to some nodes (#31233) Fix AntFixture waiting condition (#31272) [TEST] Mute RecoveryIT.testHistoryUUIDIsGenerated Ignore numeric shard count if waiting for ALL (#31265) Update checkstyle to 8.10.1 (#31269) Set analyzer version in PreBuiltAnalyzerProviderFactory (#31202) Revert upgrade to Netty 4.1.25.Final (#31282) Use armored input stream for reading public key (#31229) [DOCS] Added 'fail_on_unsupported_field' param to MLT. Closes #28008 (#31160) Fix Netty 4 Server Transport tests. Again. [DOCS] Fixed typo. [DOCS] Added release highlights for 6.3 (#31256) [DOCS] Mark SQL feature as experimental [DOCS] Updates machine learning custom URL screenshots (#31222) Fix naming conventions check for XPackTestCase Fix security Netty 4 transport tests Fix race in clear scroll (#31259) [DOCS] Clarify audit index settings when remote indexing (#30923) [ML][TEST] Mute tests using rules (#31204) Support RequestedAuthnContext (#31238) Validate xContentType in PutWatchRequest. (#31088) [INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024) Upgrade to Netty 4.1.25.Final (#31232) Suppress extras FS on caching directory tests Revert "[DOCS] Added 6.3 info & updated the upgrade table. (#30940)" Revert "Fix snippets in upgrade docs" Fix snippets in upgrade docs [DOCS] Added 6.3 info & updated the upgrade table. (#30940) Enable custom credentials for core REST tests (#31235) Move ESIndexLevelReplicationTestCase to test framework (#31243) Encapsulate Translog in Engine (#31220) [DOCS] Adds machine learning 6.3.0 release notes (#31217) Remove all unused imports and fix CRLF (#31207) [TEST] Fix testRecoveryAfterPrimaryPromotion [Docs] Remove mention pattern files in Grok processor (#31170) Use stronger write-once semantics for Azure repository (#30437) Don't swallow exceptions on replication (#31179) Compliant SAML Response destination check (#31175) Move java version checker back to its own jar (#30708) TEST: Retry synced-flush if ongoing ops on primary (#30978) [test] add fix for rare virtualbox error (#31212)

HLRest: Add get index templates API

b2e11f7

dnhatn added >enhancement v7.0.0 :Core/Java High Level REST Client v6.4.0 labels Jun 6, 2018

dnhatn requested a review from javanna June 6, 2018 22:37

add to supported api list

9587596

cbuescher self-assigned this Jun 7, 2018

cbuescher requested changes Jun 7, 2018

View reviewed changes

dnhatn added 2 commits June 7, 2018 15:17

@cbuescher comments

a6119c3

Merge branch 'master' into hl-rest-get-template

a2dbe0a

dnhatn added 2 commits June 7, 2018 22:36

Merge branch 'master' into hl-rest-get-template

7aa8107

fix test when get both templates

caea845

cbuescher approved these changes Jun 8, 2018

View reviewed changes

javanna approved these changes Jun 8, 2018

View reviewed changes

dnhatn added 4 commits June 10, 2018 15:58

Merge branch 'master' into hl-rest-get-template

16d9e27

getTemplates -> getTemplate

7117a3d

add comment

c4a2bb9

Merge branch 'master' into hl-rest-get-template

b821c39

dnhatn merged commit 99e0458 into elastic:master Jun 11, 2018

dnhatn deleted the hl-rest-get-template branch June 11, 2018 15:06

dnhatn added the backport pending label Jun 11, 2018

javanna mentioned this pull request Jun 11, 2018

Java high-level REST client completeness #27205

Closed

80 tasks

dnhatn added a commit that referenced this pull request Jun 13, 2018

HLRest: Add get index templates API (#31161)

abc6751

Relates #27205

dnhatn removed the backport pending label Jun 13, 2018

colings86 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

HLRest: Add get index templates API #31161

HLRest: Add get index templates API #31161

dnhatn commented Jun 6, 2018

elasticmachine commented Jun 6, 2018

cbuescher left a comment

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

cbuescher Jun 7, 2018

dnhatn commented Jun 7, 2018

cbuescher left a comment

javanna left a comment

javanna Jun 8, 2018

dnhatn Jun 10, 2018

javanna Jun 8, 2018

dnhatn Jun 10, 2018

javanna Jun 8, 2018

cbuescher Jun 8, 2018

dnhatn Jun 10, 2018

javanna Jun 11, 2018

dnhatn Jun 11, 2018

dnhatn commented Jun 11, 2018

HLRest: Add get index templates API #31161

HLRest: Add get index templates API #31161

Conversation

dnhatn commented Jun 6, 2018

elasticmachine commented Jun 6, 2018

cbuescher 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

dnhatn commented Jun 7, 2018

cbuescher left a comment

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnhatn commented Jun 11, 2018