Expand docs for include_defaults
#5052
Conversation
This parameter is trappy, its notion of "default" includes the values from some arbitrary node's `elasticsearch.yml` file which means they aren't really defaults. But then again the real default values would be even less useful in the (common) case that some setting is overridden on all nodes.
|
Following you can find the validation changes against the target branch for the APIs. No changes detected. You can validate these APIs yourself by using the |
| * in the `elasticsearch.yml` file of one of the nodes in the cluster. If the nodes in your | ||
| * cluster do not all have the same values in their `elasticsearch.yml` config files then the | ||
| * values returned by this API may vary from invocation to invocation and may not reflect the | ||
| * values that Elasticsearch uses in all situations. Use the `GET _nodes/settings` API to |
There was a problem hiding this comment.
I'm having trouble finding a _nodes/settings API?
There was a problem hiding this comment.
Bizarrely it's the GET /_nodes/{nodeId} path from RestNodesInfoAction, see how we compute metricsOrNodeIds.
There was a problem hiding this comment.
Should we say _nodes/{nodeId}/info instead??
I tracked down a couple of commits that suggest GET _nodes/settings actually works as an endpoint: elastic/elasticsearch@f7db7eb (adds the code you mentioned, metricsOrNodeIds) and elastic/elasticsearch@ad50afb. I can't find any documentation around the API, though. There's technically a test RemoteClusterSecurityRestIT#testNodesInfo that uses GET _nodes/settings. I gather that GET /_nodes/settings will return GET /_nodes/_all/settings
I think the _nodes/* API lacks both documentation and testing -- like a user can't look up what GET /_nodes/settings does and find an answer. I'm good with committing this as is, but it'd be great to file a JIRA ticket to address testing and docs.
There was a problem hiding this comment.
GET _nodes/{nodeId}/info isn't a valid endpoint. Here are the nodes info API docs - particularly GET /_nodes/{metric}. We seem to have lost the list of valid metrics, but info isn't one of them.
There was a problem hiding this comment.
Ohhh, it's GET _nodes/{nodeId}/info/{metrics} -- I didn't have it quite right. Though not in that documentation you linked, anyway... 🤨
There was a problem hiding this comment.
What a mess :) but yeah GET _nodes/_all/info/settings is a longer (but more cohesive) way of getting the same output as GET _nodes/settings
|
Yeah this is all really old behaviour, and mostly in the REST layer where the test coverage is pretty light. There's some YAML tests that use this functionality but I don't see anything that really tests it very well. |
specification/cluster/get_settings/ClusterGetSettingsRequest.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Lgtm. The code was added long ago without adequate testing, and I can't find any public documentation -- I don't know where this documentation has gone elastic/elasticsearch@ad50afb#diff-2e623deac23060e38d0b2541701663524ed673e30d3a571e8bab1077511ce775R3-R12. It'd be good to file something as a follow up.
This parameter is trappy, its notion of "default" includes the values from some arbitrary node's `elasticsearch.yml` file which means they aren't really defaults. But then again the real default values would be even less useful in the (common) case that some setting is overridden on all nodes. Co-authored-by: Dianna Hohensee <[email protected]> (cherry picked from commit 4d0434b)
This parameter is trappy, its notion of "default" includes the values from some arbitrary node's `elasticsearch.yml` file which means they aren't really defaults. But then again the real default values would be even less useful in the (common) case that some setting is overridden on all nodes. Co-authored-by: Dianna Hohensee <[email protected]> (cherry picked from commit 4d0434b)
This parameter is trappy, its notion of "default" includes the values from some arbitrary node's `elasticsearch.yml` file which means they aren't really defaults. But then again the real default values would be even less useful in the (common) case that some setting is overridden on all nodes. (cherry picked from commit 4d0434b) Co-authored-by: David Turner <[email protected]> Co-authored-by: Dianna Hohensee <[email protected]> Co-authored-by: Quentin Pradet <[email protected]>
This parameter is trappy, its notion of "default" includes the values from some arbitrary node's `elasticsearch.yml` file which means they aren't really defaults. But then again the real default values would be even less useful in the (common) case that some setting is overridden on all nodes. (cherry picked from commit 4d0434b) Co-authored-by: David Turner <[email protected]> Co-authored-by: Dianna Hohensee <[email protected]> Co-authored-by: Quentin Pradet <[email protected]>
This parameter is trappy, its notion of "default" includes the values
from some arbitrary node's
elasticsearch.ymlfile which means theyaren't really defaults. But then again the real default values would be
even less useful in the (common) case that some setting is overridden on
all nodes.