Add high-level client methods that accept RequestOptions #31069

javanna · 2018-06-04T12:59:23Z

With #30490 we have introduced a new way to provide request options
whenever sending a request using the high-level REST client. Before you
could provide headers as the last argument varargs of each API method,
now you can provide RequestOptions that in the future will allow to
provide more options which can be specified per request.

This commit deprecates all of the client methods that accept a Header
varargs argument in favour of new methods that accept RequestOptions
instead. For some API we don't even go through deprecation given that
they were not released since they were added, hence in that case we can
just move them to the new method.

With elastic#30490 we have introduced a new way to provide request options whenever sending a request using the high-level REST client. Before you could provide headers as the last argument varargs of each API method, now you can provide `RequestOptions` that in the future will allow to provide more options which can be specified per request. This commit deprecates all of the client methods that accept a `Header` varargs argument in favour of new methods that accept `RequestOptions` instead. For some API we don't even go through deprecation given that they were not released since they were added, hence in that case we can just move them to the new method.

elasticmachine · 2018-06-04T12:59:25Z

Pinging @elastic/es-core-infra

hub-cap

Few small changes.

hub-cap · 2018-06-04T13:57:03Z

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

+     * <p>
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-flush.html"> Flush API on elastic.co</a>
+     * @deprecated Prefer {@link #flushAsync(FlushRequest, RequestOptions, ActionListener)}
+     */


missed a @deprecated

this I had already fixed like other things, but I forgot to push my latest commit :)

hub-cap · 2018-06-04T13:59:02Z

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

+     * <p>
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-update-settings.html"> Update Indices Settings
+     * API on elastic.co</a>
+     * @deprecated Prefer {@link #putMappingAsync(PutMappingRequest, RequestOptions, ActionListener)}


s/putMappingAsync/putSettingsAsync

wow that's a good catch :) thanks for finding it.

nik9000

I left a minor thing. Other than @hub-cap's thing I think it is great! Thanks for doing this. I was going to start on it after the node selector business but you got it!

I think we'll want to do some documentation changes after this but I think you or I should do it in a followup.

nik9000 · 2018-06-04T14:48:30Z

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

@@ -146,6 +224,20 @@ public void putMappingAsync(PutMappingRequest putMappingRequest, ActionListener<
     * "https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-aliases.html">
     * Index Aliases API on elastic.co</a>


I think it'd be nice to add an @param options to the Javadoc to describe this and to point folks to RequestOptions.DEFAULTS.

hub-cap · 2018-06-04T17:13:17Z

def +1 to this being better than the Header... before.

hub-cap

not sure why i -1'd the changes, this looks fine sans the nits.

hub-cap · 2018-06-04T20:45:56Z

nik9000 · 2018-06-06T14:59:35Z

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

+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/cluster-update-settings.html"> Cluster Update Settings
+     * API on elastic.co</a>
+     * @param clusterUpdateSettingsRequest the request
+     * @param options the request options (e.g. headers), or {@link RequestOptions#DEFAULT} if nothing needs to be customized


s/or/use/ ?

nik9000 · 2018-06-06T14:59:59Z

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

+     * Updates cluster wide specific settings using the Cluster Update Settings API.
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/cluster-update-settings.html"> Cluster Update Settings
+     * API on elastic.co</a>
+     * @param clusterUpdateSettingsRequest the request


I'm not really sure this needs a param tag but it doesn't hurt anything.

I went for having all the param tags rather than only some of them

nik9000 · 2018-06-06T15:00:34Z

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

+     * API on elastic.co</a>
+     * @param clusterUpdateSettingsRequest the request
+     * @param options the request options (e.g. headers), or {@link RequestOptions#DEFAULT} if nothing needs to be customized
+     * @return the response


I don't think the@return tag adds anything here. I think it is fairly obvious from the type of the response.

agreed, I added them for consistency reasons, given that we added param tags it felt weird not to have also the return one.

This PR removes all the API methods that accept a `Header` varargs argument, in favour of the newly introduced API methods that accept a `RequestOptions` argument. Relates to elastic#31069

With #30490 we have introduced a new way to provide request options whenever sending a request using the high-level REST client. Before you could provide headers as the last argument varargs of each API method, now you can provide `RequestOptions` that in the future will allow to provide more options which can be specified per request. This commit deprecates all of the client methods that accept a `Header` varargs argument in favour of new methods that accept `RequestOptions` instead. For some API we don't even go through deprecation given that they were not released since they were added, hence in that case we can just move them to the new method.

* 6.x: Move default location of dependencies report (#31228) Remove dependencies report task dependencies (#31227) Add recognition of MPL 2.0 (#31226) Fix unknown licenses (#31223) Fully encapsulate LocalCheckpointTracker inside of the engine (#31213) Remove version from license file name for GCS SDK (#31221) Remove DocumentFieldMappers#simpleMatchToFullName. (#31041) [DOCS] Removes 6.3.1 release notes [DOCS] Splits release notes by major version Remove DocumentFieldMappers#smartNameFieldMapper, as it is no longer needed. (#31018) Remove extraneous references to 'tokenized' in the mapper code. (#31010) SQL: Make a single JDBC driver jar (#31012) QA: Fix rolling restart tests some more Allow to trim all ops above a certain seq# with a term lower than X high level REST api: cancel task (#30745) Mute TokenBackwardsCompatibilityIT.testMixedCluster Mute WatchBackwardsCompatibilityIT.testWatcherRestart Enhance license detection for various licenses (#31198) [DOCS] Add note about long-lived idle connections (#30990) Add high-level client methods that accept RequestOptions (#31069) Remove RestGetAllMappingsAction (#31129) Move RestGetSettingsAction to RestToXContentListener (#31101) Move number of language analyzers to analysis-common module (#31143) flush job to ensure all results have been written (#31187)

This commit removes all the API methods that accept a `Header` varargs argument, in favour of the newly introduced API methods that accept a `RequestOptions` argument. Relates to #31069

See #31200 & ##31069

javanna added >deprecation v7.0.0 :Core/Java High Level REST Client v6.4.0 labels Jun 4, 2018

javanna requested a review from nik9000 June 4, 2018 12:59

hub-cap suggested changes Jun 4, 2018

View reviewed changes

nik9000 approved these changes Jun 4, 2018

View reviewed changes

iter

caf3bfc

hub-cap approved these changes Jun 4, 2018

View reviewed changes

address comment

2347cdf

javanna mentioned this pull request Jun 6, 2018

high level REST api: cancel task #30745

Merged

javanna added 4 commits June 6, 2018 11:37

Merge branch 'master' into enhancement/client_methods_deprecate

519e45b

add @param tags to javadocs

53be62b

add punctuation marks

cb6331b

fix compile errors

cabeea9

nik9000 reviewed Jun 6, 2018

View reviewed changes

update docs

aee9dd3

nik9000 approved these changes Jun 6, 2018

View reviewed changes

javanna merged commit be4a101 into elastic:master Jun 6, 2018

javanna added the backport pending label Jun 6, 2018

javanna mentioned this pull request Jun 8, 2018

REST high-level Client: remove deprecated API methods #31200

Merged

javanna removed the backport pending label Jun 8, 2018

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

Remove leftover usage of deprecated client API

8953adc

See #31200 & ##31069

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

Add high-level client methods that accept RequestOptions #31069

Add high-level client methods that accept RequestOptions #31069

javanna commented Jun 4, 2018 •

edited by nik9000

Loading

elasticmachine commented Jun 4, 2018

hub-cap left a comment

hub-cap Jun 4, 2018

javanna Jun 4, 2018

hub-cap Jun 4, 2018

javanna Jun 4, 2018

nik9000 left a comment

nik9000 Jun 4, 2018

hub-cap commented Jun 4, 2018

hub-cap left a comment

hub-cap commented Jun 4, 2018

nik9000 Jun 6, 2018

nik9000 Jun 6, 2018

javanna Jun 6, 2018

nik9000 Jun 6, 2018

javanna Jun 6, 2018

		@@ -146,6 +224,20 @@ public void putMappingAsync(PutMappingRequest putMappingRequest, ActionListener<
		* "https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-aliases.html">
		* Index Aliases API on elastic.co</a>

Add high-level client methods that accept RequestOptions #31069

Add high-level client methods that accept RequestOptions #31069

Conversation

javanna commented Jun 4, 2018 • edited by nik9000 Loading

elasticmachine commented Jun 4, 2018

hub-cap 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

nik9000 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hub-cap commented Jun 4, 2018

hub-cap left a comment

Choose a reason for hiding this comment

hub-cap commented Jun 4, 2018

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

javanna commented Jun 4, 2018 •

edited by nik9000

Loading