Skip to content

Fix the "slices" parameter for the Delete By Query API in the REST specification #51535

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

Merged
merged 1 commit into from
Feb 2, 2020

Conversation

karmi
Copy link
Contributor

@karmi karmi commented Jan 28, 2020

This patch updates the type parameter in the Delete By Query API: according to the documentation, it can be set to "auto", but the type in the documentation allows only numerical values.

This prevents people from setting the parameter to "auto" eg. in the Go client, which generates source from the specification, and sets the corresponding Go type as number.

The patch uses the | notation, which we have discussed previously for encoding a "polymorphic" parameter like this.

Related: elastic/go-elasticsearch#77

/cc @elastic/es-clients

…ecification

This patch updates the `type` parameter in the Delete By Query API: according to
[the documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#docs-delete-by-query-slice), it can be set to "auto",
but the type in the documentation allows only numerical values.

This prevents people from setting the parameter to "auto" eg. in the Go client,
which generates source from the specification, and sets the corresponding Go
type as number.

The patch uses the `|` notation, which we have discussed previously for encoding
a "polymorphic" parameter like this.

Related: elastic/go-elasticsearch#77
@cbuescher cbuescher added the :Core/Infra/REST API REST infrastructure and utilities label Jan 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/REST API)

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

Makes sense to me so long as the clients team is happy with it and jenkins doesn't mind.

@karmi karmi merged commit c1d9966 into master Feb 2, 2020
karmi added a commit that referenced this pull request Feb 2, 2020
…ecification (#51535)

This patch updates the `type` parameter in the Delete By Query API: according to
[the documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#docs-delete-by-query-slice),
it can be set to "auto", but the type in the documentation allows only numerical values.

This prevents people from setting the parameter to "auto" eg. in the Go client,
which generates source from the specification, and sets the corresponding Go
type as number.

The patch uses the `|` notation, which we have discussed previously for encoding
a "polymorphic" parameter like this.

Related: elastic/go-elasticsearch#77
karmi added a commit that referenced this pull request Feb 2, 2020
* REST: Test: Fix the `accept_enterprise` parameter for Get License API (#51527)

The Get License API specifies the `accept_enterprise` parameter as a `boolean`:

https://github.com/elastic/elasticsearch/blob/0ca5cb8cb636a4be9c36b8e38e565af66abc423b/x-pack/plugin/src/test/resources/rest-api-spec/api/license.get.json#L22-L27

In the test, a `string` is passed however, which makes the test compilation fail in the Go client.

(cherry picked from commit e2a2169)

* Fix the SQL API documentation in REST specification (#51534)

This patch fixes the SQL REST API documentation to conform to the current schema.

(cherry picked from commit c8b6a84)

* Fix the "slices" parameter for the Delete By Query API in the REST specification (#51535)

This patch updates the `type` parameter in the Delete By Query API: according to
[the documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/docs-delete-by-query.html#docs-delete-by-query-slice),
it can be set to "auto", but the type in the documentation allows only numerical values.

This prevents people from setting the parameter to "auto" eg. in the Go client,
which generates source from the specification, and sets the corresponding Go
type as number.

The patch uses the `|` notation, which we have discussed previously for encoding
a "polymorphic" parameter like this.

Related: elastic/go-elasticsearch#77

* Fix the Enrich API documentation in REST specification (#51528)

This patch fixes the REST API documentation for the Enrich APIs to conform to the current schema.

(cherry picked from commit 59f28f4)
@karmi karmi deleted the rest_spec_fix_slices branch February 2, 2020 14:34
karmi added a commit that referenced this pull request Feb 2, 2020
…on (#51792)

The previous patch in c1d9966 incorrectly set the `type` to `number|auto`,
which is incorrect — the "polymorphic" type, denoted with the `|` sign,
should contain only other types, ie. number, string, bool, etc.

Fixes #51535
karmi added a commit that referenced this pull request Feb 2, 2020
…on (#51792)

The previous patch in c1d9966 incorrectly set the `type` to `number|auto`,
which is incorrect — the "polymorphic" type, denoted with the `|` sign,
should contain only other types, ie. number, string, bool, etc.

Fixes #51535

(cherry picked from commit 68db7fc)
karmi added a commit that referenced this pull request Feb 2, 2020
…on (#51792) (#51793)

The previous patch in c1d9966 incorrectly set the `type` to `number|auto`,
which is incorrect — the "polymorphic" type, denoted with the `|` sign,
should contain only other types, ie. number, string, bool, etc.

Fixes #51535

(cherry picked from commit 68db7fc)
@russcam
Copy link
Contributor

russcam commented Feb 3, 2020

should the type for "auto" be string, rather than auto? I understand that type string is general, but using auto to me implies it's a type rather than a specific value for the param. Should the documentation also be updated to indicate what auto is?

@karmi
Copy link
Contributor Author

karmi commented Feb 3, 2020

Yes, absolutely, that was a mistake I did, it's already fixed via https://github.com/elastic/elasticsearch/pull/51792/files.

karmi added a commit that referenced this pull request Feb 5, 2020
… specification

This patch supplements #51792 and #51535 where the type of the "slices" parameter has been fixed.
karmi added a commit that referenced this pull request Feb 5, 2020
… specification (#51908)

This patch supplements #51792 and #51535 where the type of the "slices" parameter has been fixed.
karmi added a commit that referenced this pull request Feb 5, 2020
… specification (#51908)

This patch supplements #51792 and #51535 where the type of the "slices" parameter has been fixed.

(cherry picked from commit 2ed9e95)
karmi added a commit that referenced this pull request Feb 5, 2020
… specification (#51908) (#51911)

This patch supplements #51792 and #51535 where the type of the "slices" parameter has been fixed.

(cherry picked from commit 2ed9e95)
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

Successfully merging this pull request may close these issues.

5 participants