Deprecate types in the put mapping API. by jtibshirani · Pull Request #37280 · elastic/elasticsearch

jtibshirani · 2019-01-09T21:37:57Z

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 RestPutMappingAction, plus tests in RestPutMappingActionTests.
Add a typeless 'put mappings' method to the Java HLRC, and deprecate the old typed version. To do this cleanly, I opted to create a new PutMappingRequest object that differs from the existing server one.

elasticmachine · 2019-01-09T21:38:00Z

Pinging @elastic/es-search

jtibshirani · 2019-01-15T21:58:14Z

rest-api-spec/src/main/resources/rest-api-spec/test/indices.put_mapping/10_basic.yml

This was replaced by a unit test in RestPutMappingActionTests.

cbuescher

@jtibshirani thanks, I left some comments. I think introducing a dedicated new request object for the HLRC side here helps if that is acceptable by all parties. I left some comments about whether we should replicate some of the existing API setters if we are willing to make a fresh start on the client side anyway but I think that discussion needs to involve a few more people.

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

...t/rest-high-level/src/test/java/org/elasticsearch/client/indices/PutMappingRequestTests.java

cbuescher · 2019-01-16T15:26:02Z

test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java

+    /**
+     * Creates a random mapping, with no mention of types.
+     */
+    public static XContentBuilder randomMapping() throws IOException {


Could this be used in the "old" PutMappingRequestTests where it seems to be pulled from as well?

Sure, I can update the old PutMappingRequestTests as well.

Trying to make this switch actually exposed a bug in the server-side PutMappingRequest. If it's okay with you, I'd like to fix this in another PR.

Great you found it. Would be nice to track this in an issue then so we don't forget it. Can you open one please?

An update: I ended up opening a PR to fix the issue (#37720).

…eful for the HLRC.

…ent.

…n-put-mapping

jtibshirani · 2019-01-17T06:01:56Z

@elasticmachine test this please

hub-cap

I only had 1 question, the rest stuff LGTM.

hub-cap · 2019-01-17T14:35:18Z

server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestPutMappingAction.java

        final boolean includeTypeName = request.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER,
            DEFAULT_INCLUDE_TYPE_NAME_POLICY);
-        PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index")));
+        if (request.hasParam(INCLUDE_TYPE_NAME_PARAMETER)) {


is this supposed to log even if include_type_name is set to false?

Yes, on master we planned to issue a deprecation warning if the parameter is set at all. This is because the parameter is going to be removed in 8.0.

cbuescher · 2019-01-17T17:06:55Z

test/framework/src/main/java/org/elasticsearch/index/RandomCreateIndexGenerator.java

     */
    public static XContentBuilder randomMapping() throws IOException {
-        XContentBuilder builder = XContentFactory.jsonBuilder();
+        XContentBuilder builder = XContentFactory.contentBuilder(randomFrom(XContentType.values()));


This might have side-effects on other tests using this. Its good that its tested though. Maybe make sure to run a few of the tests that somehow use this with a couple of repetitions locally (@repeat) just to be sure and not be suprised by test failures some time later on CI.

cbuescher

I left two comments but nothing that should prevent this from going in as soon as tests pass.

markharwood · 2019-01-17T17:43:07Z

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

-        putMappingRequest.type("_doc");
        XContentBuilder mappingBuilder = JsonXContent.contentBuilder();
        mappingBuilder.startObject().startObject("properties").startObject("field");
        mappingBuilder.field("type", "text");


a little later in this test method we still have a type ref:

Map<String, Object> mappings = getMappingsResponse.getMappings().get(indexName).get("_doc").sourceAsMap();

@jpountz said in a recent email thread "we should feel free to use better objects"
Is that something we should address in this PR?

This PR is just focused on put mappings (and not get mappings), so I think we should plan to address that in a subsequent PR.

jtibshirani · 2019-01-17T18:58:47Z

@elasticmachine test this please

jtibshirani · 2019-01-17T22:33:24Z

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

…n-put-mapping

…me name.

jtibshirani · 2019-01-18T05:31:28Z

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

jtibshirani · 2019-01-18T17:28:36Z

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

jtibshirani · 2019-01-18T18:13:46Z

@elasticmachine run gradle build tests 2

jtibshirani · 2019-01-18T18:48:35Z

@elasticmachine run gradle build tests 2

* elastic/master: (104 commits) Permission for restricted indices (elastic#37577) Remove Watcher Account "unsecure" settings (elastic#36736) Add cache cleaning task for ML snapshot (elastic#37505) Update jdk used by the docker builds (elastic#37621) Remove an unused constant in PutMappingRequest. Update get users to allow unknown fields (elastic#37593) Do not add index event listener if CCR disabled (elastic#37432) Add local session timeouts to leader node (elastic#37438) Add some deprecation optimizations (elastic#37597) refactor inner geogrid classes to own class files (elastic#37596) Remove obsolete deprecation checks (elastic#37510) ML: Add support for single bucket aggs in Datafeeds (elastic#37544) ML: creating ML State write alias and pointing writes there (elastic#37483) Deprecate types in the put mapping API. (elastic#37280) [ILM] Add unfollow action (elastic#36970) Packaging: Update marker used to allow ELASTIC_PASSWORD (elastic#37243) Fix setting openldap realm ssl config Document the need for JAVA11_HOME (elastic#37589) SQL: fix object extraction from sources (elastic#37502) Nit in settings.gradle for Eclipse ...

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

jtibshirani force-pushed the deprecate-types-in-put-mapping branch 2 times, most recently from ee86fc8 to d55dc19 Compare January 9, 2019 23:29

jtibshirani mentioned this pull request Jan 9, 2019

Implementation tracking for 7.0 types deprecation. #35190

Closed

48 tasks

jtibshirani added the WIP label Jan 10, 2019

Deprecate types in the put mapping API.

5446f57

jtibshirani force-pushed the deprecate-types-in-put-mapping branch from d55dc19 to 027ce6d Compare January 15, 2019 21:54

jtibshirani added >breaking-java and removed >breaking WIP labels Jan 15, 2019

jtibshirani commented Jan 15, 2019

View reviewed changes

Remove a REST test that is already covered by a unit test.

79a17d6

jtibshirani force-pushed the deprecate-types-in-put-mapping branch from 339de14 to 4f94888 Compare January 16, 2019 00:17

Add a typeless put mapping method to the HLRC.

49964a0

jtibshirani force-pushed the deprecate-types-in-put-mapping branch from 4f94888 to 49964a0 Compare January 16, 2019 00:23

jtibshirani requested review from cbuescher and hub-cap January 16, 2019 00:23

jtibshirani removed the >breaking-java label Jan 16, 2019

cbuescher requested changes Jan 16, 2019

View reviewed changes

jtibshirani added 3 commits January 16, 2019 11:24

Remove PutMappingRequest#source(Object... source), as it is not as us…

4b7fb35

…eful for the HLRC.

Make sure to use the right xContent type in PutMappingRequest#toXCont…

9aad5dd

…ent.

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

634d1e0

…n-put-mapping

hub-cap approved these changes Jan 17, 2019

View reviewed changes

cbuescher reviewed Jan 17, 2019

View reviewed changes

cbuescher approved these changes Jan 17, 2019

View reviewed changes

markharwood reviewed Jan 17, 2019

View reviewed changes

jtibshirani added 3 commits January 17, 2019 17:14

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

b96cb3a

…n-put-mapping

Make sure RestHighLevelClientTests can handle two methods with the sa…

6335ca5

…me name.

Fix static analysis violations.

314fb3b

jtibshirani added the v6.7.0 label Jan 18, 2019

Correct a deprecation comment in IndicesClient.

393877a

Fix IndicesClientDocumentationIT#testAnalyze.

8ba19ac

jtibshirani merged commit 8da7a27 into elastic:master Jan 18, 2019

jtibshirani deleted the deprecate-types-in-put-mapping branch January 18, 2019 20:28

jtibshirani mentioned this pull request Jan 19, 2019

Deprecate types in put mapping (backport to 6.x). #37627

Merged

jtibshirani mentioned this pull request Jan 22, 2019

Types removal - deprecate include_type_name with index templates #37484

Merged

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

Conversation

jtibshirani commented Jan 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Jan 9, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbuescher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtibshirani commented Jan 17, 2019

Uh oh!

hub-cap left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbuescher left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtibshirani commented Jan 17, 2019

Uh oh!

jtibshirani commented Jan 17, 2019

Uh oh!

jtibshirani commented Jan 18, 2019

Uh oh!

jtibshirani commented Jan 18, 2019

Uh oh!

jtibshirani commented Jan 18, 2019

Uh oh!

jtibshirani commented Jan 18, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jtibshirani commented Jan 9, 2019 •

edited

Loading