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

Incorrect REST API spec? #27681

Closed
codebrain opened this issue Dec 5, 2017 · 9 comments
Closed

Incorrect REST API spec? #27681

codebrain opened this issue Dec 5, 2017 · 9 comments
Labels
:Core/Infra/REST API REST infrastructure and utilities

Comments

@codebrain
Copy link
Contributor

Looking at the breaking changes for the .NET client. I believe I have found an issue.

The documentation for 6.0 indicates that the ignore_unavailable and allow_no_indices parameters on the indices.exists call is no longer accepted: https://www.elastic.co/guide/en/elasticsearch/reference/current/breaking_60_rest_changes.html#_indices_exists_api

however

The REST spec (https://github.com/elastic/elasticsearch/blob/6.x/rest-api-spec/src/main/resources/rest-api-spec/api/indices.exists.json) still contains them.

@javanna
Copy link
Member

javanna commented Dec 5, 2017

good catch. they were properly removed as part of #20712. I'm afraid they were added back later (see e579629) which kind of reverted the original change. I think that as a followup, the breaking change should have been removed from the migrate guide.

@codebrain
Copy link
Contributor Author

So... the parameters are still valid, the REST spec is correct and the documentation for the Breaking Changes is incorrect?

@tvernum
Copy link
Contributor

tvernum commented Dec 6, 2017

@javanna I'm confused - I would interpret your comments the same way @codebrain did, but that doesn't match the code.

The change to the Java request object (from #20712) still stands in v6.0.0 but it seems that the change to the REST spec was accidentally reverted.

So, I believe the answer is those parameters are not valid, the rest spec is wrong and the breaking changes doc is correct.

@javanna
Copy link
Member

javanna commented Dec 6, 2017

@tvernum the changes to the java request objects are still valid but they are only exposed to the transport client... if you hit HEAD /index it will go to RestGetIndicesAction, which doesn't use IndicesExistsRequest anymore. This was changed as part of the effort to make HEAD requests spec compliant, so that they behave the same as their corresponding GET request (accept same parameters, return same content-length) but they just don't return a response body.

This could be cleaned up further, for instance TransportIndicesExistsAction is currently only used by the transport client which is odd and could be removed. Also, IndicesExistsRequest is leveraged in #27384 , but maybe we should rather use GetIndexRequest there as the API accepts the same parameters.

@tvernum
Copy link
Contributor

tvernum commented Dec 6, 2017

Ah, that makes much more sense. I should have looked at Jason's change more closely.

@danielmitterdorfer danielmitterdorfer added the :Core/Infra/REST API REST infrastructure and utilities label Dec 7, 2017
@codebrain
Copy link
Contributor Author

So... I can ignore the documentation and leave the parameters where they are?

@tvernum
Copy link
Contributor

tvernum commented Dec 20, 2017

@codebrain Yes, the parameters still exist.

GET /my-index

and

HEAD /my-index

allow exactly the same parameters, which includes ignore_unavailable and allow_no_indices .

The breaking changes doc is incorrect (out of date)

@codebrain
Copy link
Contributor Author

OK, I will submit a PR for the breaking changes documentation.

codebrain pushed a commit that referenced this issue Dec 20, 2017
@nik9000
Copy link
Member

nik9000 commented Mar 12, 2018

This was fixed by #27915.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/REST API REST infrastructure and utilities
Projects
None yet
Development

No branches or pull requests

5 participants