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

Only issue a deprecation warning if include_type_name is not set. #38825

Merged
merged 1 commit into from
Feb 13, 2019

Conversation

jtibshirani
Copy link
Contributor

We expect many users to have a custom document type in 6.x, for example they
might have used doc when creating indices. In this case, users cannot easily
switch over to typeless index creations in 6.7 using include_type_name=false,
because the rest of their document CRUD calls will refer to the custom type.
Instead, we are recommending that users take the following steps: set
include_type_name=true in 6.7 for all relevant calls, upgrade to 7.0, then
switch over completely to the typeless APIs and stop using include_type_name.

This means that it will be very common to set include_type_name=true in 6.7,
so it is misleading to emit a deprecation warning to tell the user to switch to
using include_type_name=false. This PR switches to emitting a deprecation
warning only if include_type_name is not set at all. The warning serves as an
important note to users that the request and response format of these APIs will
change in a breaking way in 7.0.

Relates to #35190.

We expect many users to have a custom document type in 6.x, for example they
might have used `doc` when creating indices. In this case, users cannot easily
switch over to typeless index creations in 6.7 using `include_type_name=false`,
because the rest of their document CRUD calls will refer to the custom type.
Instead, we are recommending that users take the following steps: set
`include_type_name=true` in 6.7 for all relevant calls, upgrade to 7.0, then
switch over completely to the typeless APIs and stop using `include_type_name`.

This means that it will be very common to set `include_type_name=true` in 6.7,
so it is misleading to emit a deprecation warning to tell the user to switch to
using `include_type_name=false`. This PR switches to emitting a deprecation
warning only if `include_type_name` is not set at all. The warning serves as an
important note to users that the request and response format of these APIs will
change in a breaking way in 7.0.
@jtibshirani jtibshirani added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v6.7.0 labels Feb 12, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

@jtibshirani
Copy link
Contributor Author

This PR came out of a discussion with @bleskes and @jpountz about the upgrade path for users with a custom type name.

Once this is settled, I'm planning to update the 'removal of types' documentation in a separate PR against master, and also the relevant internal documents.

@jtibshirani jtibshirani merged commit 829cb03 into elastic:6.7 Feb 13, 2019
@jtibshirani jtibshirani deleted the include-type-name-warnings branch February 13, 2019 21:13
jtibshirani added a commit to jtibshirani/elasticsearch that referenced this pull request Feb 13, 2019
Follow-up to elastic#38825, where we made a tweak to the deprecation behavior.
"mapping requests will change in 7.0. Please start using the include_type_name parameter set to false " +
"to move to the new, typeless response format that will become the default.";
static final String TYPES_DEPRECATION_MESSAGE = "[types removal] The parameter include_type_name " +
"should be explicitly specified in get index requests to prepare for 7.0. In 7.0 include_type_name " +
Copy link
Contributor

Choose a reason for hiding this comment

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

get index requests
@jtibshirani should it be "get field mapping requests"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @mayya-sharipova, will fix. I somehow make this mistake every time...

jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Feb 13, 2019
Follow-up to #38825, where we made a tweak to the deprecation behavior.
jtibshirani added a commit that referenced this pull request Jun 30, 2020
In #38825, we switched from issuing a types warning if `include_type_name` is
missing or `true`, to only issuing a warning when `include_type_name` is
missing. However we missed the 'put mapping' API in that PR.

Addresses #58675.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v6.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants