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.

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

Remove CreateIndexRequest.addMapping(type, string, xcontenttype) #50419

Merged

Conversation

romseygeek
Copy link
Contributor

We still have a number of places, mainly in test code but some in production, that
are building mappings with a named type as the root of a map. CreateIndexRequest
handles this automatically, but PutMappingRequest does not, which is a bit trappy -
we can get situations like #50359 where the same mapping will work when an
index is created but fail on an update.

This commit is a first step to removing the leniency in CreateIndexRequest so that
we can catch mappings with a named type root earlier.

Relates to #41059

@romseygeek romseygeek added :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v8.0.0 labels Dec 20, 2019
@romseygeek romseygeek self-assigned this Dec 20, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/CRUD)

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM

String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject().startObject("type1")
.startObject("properties").startObject("appAccountIds").field("type", "text").endObject().endObject()
String mapping = Strings.toString(XContentFactory.jsonBuilder().startObject()
.startObject("properties").startObject("appAccountIds").field("type", "text").endObject()
Copy link
Member

Choose a reason for hiding this comment

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

I try to re-indent these when I see them to make them a bit more readable.

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek romseygeek merged commit 418edc6 into elastic:master Jan 2, 2020
@romseygeek romseygeek deleted the types-removal/create-index-request branch January 2, 2020 11:14
romseygeek added a commit that referenced this pull request Jan 8, 2020
…Builder)` (#50586)

This continues the removal of type parameters from CreateIndexRequest.mapping
methods started in #50419. Here the removed methods are almost entirely in test
code, with the exception of a change to TransformIndex in the transform plugin.

Relates to #41059
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…stic#50419)

We still have a number of places, mainly in test code but some in production, that
are building mappings with a named type as the root of a map. CreateIndexRequest
handles this automatically, but PutMappingRequest does not, which is a bit trappy -
we can get situations like elastic#50359 where the same mapping will work when an
index is created but fail on an update.

This commit is a first step to removing the leniency in CreateIndexRequest so that
we can catch mappings with a named type root earlier.

Relates to elastic#41059
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this pull request Jan 23, 2020
…Builder)` (elastic#50586)

This continues the removal of type parameters from CreateIndexRequest.mapping
methods started in elastic#50419. Here the removed methods are almost entirely in test
code, with the exception of a change to TransformIndex in the transform plugin.

Relates to elastic#41059
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >refactoring v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants