[Rest Api Compatibility] Typed endpoints for search and related endpoints #72155
Conversation
pgomulka changed the title pgomulka added :Core/Infra/REST API 
elasticmachine added Team:Core/Infra 
| Pinging @elastic/es-core-infra (Team:Core/Infra) |
| Pinging @elastic/es-search (Team:Search) |
romseygeek left a comment
romseygeek left a comment There was a problem hiding this comment.
LGTM from a search perspective.
| NamedWriteableRegistry namedWriteableRegistry, | ||
| IntConsumer setSize) throws IOException { | ||
| if (request.getRestApiVersion() == RestApiVersion.V_7 && | ||
| (request.hasParam(INCLUDE_TYPE_NAME_PARAMETER) || request.hasParam("type"))) { |
There was a problem hiding this comment.
Does include_type_name actually have an effect here?
There was a problem hiding this comment.
good point, it is not needed. I mistakenly assumed that it is needed as other APIs so far were expecting it.
I removed it
| //possibly this needs a separate issue to track the problem | ||
| // if we emit a compatible warning when a type parameter (?type=some_type) is present, | ||
| // then for APIs where a {type} is used a compatible warning would be emitted twice | ||
| // this is because we cannot distinguish between them and for {type} we already emit a warning when using a Route |
There was a problem hiding this comment.
I think this is fine? If you specify the type in two places you get two warnings.
There was a problem hiding this comment.
I probably did not made this clear. The problem is that you would get two warnings when specifying the type in {type} as in path.
The problem only occurs when an expected paramter ?type has the same name as the parameter in braces {type}
enabeling the tests and adds a type field for termvector response the commit that enabled typed enpoints but missed to update the response elastic#72155
Enabling the tests and adds a type field for termvector response the commit that enabled typed endpoints but missed to update the response #72155
4 participants
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