Deprecate types in create index requests. #37134

jtibshirani · 2019-01-04T02:58:52Z

From #29453 and #37285, the include_type_name parameter was already present and defaulted to false. This PR makes the following updates:

Add deprecation warnings to RestCreateIndexAction, plus tests in RestCreateIndexActionTests.
Add a typeless 'create index' method to the Java HLRC, and deprecate the old typed version. To do this cleanly, I created new CreateIndexRequest and CreateIndexResponse objects that differ from the existing server ones.

elasticmachine · 2019-01-04T02:58:53Z

Pinging @elastic/es-search

jtibshirani · 2019-01-04T05:33:24Z

@elasticmachine run gradle build tests 1

…seful for the HLRC.

jtibshirani · 2019-01-18T07:45:50Z

@elasticmachine run gradle build tests 1
@elasticmachine run gradle build tests 2

cbuescher

@jtibshirani I think the general approach to provide two variants, a typed and an un-typed call in the HLRC and to use other Request-Classes for the "new" calls is fine. In this case I'm wondering if we can avoid copying to much code from server, unless we really need it. I left some comments along those lines.

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

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

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

cbuescher · 2019-01-18T16:31:55Z

...st-high-level/src/test/java/org/elasticsearch/client/indices/RandomCreateIndexGenerator.java

+import static org.elasticsearch.test.ESTestCase.randomBoolean;
+import static org.elasticsearch.test.ESTestCase.randomIntBetween;
+
+public final class RandomCreateIndexGenerator {


I understand that this helper creates the new "client-side" CreateIndexRequest, but I'm a bit reluctant to copy yet another full over from test.frameworks. Can you somehow create a simple converter from server-side CreateIndexRequest to client side ones instead? We could then reuse the existing RandomCreateIndexGenerator.

This is a nice idea, I'll try it out. One note is that we'll have to swap out the mappings, since client-side CreateIndexRequest expects typeless mappings, whereas the server-side one always expects typed mappings.

server/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java

…xGenerator.

jtibshirani · 2019-01-21T18:12:18Z

@cbuescher this is ready for another look.

hub-cap

great work. I called out something that was brought over from server (all of the methods to set source), but im not sure we should do anything about it. Im torn between cleaning these client side objects up now, or waiting till we can generate them, and trying to keep them as close to server side as possible, for now. I hate to see them diverge, but I also hate to add all this stuff to client that we will have to maintain. I think im still leaning toward removing whatever we dont think is useful to make the classes as lean as possible, and it means that people wont use methods that dont exist! Im also cool w/ extending a server side class if there is 0 tangible difference, for now. Making the package change is ++.

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

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

hub-cap · 2019-01-22T15:31:35Z

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

+     *
+     * Note that the mapping definition should *not* be nested under a type name.
+     */
+    public CreateIndexRequest source(byte[] source, int offset, int length, XContentType xContentType) {


do we need all these ways to set source here?

in general, i think we should try to clean up these classes if we can, so that people have less ways to set these values on the client side objects.

I'll take a stab at removing some of the less important setters on this class.

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

...level/src/test/java/org/elasticsearch/client/documentation/IndicesClientDocumentationIT.java

jtibshirani · 2019-01-22T22:51:36Z

Thanks @hub-cap for the review! I tried to address your comments.

I think im still leaning toward removing whatever we dont think is useful to make the classes as lean as possible, and it means that people wont use methods that dont exist!

I didn't have a great intuition as to how far to go with the clean-up. I ended up removing two source methods that didn't seem useful in a client context. Another option is to remove all source methods, so users must use the individual setters for mappings, aliases, etc., but I was hesitant to do this because I saw a source method being advertised in the HLRC create index documentation. Please let me know if you have other suggestions for cleaning this up.

…n-create-index

jtibshirani · 2019-01-23T02:19:54Z

@elasticmachine run elasticsearch-ci/1

jtibshirani · 2019-01-23T17:19:00Z

@elasticmachine run elasticsearch-ci/1

jtibshirani · 2019-01-23T17:39:22Z

@elasticmachine run elasticsearch-ci/1

hub-cap

I would be ok with removing all but the source(BytesReference, ... call, but its not that big of a deal. These will, hopefully, one day be replaced by concrete objects so we can leave them all for now.

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

…n-create-index

cbuescher

Thanks @jtibshirani for adressing my comments as well. LGTM

From #29453 and #37285, the include_type_name parameter was already present and defaulted to false. This PR makes the following updates: * Add deprecation warnings to RestCreateIndexAction, plus tests in RestCreateIndexActionTests. * Add a typeless 'create index' method to the Java HLRC, and deprecate the old typed version. To do this cleanly, I created new CreateIndexRequest and CreateIndexResponse objects that differ from the existing server ones.

jtibshirani added :Search Foundations/Mapping Index mappings, including merging and defining field types >deprecation v7.0.0 labels Jan 4, 2019

jtibshirani force-pushed the deprecate-types-in-create-index branch 3 times, most recently from 2788ffe to 6041a84 Compare January 4, 2019 05:26

jtibshirani added the WIP label Jan 10, 2019

Add a deprecation warning to RestCreateIndexAction.

19accde

jtibshirani force-pushed the deprecate-types-in-create-index branch from 6041a84 to 647a732 Compare January 18, 2019 03:00

jtibshirani added v6.7.0 and removed WIP labels Jan 18, 2019

jtibshirani force-pushed the deprecate-types-in-create-index branch from 647a732 to aed279e Compare January 18, 2019 03:08

jtibshirani requested review from hub-cap and cbuescher January 18, 2019 03:11

jtibshirani mentioned this pull request Jan 18, 2019

Implementation tracking for 7.0 types deprecation. #35190

Closed

48 tasks

jtibshirani added 2 commits January 17, 2019 19:43

Introduce a new typeless HLRC method.

6892e0f

Remove CreateIndexRequest#mapping(Object... source), as it's not as u…

8519ca2

…seful for the HLRC.

jtibshirani force-pushed the deprecate-types-in-create-index branch from aed279e to c3cb3b2 Compare January 18, 2019 03:49

More clean-up in CreateIndexRequest.

457a69e

jtibshirani force-pushed the deprecate-types-in-create-index branch from c3cb3b2 to 457a69e Compare January 18, 2019 05:29

cbuescher requested changes Jan 18, 2019

View reviewed changes

jtibshirani added 5 commits January 18, 2019 10:47

Remove the redundant method CreateIndexRequest#waitForActiveShards.

0a6d805

Undo an unintentional change to the server-side CreateIndexResponse.

6329214

Avoid duplicating all logic in RandomCreateIndexGenerator.

c9e20dd

Merge remote-tracking branch 'upstream/master' into pr

2762c77

Update PutMappingRequestTests to use the client-side RandomCreateInde…

49d8e70

…xGenerator.

hub-cap suggested changes Jan 22, 2019

View reviewed changes

jtibshirani added 3 commits January 22, 2019 14:48

Undo an accidental change in IndicesClientDocumentationIT.

3f266f2

In the new CreateIndexRequest, avoid extending IndicesRequest.

455def3

Remove some non-critical setters from CreateIndexRequest.

9c11c67

Merge remote-tracking branch 'upstream/master' into deprecate-types-i…

19efb19

…n-create-index

hub-cap approved these changes Jan 23, 2019

View reviewed changes

client/rest-high-level/src/main/java/org/elasticsearch/client/indices/CreateIndexRequest.java Show resolved Hide resolved

jtibshirani added 4 commits January 23, 2019 15:04

Merge remote-tracking branch 'upstream/master' into deprecate-types-i…

5d0882e

…n-create-index

Merge remote-tracking branch 'upstream/master' into deprecate-types-i…

196e2ad

…n-create-index

Fix a merge conflict.

5ad7b3a

Merge remote-tracking branch 'upstream/master' into deprecate-types-i…

f0c7e55

…n-create-index

cbuescher approved these changes Jan 24, 2019

View reviewed changes

jtibshirani merged commit e1d8df4 into elastic:master Jan 24, 2019

jtibshirani deleted the deprecate-types-in-create-index branch January 24, 2019 21:17

jtibshirani mentioned this pull request Jan 24, 2019

Deprecate types in create index requests (backport to 6.x). #37838

Merged

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

Deprecate types in create index requests. #37134

Deprecate types in create index requests. #37134

jtibshirani commented Jan 4, 2019 •

edited

Loading

elasticmachine commented Jan 4, 2019

jtibshirani commented Jan 4, 2019

jtibshirani commented Jan 18, 2019

cbuescher left a comment

cbuescher Jan 18, 2019

jtibshirani Jan 18, 2019

jtibshirani commented Jan 21, 2019

hub-cap left a comment

hub-cap Jan 22, 2019

hub-cap Jan 22, 2019

jtibshirani Jan 22, 2019

jtibshirani commented Jan 22, 2019

jtibshirani commented Jan 23, 2019

jtibshirani commented Jan 23, 2019

jtibshirani commented Jan 23, 2019

hub-cap left a comment

cbuescher left a comment

Deprecate types in create index requests. #37134

Deprecate types in create index requests. #37134

Conversation

jtibshirani commented Jan 4, 2019 • edited Loading

elasticmachine commented Jan 4, 2019

jtibshirani commented Jan 4, 2019

jtibshirani commented Jan 18, 2019

cbuescher left a comment

Choose a reason for hiding this comment

cbuescher Jan 18, 2019

Choose a reason for hiding this comment

jtibshirani Jan 18, 2019

Choose a reason for hiding this comment

jtibshirani commented Jan 21, 2019

hub-cap left a comment

Choose a reason for hiding this comment

hub-cap Jan 22, 2019

Choose a reason for hiding this comment

hub-cap Jan 22, 2019

Choose a reason for hiding this comment

jtibshirani Jan 22, 2019

Choose a reason for hiding this comment

jtibshirani commented Jan 22, 2019

jtibshirani commented Jan 23, 2019

jtibshirani commented Jan 23, 2019

jtibshirani commented Jan 23, 2019

hub-cap left a comment

Choose a reason for hiding this comment

cbuescher left a comment

Choose a reason for hiding this comment

jtibshirani commented Jan 4, 2019 •

edited

Loading