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 include_type_name parameter from REST layer #48632

Merged
merged 14 commits into from
Nov 1, 2019

Conversation

romseygeek
Copy link
Contributor

This commit removes support for the include_type_name parameter from all
REST actions that receive or return mapping configurations.

Relates to #41059

@romseygeek romseygeek added >breaking :Search Foundations/Mapping Index mappings, including merging and defining field types :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 labels Oct 29, 2019
@romseygeek romseygeek self-assigned this Oct 29, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Indices APIs)

Map<String, Object> mappings = parser.map();
if (MapperService.isMappingSourceTyped(MapperService.SINGLE_MAPPING_NAME, mappings)) {
throw new IllegalArgumentException("The mapping definition cannot be nested under a type " +
"[" + MapperService.SINGLE_MAPPING_NAME + "] unless include_type_name is set to true.");
Copy link
Contributor

Choose a reason for hiding this comment

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

As we don't have include_type_name parameter anymore, I think the error message should just say: "The mapping definition cannot be nested under a type."

Copy link
Contributor

@mayya-sharipova mayya-sharipova left a comment

Choose a reason for hiding this comment

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

@romseygeek, this PR LGTM.
I have just a small comment about the error message, which can also be addressed in next PRs of type removals.

if (includeTypeName
|| source.containsKey("mappings") == false
static Map<String, Object> prepareMappings(Map<String, Object> source) {
if (source.containsKey("mappings") == false
Copy link
Contributor

Choose a reason for hiding this comment

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

here also on the line 80, may be also worth correcting the error message to have only "The mapping definition cannot be nested under a type"

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek
Copy link
Contributor Author

@elasticmachine update branch

@romseygeek romseygeek merged commit 37a997a into elastic:master Nov 1, 2019
@romseygeek romseygeek deleted the types-removal/include-type-name branch November 1, 2019 09:30
@jpountz jpountz mentioned this pull request Nov 1, 2019
66 tasks
jtibshirani added a commit that referenced this pull request Nov 12, 2019
In #48632 we removed support for `include_type_name` in the REST API. This PR
removes it from the API spec and documentation.
@pgomulka pgomulka mentioned this pull request Mar 25, 2020
66 tasks
pgomulka added a commit that referenced this pull request Apr 19, 2021
This commit allows to use the include_type_name parameter with the compatible rest api.
The support for include_type_name was previously removed in #48632

relates #51816
types removal meta issue #54160
jrodewig added a commit that referenced this pull request Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>breaking :Data Management/Indices APIs APIs to create and manage indices and templates :Search Foundations/Mapping Index mappings, including merging and defining field types v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants