Skip to content

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

Merged
romseygeek merged 4 commits intoelastic:masterfrom
romseygeek:types-removal/create-index-request
Jan 2, 2020
Merged

Remove CreateIndexRequest.addMapping(type, string, xcontenttype)#50419
romseygeek merged 4 commits intoelastic:masterfrom
romseygeek:types-removal/create-index-request

Conversation

@romseygeek
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

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

@romseygeek
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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