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 types from Get/MultiGet #46587

Merged
merged 16 commits into from
Sep 20, 2019
Merged

Conversation

romseygeek
Copy link
Contributor

This commit removes types from the ShardGetService, and propagates this API change
up through the Transport and Rest actions for Get and MultiGet

Relates to #41059

@romseygeek romseygeek added >breaking :Search Foundations/Mapping Index mappings, including merging and defining field types >breaking-java >refactoring v8.0.0 labels Sep 11, 2019
@romseygeek romseygeek self-assigned this Sep 11, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search

Copy link
Contributor

@jimczi jimczi left a comment

Choose a reason for hiding this comment

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

LGTM

@alpar-t
Copy link
Contributor

alpar-t commented Sep 12, 2019

@elasticmachine test this please

1 similar comment
@romseygeek
Copy link
Contributor Author

@elasticmachine test this please

@romseygeek
Copy link
Contributor Author

I've chased down a bunch of test failures here due to a change in MultiGetRequestBuilder that doesn't get caught by the compiler. The #add(String index, String type, String id) method is removed, but calling code that uses this signature is silently redirected to the #add(String index, String... ids) method, with the result that the old type parameter gets repurposed as a String id.

This could introduce some quite nasty and hard-to-track-down bugs into client code over an upgrade. I think the best solution is probably to rename the add() methods that take doc ids to addIds(), which will at least force a recompilation in client code

@romseygeek romseygeek merged commit 7c90801 into elastic:master Sep 20, 2019
@jpountz jpountz mentioned this pull request Sep 20, 2019
66 tasks
@pgomulka pgomulka mentioned this pull request Mar 25, 2020
66 tasks
pgomulka added a commit that referenced this pull request Mar 23, 2021
The types removal effort has removed the type from Index API in #47671 and from Get API in #46587
This commit allows to use 'typed' endpoints for the both Index and Get APIs

relates compatible types-removal meta issue #54160
pgomulka added a commit that referenced this pull request Jun 9, 2021
Retrofits typed api for M-get api removed in #46587

relates #51816
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Jun 9, 2021
pgomulka added a commit that referenced this pull request Jun 14, 2021
retrofits typed get_source api removed in #46931 and #46587
relates #51816
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants