Deprecate `_type` in simulate pipeline requests by AthenaEryma · Pull Request #37949 · elastic/elasticsearch

AthenaEryma · 2019-01-29T01:15:20Z

As mapping types are being removed throughout Elasticsearch, the use of
_type in pipeline simulation requests is deprecated.

Closes #37731

As mapping types are being removed throughout Elasticsearch, the use of `_type` in pipeline simulation requests is deprecated.

elasticmachine · 2019-01-29T01:15:21Z

Pinging @elastic/es-core-features

jpountz

Thanks @gwbrown for tackling this.

jpountz · 2019-01-29T08:41:28Z

server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java

+                    "Specifying _type in pipeline simulation requests is deprecated.");
+            }
            String type = ConfigurationUtils.readStringOrIntProperty(null, null,
                dataMap, MetaData.TYPE.getFieldName(), "_type");


we have been using _doc as a default type name elsewhere (there is a constant for it at MapperService.SINGLE_TYPE_NAME)

I did notice that, but I wasn't sure if we wanted to switch it out here as this API has been using _type as the default for quite a while. What are your thoughts, @jakelandis?

+1 to changing to _doc

server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java

jakelandis · 2019-01-29T17:13:08Z

LGTM with a minor preference to use _doc as the default and explicitly test the both cases (as @jpountz comments)

jakelandis · 2019-01-29T17:15:37Z

docs/reference/ingest/apis/simulate-pipeline.asciidoc

    {
      "_index": "index",
-      "_type": "_doc",
+      "_type": "_type",


I would go ahead and remove the _type example here (and below) as well. We don't to encourage users to do anything with it.

This is an example of the output of the call, which will have the _type field present. However, it looks like we can just provide null for the type to the IngestDocument if one isn't provided in the request and _type will be omitted from the response - what do you think of doing that instead of changing the default to _doc?

I've removed _type from all of the example inputs already.

However, it looks like we can just provide null for the type to the IngestDocument if one isn't provided in the request and _type will be omitted from the response - what do you think of doing that instead of changing the default to _doc

I think we should keep the "_type": "_doc" on the return values since it better mirrors the return of a typeless search

POST foo/_create/1 {} GET foo/_search

Also, this line (164) is from the request, and there are some responses with it missing (for example 186-187).

+1 to @jakelandis's recommendation!

AthenaEryma · 2019-01-29T18:48:47Z

I've pushed changes that:

Make the requested changes to tests,
Omit _type entirely if it isn't provided in the request rather than providing a default, and
Removed _type from the docs as appropriate per the change in 2).

How do folks feel about this as opposed to changing the default _type to _doc?

jtibshirani · 2019-01-30T21:28:11Z

server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java

-            String type = ConfigurationUtils.readStringOrIntProperty(null, null,
-                dataMap, MetaData.TYPE.getFieldName(), "_type");
+            if (dataMap.containsKey(MetaData.TYPE.getFieldName())) {
+                deprecationLogger.deprecatedAndMaybeLog("type_in_pipeline_simulation",


Small comment, but for consistency with our other logging statements it'd be nice to (1) use the key simulate_pipeline_with_types, and (2) prefix the warning message with [types removal]. Adding a prefix to messages was a bit unusual, but we needed a way to identify and ignore types-related warnings in tests.

jtibshirani · 2019-01-30T21:30:13Z

docs/reference/ingest/apis/simulate-pipeline.asciidoc

    {
      "_index": "index",
-      "_type": "_doc",
+      "_type": "_type",


+1 to @jakelandis's recommendation!

jtibshirani · 2019-01-30T21:36:03Z

One note in addition to the small comments above: I think we'll need to update IngestClientDocumentationIT and IngestClientIT, as they currently make HLRC calls that reference _type.

This reverts commit 7a3a08c.

This reverts commit acc857f.

This reverts commit 8d3ee4d.

jakelandis

Still LGTM !

…-lease-expiration * elastic/master: (24 commits) Add support for API keys to access Elasticsearch (elastic#38291) Add typless client side GetIndexRequest calls and response class (elastic#37778) Limit token expiry to 1 hour maximum (elastic#38244) add docs saying mixed-cluster ILM is not supported (elastic#37954) Skip unsupported languages for tests (elastic#38328) Deprecate `_type` in simulate pipeline requests (elastic#37949) Mute testCannotShrinkLeaderIndex (elastic#38374) Tighten mapping syncing in ccr remote restore (elastic#38071) Add test for `PutFollowAction` on a closed index (elastic#38236) Fix SSLContext pinning to TLSV1.2 in reload tests (elastic#38341) Mute RareClusterStateIT.testDelayedMappingPropagationOnReplica (elastic#38357) Deprecate types in rollover index API (elastic#38039) Types removal - fix FullClusterRestartIT warning expectations (elastic#38310) Fix ILM explain response to allow unknown fields (elastic#38054) Mute testFollowIndexAndCloseNode (elastic#38360) Docs: Drop inline callout from scroll example (elastic#38340) Deprecate HLRC security methods (elastic#37883) Remove types from Monitoring plugin "backend" code (elastic#37745) Add Composite to AggregationBuilders (elastic#38207) Clarify slow cluster-state log messages (elastic#38302) ...

AthenaEryma added 4 commits January 28, 2019 18:04

Deprecate _type in simulate pipeline requests

3d47d5f

As mapping types are being removed throughout Elasticsearch, the use of `_type` in pipeline simulation requests is deprecated.

Update docs

1a6c9b0

Cleanup

8a85a2f

Tweak test setup

5d8cacd

AthenaEryma added >deprecation :Distributed/Ingest Node Execution or management of Ingest Pipelines v7.0.0 labels Jan 29, 2019

AthenaEryma requested a review from jakelandis January 29, 2019 01:15

jtibshirani mentioned this pull request Jan 29, 2019

Implementation tracking for 7.0 types deprecation. #35190

Closed

48 tasks

jpountz reviewed Jan 29, 2019

View reviewed changes

server/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java Outdated Show resolved Hide resolved

jakelandis approved these changes Jan 29, 2019

View reviewed changes

jakelandis reviewed Jan 29, 2019

View reviewed changes

AthenaEryma added 4 commits January 29, 2019 11:28

Omit _type from response if not present in request

7a3a08c

Always test with and without types in both tests

6fcccd9

Remove _type from docs entirely

acc857f

Merge branch 'master' into ingest/deprecate-types

5524c2a

Fix docs to remove _type as needed

8d3ee4d

jtibshirani reviewed Jan 30, 2019

View reviewed changes

AthenaEryma added 6 commits February 1, 2019 15:00

Revert "Omit _type from response if not present in request"

b9fb127

This reverts commit 7a3a08c.

Revert "Remove _type from docs entirely"

1a5949f

This reverts commit acc857f.

Revert "Fix docs to remove _type as needed"

16ae2c5

This reverts commit 8d3ee4d.

Review feedback, change to _doc

395c533

Merge branch 'master' into ingest/deprecate-types

138c81f

Unused import

beaa82e

jakelandis approved these changes Feb 4, 2019

View reviewed changes

AthenaEryma merged commit 292e0f6 into elastic:master Feb 4, 2019

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

Conversation

AthenaEryma commented Jan 29, 2019

Uh oh!

elasticmachine commented Jan 29, 2019

Uh oh!

jpountz left a comment

Choose a reason for hiding this comment

Uh oh!

jpountz Jan 29, 2019

Choose a reason for hiding this comment

Uh oh!

AthenaEryma Jan 29, 2019

Choose a reason for hiding this comment

Uh oh!

jakelandis Jan 29, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jakelandis commented Jan 29, 2019

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakelandis Jan 29, 2019

Choose a reason for hiding this comment

Uh oh!

jtibshirani Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jtibshirani Jan 30, 2019

Choose a reason for hiding this comment

Uh oh!

jtibshirani commented Jan 30, 2019

Uh oh!

jakelandis left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jakelandis Jan 29, 2019 •

edited

Loading

AthenaEryma Jan 29, 2019 •

edited

Loading

AthenaEryma commented Jan 29, 2019 •

edited

Loading

jtibshirani Jan 30, 2019 •

edited

Loading