Skip to content

[DOCS] Document bytes and time params in cat API docs #47672

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 10, 2019
Merged

[DOCS] Document bytes and time params in cat API docs #47672

merged 4 commits into from
Oct 10, 2019

Conversation

@jrodewig
Copy link
Contributor

@jrodewig jrodewig commented Oct 7, 2019

Changes

  • Corrects the request convention for several cat APIs
  • Adds the bytes and time parameters to cat API docs and JSON specs

Closes #30202

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 7, 2019

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 7, 2019

Pinging @elastic/es-core-features (:Core/Features/CAT APIs)

@jrodewig
Copy link
Contributor Author

jrodewig commented Oct 7, 2019

@elasticmachine run elasticsearch-ci/2

1 similar comment
@jrodewig
Copy link
Contributor Author

jrodewig commented Oct 7, 2019

@elasticmachine run elasticsearch-ci/2

jrodewig
jrodewig commented Oct 8, 2019
View reviewed changes
docs/reference/rest-api/common-parms.asciidoc
A value of `-1` indicates {es} was unable to compute this number.
end::memory[]

tag::name[]
Copy link
Contributor Author

@jrodewig jrodewig Oct 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Replaced with https://github.com/elastic/elasticsearch/pull/47672/files#diff-38f70f3453c23ad1fda645b01d00773cL19

@jrodewig jrodewig requested a review from jakelandis October 8, 2019 00:21
@jakelandis
Copy link
Contributor

jakelandis commented Oct 8, 2019 •
edited
Loading

While the parameters are supported for the APIs, they may not influence the result. I think we should only add the documentation to the _cat/ endpoints that are influence by the parameter.

For example GET _cat/aliases?help shows that the following is supported:

alias          | a                | alias name           
index          | i,idx            | index alias points to
filter         | f,fi             | filter               
routing.index  | ri,routingIndex  | index routing        
routing.search | rs,routingSearch | search routing

These columns are not bytes or time and thus should not document or spec-api declare it's support. It won't error but that feels like an implementation detail/bug with the _cat APIs.

Could you update only those endpoints that will be influenced by the parameters ? I have included a full list below.

GET  _cat/aliases?help
GET _cat/allocation?help
GET _cat/count?help
GET _cat/fielddata?help
GET _cat/health?help
GET _cat/indices?help
GET _cat/master?help
GET _cat/nodeattrs?help
GET _cat/nodes?help
GET _cat/pending_tasks?help
GET _cat/plugins?help
GET _cat/recovery?help
GET _cat/repositories?help
GET _cat/segments?help
GET _cat/shards?help
GET _cat/snapshots?help
GET _cat/tasks?help
GET _cat/templates?help
GET _cat/thread_pool?help

@jakelandis
Copy link
Contributor

jakelandis commented Oct 8, 2019 •
edited
Loading

It won't error but that feels like an implementation detail/bug with the _cat APIs.

I logged #47755 to get better validation around these APIs.

@jrodewig
Copy link
Contributor Author

jrodewig commented Oct 9, 2019

Thanks for the feedback @jakelandis.

I scaled back the changes with 1904efc.
The bytes and time parameters are now documented only when it impacts the response.

@jrodewig
Copy link
Contributor Author

jrodewig commented Oct 9, 2019

@elasticmachine update branch

jakelandis
jakelandis reviewed Oct 9, 2019
View reviewed changes
docs/reference/cat/indices.asciidoc
include::{docdir}/rest-api/common-parms.asciidoc[tag=cat-s]

include::{docdir}/rest-api/common-parms.asciidoc[tag=time]

Copy link
Contributor

@jakelandis jakelandis Oct 9, 2019

Choose a reason for hiding this comment

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

I think this one should include bytes too

GET _cat/indices?h=ftt,ss&bytes=g&time=ms

Copy link
Contributor Author

@jrodewig jrodewig Oct 9, 2019

Choose a reason for hiding this comment

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

It looks like bytes is already documented for this API:
https://github.com/elastic/elasticsearch/blame/be233f3d235b39d9315fc79974e9b69f20b1fe38/docs/reference/cat/indices.asciidoc#L48

jakelandis
jakelandis reviewed Oct 9, 2019
View reviewed changes
docs/reference/cat/shards.asciidoc
include::{docdir}/rest-api/common-parms.asciidoc[tag=cat-s]

include::{docdir}/rest-api/common-parms.asciidoc[tag=time]

Copy link
Contributor

@jakelandis jakelandis Oct 9, 2019

Choose a reason for hiding this comment

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

i think this should also include bytes

GET _cat/shards?h=sm&bytes=b

Copy link
Contributor Author

@jrodewig jrodewig Oct 9, 2019

Choose a reason for hiding this comment

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

It looks like bytes is already documented for this API:
https://github.com/elastic/elasticsearch/blame/be233f3d235b39d9315fc79974e9b69f20b1fe38/docs/reference/cat/shards.asciidoc#L29

jakelandis
jakelandis approved these changes Oct 9, 2019
View reviewed changes
Copy link
Contributor

@jakelandis jakelandis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM (pending the two minor changes)

Thanks for doing this!

@jrodewig jrodewig merged commit 15e2de6 into elastic:master Oct 10, 2019
@jrodewig jrodewig deleted the doc-missing-cat-parms branch October 10, 2019 12:29
This was referenced Oct 10, 2019
Merged
Merged
Merged
jrodewig added a commit that referenced this pull request Oct 10, 2019
@jrodewig
[DOCS] Document bytes and time params in cat API docs (#47672) (#…
a8e0275 
…47854)
jrodewig added a commit that referenced this pull request Oct 10, 2019
@jrodewig
[DOCS] Document bytes and time params in cat API docs (#47672) (#…
57b6591 
…47855)
jrodewig added a commit that referenced this pull request Oct 10, 2019
@jrodewig
[DOCS] Document bytes and time params in cat API docs (#47672) (#…
04451d0 
…47856)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jakelandis jakelandis jakelandis approved these changes

Assignees

No one assigned

Labels

:Data Management/CAT APIs Text APIs behind /_cat >docs General docs changes v7.3.3 v7.4.1 v7.5.0 v8.0.0-alpha1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing params in rest-api-specs

3 participants

@jrodewig @elasticmachine @jakelandis