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

Deprecate the update_all_types option. #28284

Merged
merged 3 commits into from
Jan 24, 2018

Conversation

jpountz
Copy link
Contributor

@jpountz jpountz commented Jan 18, 2018

This option makes no sense on indices that have only one type, which is
enforced since 6.0.

@jpountz jpountz added :Search Foundations/Mapping Index mappings, including merging and defining field types >deprecation v7.0.0 v6.3.0 labels Jan 18, 2018
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@jpountz I left a question, don't know if you thought about that option and rejected it for some reason or it has other disadvantages, but would like to hear your thoughts.

@@ -49,6 +54,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
if (request.hasContent()) {
createIndexRequest.source(request.content(), request.getXContentType());
}
if (request.hasParam("update_all_types")) {
DEPRECATION_LOGGER.deprecated("[update_all_types] is deprecated since indices may not have more than one type anymore");
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would make sense to put this in createIndexRequest#updateAllTypes, so we also log warnings for Java API users, wdyt?

@@ -70,6 +75,9 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
PutMappingRequest putMappingRequest = putMappingRequest(Strings.splitStringByCommaToArray(request.param("index")));
putMappingRequest.type(request.param("type"));
putMappingRequest.source(request.requiredContent(), request.getXContentType());
if (request.hasParam("update_all_types")) {
DEPRECATION_LOGGER.deprecated("[update_all_types] is deprecated since indices may not have more than one type anymore");
Copy link
Member

Choose a reason for hiding this comment

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

Same here but for putMappingRequest#updateAllTypes.

@jpountz
Copy link
Contributor Author

jpountz commented Jan 18, 2018

Hmm I had never thought of using the deprecation logger for client-side warnings, I'm not sure what I think about it. However I'd be happy to mark the Java methods as @Deprecated.

@cbuescher
Copy link
Member

@jpountz sorry, of course I forgot about the logging infra not being available there. Deprecating the methods makes sense to me though.

This option makes no sense on indices that have only one type, which is
enforced since 6.0.
@jpountz jpountz removed the v7.0.0 label Jan 24, 2018
@jpountz jpountz merged commit d9e9094 into elastic:6.x Jan 24, 2018
@jpountz jpountz deleted the deprecate/update_all_types branch January 24, 2018 15:12
martijnvg added a commit that referenced this pull request Jan 25, 2018
* es/6.x:
  [Docs] Fix explanation for `from` and `size` example (#28320)
  Adapt bwc version after backport #28358
  Always return the after_key in composite aggregation response (#28358)
  Adds a note in the `terms` aggregation docs regarding pagination (#28360)
  Update packaging tests to work with meta plugins (#28336)
  Remove Painless Type from MethodWriter in favor of Java Class. (#28346)
  [Doc] Fixs typo in reverse-nested-aggregation.asciidoc (#28348)
  [Docs] Fixed Indices information breaking changes (#27914)
  Reindex: Shore up rethrottle test
  isHeldByCurrentThread should return primitive bool
  [Docs] Clarify `html` encoder in highlighting.asciidoc (#27766)
  Only assert single commit iff index created on 6.2
  Deprecate the `update_all_types` option. (#28284)
  Fix GeoDistance query example
  Settings: Introduce settings updater for a list of settings (#28338)
  [Docs] Fix asciidoc style in composite agg docs
  Adapt bwc version after backport #28310
  Adds the ability to specify a format on composite date_histogram source (#28310)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>deprecation :Search Foundations/Mapping Index mappings, including merging and defining field types v6.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants