Removes typed endpoint from search and related APIs by colings86 · Pull Request #41640 · elastic/elasticsearch

colings86 · 2019-04-29T13:37:24Z

No description provided.

elasticmachine · 2019-04-29T13:37:26Z

Pinging @elastic/es-search

colings86 · 2019-04-30T13:46:27Z

@elasticmachine run elasticsearch-ci/2

colings86 · 2019-04-30T14:45:11Z

@elasticmachine run elasticsearch-ci/2

colings86 · 2019-04-30T15:46:12Z

@elasticmachine run elasticsearch-ci/1

colings86 · 2019-04-30T17:34:23Z

@elasticmachine run elasticsearch-ci/2

jtibshirani

Just filling in on types-related reviews as @jpountz is out. It is great to see +72 -1,104 lines!

I left one comment on the code and had a couple thoughts:

Since we're removing support for the typed endpoints, using types in certain Java HLRC calls will now result in failures. Are we planning to remove types in HLRC requests/ request conversion code in a future PR, or would it make sense to also cover it here to avoid an 'in-between' state?
There are a few more search-related areas where types have been deprecated, such as searches on the _type field and the use of types in 'lookup' queries like terms lookup and percolator. If this PR is already getting fairly large, perhaps we could just make sure to track them on the meta-issue.

jtibshirani · 2019-05-01T00:02:06Z

docs/reference/migration/migrate_8_0/reindex.asciidoc

I wonder if the phrasing 'indexes can now only contain a single unnamed type' could be confusing. We now provide 'typeless' index creation APIs, which to users seem like they don't introduce a type at all. Perhaps we could just say 'Since indexes no longer contain types, these typed endpoints are obsolete.'

Also small typo: obselete -> obsolete.

++ good catch, I think your wording is much better. I'll change it in this and the other PR

colings86 · 2019-05-01T08:12:34Z

@elasticmachine run elasticsearch-ci/2

colings86 · 2019-05-01T08:21:21Z

Thanks for the review @jtibshirani

Since we're removing support for the typed endpoints, using types in certain Java HLRC calls will now result in failures. Are we planning to remove types in HLRC requests/ request conversion code in a future PR, or would it make sense to also cover it here to avoid an 'in-between' state?

I had a look at doing this but unless I'm mistaken SearchRequest is one of the remaining classes shared between the HLRC and the server code? So removing types from the SearchRequest very quickly gets into removing types from the search execution in general which is a bigger fix that IMO is suited to solving in its own PR. I added a MUST to #41059 to remove types from the Search Request and I'm happy to tackle this one straight after this PR is merged to avoid the "in between" state being around for very long which I think is ok since the "in between" state will only exist on master so there is a low risk of releasing in this state. Does this sound ok to you or do you think we really need to tackle these together?

There are a few more search-related areas where types have been deprecated, such as searches on the _type field and the use of types in 'lookup' queries like terms lookup and percolator. If this PR is already getting fairly large, perhaps we could just make sure to track them on the meta-issue.

I've added a check-box for this in #41059. I would like to handle this in a separate change to avoid this PR becoming too big

colings86 · 2019-05-01T11:57:54Z

@elasticmachine run elasticsearch-ci/1

jtibshirani

I'm happy to tackle this one straight after this PR is merged to avoid the "in between" state being around for very long

Tackling it in the next PR sounds good to me, thanks! I agree there's not a perfect split into PRs because we share the server and HLRC classes in some places.

jtibshirani · 2019-05-01T17:35:38Z

docs/reference/migration/migrate_8_0/search.asciidoc

Accidental double period here (and below).

colings86 · 2019-05-09T08:39:33Z

@elasticmachine run elasticsearch-ci/docbldesx

colings86 · 2019-05-09T09:31:38Z

@elasticmachine run elasticsearch-ci/1

colings86 · 2019-05-09T11:51:59Z

@elasticmachine run elasticsearch-ci/packaging-sample

)

…PIs (#54572) This adds a compatible APIs for typed endpoints removed in #41640 202 failing tests These 2 tests are explicitly fixed by this PR CompatRestIT. test {yaml=mtermvectors/21_deprecated_with_types/Deprecated camel case and _ parameters should fail in Term Vectors query} CompatRestIT. test {yaml=mtermvectors/30_mix_typeless_typeful/mtermvectors without types on an index that has types} I think more yml tests should be added to 7.x to cover these endpoints 17th june 1306tests | 197failures | 16ignored | 10m56.91sduration

…ints (#72155) Implements a V7 compatible typed endpoints for REST for search related apis retrofits the REST layer change removed in #41640 relates main meta issue #51816 relates types removal issue #54160

colings86 added >breaking :Search/Search Search-related issues that do not fall into other categories v8.0.0 labels Apr 29, 2019

colings86 self-assigned this Apr 29, 2019

jpountz mentioned this pull request Apr 29, 2019

Types removal in 8.0 #41059

Closed

66 tasks

colings86 marked this pull request as ready for review April 30, 2019 07:03

colings86 requested a review from jpountz April 30, 2019 07:03

colings86 mentioned this pull request Apr 30, 2019

Removes tpyed endpoints for msearch and mtermvector APIs #41674

Closed

jtibshirani reviewed May 1, 2019

View reviewed changes

jtibshirani approved these changes May 1, 2019

View reviewed changes

docs/reference/migration/migrate_8_0/search.asciidoc Outdated

Copy link
Copy Markdown

Contributor

jtibshirani May 1, 2019

Choose a reason for hiding this comment

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

Accidental double period here (and below).

colings86 added 2 commits May 9, 2019 09:29

Removes typed endpoint from search and related APIs

08c48ec

Removes tpyed endpoints for msearch and mtermvector APIs

29a8e38

colings86 merged commit 3647d7c into elastic:master May 9, 2019

colings86 deleted the types-removal/search-url branch May 9, 2019 13:41

gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019

Removes typed REST endpoints from search and related APIs (elastic#41640

8ccfc12

)

pgomulka mentioned this pull request Mar 25, 2020

Types removal - compatible changes #54160

Closed

66 tasks

pgomulka mentioned this pull request Apr 1, 2020

Compatible logic for Removes typed endpoint from search and related APIs #54572

Merged

jakelandis mentioned this pull request Feb 22, 2021

DRAFT [META] REST Compatible API V7 completeness #68905

Closed

pgomulka mentioned this pull request May 10, 2021

[Rest Api Compatibility] Typed endpoints for search and related endpoints #72155

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

hkulekci mentioned this pull request Feb 17, 2022

Elasticsearch 8.0 support hkulekci/ElasticsearchDSL#2

Merged

23 tasks

Conversation

colings86 commented Apr 29, 2019

Uh oh!

elasticmachine commented Apr 29, 2019

Uh oh!

colings86 commented Apr 30, 2019

Uh oh!

colings86 commented Apr 30, 2019

Uh oh!

colings86 commented Apr 30, 2019

Uh oh!

colings86 commented Apr 30, 2019

Uh oh!

jtibshirani left a comment

Choose a reason for hiding this comment

Uh oh!

jtibshirani May 1, 2019

Choose a reason for hiding this comment

Uh oh!

colings86 May 1, 2019

Choose a reason for hiding this comment

Uh oh!

colings86 commented May 1, 2019

Uh oh!

colings86 commented May 1, 2019

Uh oh!

colings86 commented May 1, 2019

Uh oh!

jtibshirani left a comment

Choose a reason for hiding this comment

Uh oh!

jtibshirani May 1, 2019

Choose a reason for hiding this comment

Uh oh!

colings86 commented May 9, 2019

Uh oh!

colings86 commented May 9, 2019

Uh oh!

colings86 commented May 9, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants