-
Notifications
You must be signed in to change notification settings - Fork 105
cluster/allocation/explain
Query Parameters
#4939
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
Conversation
Extends the documentation for the `cluster/allocation/explain` API after query parameter support was added in elastic/elasticsearch#129342
Following you can find the validation changes against the target branch for the API.
You can validate this API yourself by using the |
/** | ||
* If true, returns an explanation for the primary shard for the specified shard ID. | ||
*/ | ||
primary?: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validation currently fails because you have YAML tests where primary
can be a string. Is it really needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the ElasticSearch API, it is documented that we facilitate strings to be passed for boolean values. I was adding YAML Rest Tests to ensure that if a user is to do so, we correctly parse this and proceed as expected.
A separate question, why is validation here subjected to whether or not a YAML rest test exists / does not exist? What changes if I was to delete the test?
Looking more closely, we have YAML tests allowing different types in multiple places. Here is the full list of changes needed to make the validation pass. This won't give a good developer experience. I don't recommend applying this diff unless there's a good reason. diff --git a/specification/cluster/allocation_explain/ClusterAllocationExplainRequest.ts b/specification/cluster/allocation_explain/ClusterAllocationExplainRequest.ts
index 50cda8f21..5bffd3ce4 100644
--- a/specification/cluster/allocation_explain/ClusterAllocationExplainRequest.ts
+++ b/specification/cluster/allocation_explain/ClusterAllocationExplainRequest.ts
@@ -48,15 +48,15 @@ export interface Request extends RequestBase {
/**
* The name of the index that you would like an explanation for.
*/
- index?: IndexName
+ index?: IndexName | integer
/**
* An identifier for the shard that you would like an explanation for.
*/
- shard?: integer
+ shard?: integer | string
/**
* If true, returns an explanation for the primary shard for the specified shard ID.
*/
- primary?: boolean
+ primary?: boolean | string
/**
* Explain a shard only if it is currently located on the specified node name or node ID.
*/
@@ -65,12 +65,12 @@ export interface Request extends RequestBase {
* If true, returns information about disk usage and shard sizes.
* @server_default false
*/
- include_disk_info?: boolean
+ include_disk_info?: boolean | string
/**
* If true, returns YES decisions in explanation.
* @server_default false
*/
- include_yes_decisions?: boolean
+ include_yes_decisions?: boolean | string
/**
* Period to wait for a connection to the master node.
* @server_default 30s
@@ -85,11 +85,11 @@ export interface Request extends RequestBase {
/**
* An identifier for the shard that you would like an explanation for.
*/
- shard?: integer
+ shard?: integer | string
/**
* If true, returns an explanation for the primary shard for the specified shard ID.
*/
- primary?: boolean
+ primary?: boolean | string
/**
* Explain a shard only if it is currently located on the specified node name or node ID.
*/
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM. Validation fails but this will be fixed by removing some YAML tests from Elasticsearch.
Change Details
Extends the documentation for the
cluster/allocation/explain
API after query parameter support was added in elastic/elasticsearch#129342Release
To be merged after elastic/elasticsearch#129342
Github Issue - elastic/elasticsearch#127028
Jira Ticket - ES-11865