Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Deprecate _type in simulate pipeline requests #37949

Merged
merged 15 commits into from
Feb 4, 2019
Merged

Deprecate _type in simulate pipeline requests #37949

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
3d47d5f
Deprecate _type in simulate pipeline requests
gwbrown Jan 23, 2019
1a6c9b0
Update docs
gwbrown Jan 23, 2019
8a85a2f
Cleanup
gwbrown Jan 29, 2019
5d8cacd
Tweak test setup
gwbrown Jan 29, 2019
7a3a08c
Omit _type from response if not present in request
gwbrown Jan 29, 2019
6fcccd9
Always test with and without types in both tests
gwbrown Jan 29, 2019
acc857f
Remove _type from docs entirely
gwbrown Jan 29, 2019
5524c2a
Merge branch 'master' into ingest/deprecate-types
gwbrown Jan 29, 2019
8d3ee4d
Fix docs to remove _type as needed
gwbrown Jan 29, 2019
b9fb127
Revert "Omit _type from response if not present in request"
gwbrown Feb 1, 2019
1a5949f
Revert "Remove _type from docs entirely"
gwbrown Feb 1, 2019
16ae2c5
Revert "Fix docs to remove _type as needed"
gwbrown Feb 1, 2019
395c533
Review feedback, change to _doc
gwbrown Feb 1, 2019
138c81f
Merge branch 'master' into ingest/deprecate-types
gwbrown Feb 1, 2019
beaa82e
Unused import
gwbrown Feb 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 0 additions & 10 deletions docs/reference/ingest/apis/simulate-pipeline.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@ POST _ingest/pipeline/_simulate
"docs": [
{
"_index": "index",
"_type": "_doc",
"_id": "id",
"_source": {
"foo": "bar"
}
},
{
"_index": "index",
"_type": "_doc",
"_id": "id",
"_source": {
"foo": "rab"
Expand All @@ -94,7 +92,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field2": "_value",
"foo": "bar"
Expand All @@ -108,7 +105,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field2": "_value",
"foo": "rab"
Expand Down Expand Up @@ -158,15 +154,13 @@ POST _ingest/pipeline/_simulate?verbose
"docs": [
{
"_index": "index",
"_type": "_doc",
"_id": "id",
"_source": {
"foo": "bar"
}
},
{
"_index": "index",
"_type": "_doc",
"_id": "id",
"_source": {
"foo": "rab"
Expand All @@ -189,7 +183,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field2": "_value2",
"foo": "bar"
Expand All @@ -203,7 +196,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field3": "_value3",
"field2": "_value2",
Expand All @@ -222,7 +214,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field2": "_value2",
"foo": "rab"
Expand All @@ -236,7 +227,6 @@ Response:
"doc": {
"_id": "id",
"_index": "index",
"_type": "_doc",
"_source": {
"field3": "_value3",
"field2": "_value2",
Expand Down
1 change: 0 additions & 1 deletion docs/reference/ingest/processors/date-index-name.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,6 @@ and the result:
"doc" : {
"_id" : "_id",
"_index" : "<myindex-{2016-04-25||/M{yyyy-MM-dd|UTC}}>",
"_type" : "_type",
"_source" : {
"date1" : "2016-04-25T12:02:01.789Z"
},
Expand Down
2 changes: 0 additions & 2 deletions docs/reference/ingest/processors/grok.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ response:
"docs": [
{
"doc": {
"_type": "_type",
"_index": "_index",
"_id": "_id",
"_source": {
Expand Down Expand Up @@ -254,7 +253,6 @@ POST _ingest/pipeline/_simulate
"docs": [
{
"doc": {
"_type": "_type",
"_index": "_index",
"_id": "_id",
"_source": {
Expand Down
13 changes: 11 additions & 2 deletions server/src/main/java/org/elasticsearch/action/ingest/SimulatePipelineRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,14 @@

package org.elasticsearch.action.ingest;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.ToXContentObject;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentType;
Expand All @@ -43,6 +46,9 @@

public class SimulatePipelineRequest extends ActionRequest implements ToXContentObject {

private static final Logger logger = LogManager.getLogger(SimulatePipelineRequest.class);
private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);

private String id;
private boolean verbose;
private BytesReference source;
Expand Down Expand Up @@ -178,8 +184,11 @@ private static List<IngestDocument> parseDocs(Map<String, Object> config) {
dataMap, Fields.SOURCE);
String index = ConfigurationUtils.readStringOrIntProperty(null, null,
dataMap, MetaData.INDEX.getFieldName(), "_index");
String type = ConfigurationUtils.readStringOrIntProperty(null, null,
dataMap, MetaData.TYPE.getFieldName(), "_type");
if (dataMap.containsKey(MetaData.TYPE.getFieldName())) {
deprecationLogger.deprecatedAndMaybeLog("type_in_pipeline_simulation",
Copy link
Contributor

@jtibshirani jtibshirani Jan 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"Specifying _type in pipeline simulation requests is deprecated.");
}
String type = (String) dataMap.get(MetaData.TYPE.getFieldName());
String id = ConfigurationUtils.readStringOrIntProperty(null, null,
dataMap, MetaData.ID.getFieldName(), "_id");
String routing = ConfigurationUtils.readOptionalStringOrIntProperty(null, null,
Expand Down
59 changes: 45 additions & 14 deletions ...er/src/test/java/org/elasticsearch/action/ingest/SimulatePipelineRequestParsingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,6 @@

package org.elasticsearch.action.ingest;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import org.elasticsearch.index.VersionType;
import org.elasticsearch.ingest.CompoundProcessor;
import org.elasticsearch.ingest.IngestDocument;
Expand All @@ -38,6 +29,15 @@
import org.elasticsearch.test.ESTestCase;
import org.junit.Before;

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;

import static org.elasticsearch.action.ingest.SimulatePipelineRequest.Fields;
import static org.elasticsearch.action.ingest.SimulatePipelineRequest.SIMULATED_PIPELINE_ID;
import static org.elasticsearch.ingest.IngestDocument.MetaData.ID;
Expand Down Expand Up @@ -67,7 +67,15 @@ public void init() throws IOException {
when(ingestService.getProcessorFactories()).thenReturn(registry);
}

public void testParseUsingPipelineStore() throws Exception {
public void testParseUsingPipelineStoreNoType() throws Exception {
innerTestParseUsingPipelineStore(false);
}

public void testParseUsingPipelineStoreWithType() throws Exception {
innerTestParseUsingPipelineStore(true);
}

private void innerTestParseUsingPipelineStore(boolean useExplicitType) throws Exception {
int numDocs = randomIntBetween(1, 10);

Map<String, Object> requestContent = new HashMap<>();
Expand All @@ -80,15 +88,19 @@ public void testParseUsingPipelineStore() throws Exception {
String type = randomAlphaOfLengthBetween(1, 10);
String id = randomAlphaOfLengthBetween(1, 10);
doc.put(INDEX.getFieldName(), index);
doc.put(TYPE.getFieldName(), type);
if (useExplicitType) {
doc.put(TYPE.getFieldName(), type);
}
doc.put(ID.getFieldName(), id);
String fieldName = randomAlphaOfLengthBetween(1, 10);
String fieldValue = randomAlphaOfLengthBetween(1, 10);
doc.put(Fields.SOURCE, Collections.singletonMap(fieldName, fieldValue));
docs.add(doc);
Map<String, Object> expectedDoc = new HashMap<>();
expectedDoc.put(INDEX.getFieldName(), index);
expectedDoc.put(TYPE.getFieldName(), type);
if (useExplicitType) {
expectedDoc.put(TYPE.getFieldName(), type);
}
expectedDoc.put(ID.getFieldName(), id);
expectedDoc.put(Fields.SOURCE, Collections.singletonMap(fieldName, fieldValue));
expectedDocs.add(expectedDoc);
Expand All @@ -111,9 +123,20 @@ public void testParseUsingPipelineStore() throws Exception {
assertThat(actualRequest.getPipeline().getId(), equalTo(SIMULATED_PIPELINE_ID));
assertThat(actualRequest.getPipeline().getDescription(), nullValue());
assertThat(actualRequest.getPipeline().getProcessors().size(), equalTo(1));
if (useExplicitType) {
assertWarnings("Specifying _type in pipeline simulation requests is deprecated.");
}
}

public void testParseWithProvidedPipelineNoType() throws Exception {
innerTestParseWithProvidedPipeline(false);
}

public void testParseWithProvidedPipeline() throws Exception {
public void testParseWithProvidedPipelineWithType() throws Exception {
innerTestParseWithProvidedPipeline(true);
}

private void innerTestParseWithProvidedPipeline(boolean useExplicitType) throws Exception {
int numDocs = randomIntBetween(1, 10);

Map<String, Object> requestContent = new HashMap<>();
Expand All @@ -135,6 +158,12 @@ public void testParseWithProvidedPipeline() throws Exception {
);
doc.put(field.getFieldName(), value);
expectedDoc.put(field.getFieldName(), value);
} else if (field == TYPE) {
if (useExplicitType) {
String value = randomAlphaOfLengthBetween(1, 10);
doc.put(field.getFieldName(), value);
expectedDoc.put(field.getFieldName(), value);
}
} else {
if (randomBoolean()) {
String value = randomAlphaOfLengthBetween(1, 10);
Expand Down Expand Up @@ -191,7 +220,6 @@ public void testParseWithProvidedPipeline() throws Exception {
Map<String, Object> expectedDocument = expectedDocsIterator.next();
Map<IngestDocument.MetaData, Object> metadataMap = ingestDocument.extractMetadata();
assertThat(metadataMap.get(INDEX), equalTo(expectedDocument.get(INDEX.getFieldName())));
assertThat(metadataMap.get(TYPE), equalTo(expectedDocument.get(TYPE.getFieldName())));
assertThat(metadataMap.get(ID), equalTo(expectedDocument.get(ID.getFieldName())));
assertThat(metadataMap.get(ROUTING), equalTo(expectedDocument.get(ROUTING.getFieldName())));
assertThat(metadataMap.get(VERSION), equalTo(expectedDocument.get(VERSION.getFieldName())));
Expand All @@ -202,6 +230,9 @@ public void testParseWithProvidedPipeline() throws Exception {
assertThat(actualRequest.getPipeline().getId(), equalTo(SIMULATED_PIPELINE_ID));
assertThat(actualRequest.getPipeline().getDescription(), nullValue());
assertThat(actualRequest.getPipeline().getProcessors().size(), equalTo(numProcessors));
if (useExplicitType) {
assertWarnings("Specifying _type in pipeline simulation requests is deprecated.");
}
}

public void testNullPipelineId() {
Expand Down