Skip to content

[Docs] Clarify default value for allow_no_indices #52635

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 4 commits into from
Feb 24, 2020

Conversation

matriv
Copy link
Contributor

@matriv matriv commented Feb 21, 2020

Add default value to each one of the usages of allow_no_indices
since it differs between different APIs.

Relates to: #52534

Add a cliarification that default value is `true`, so no error
is returned if no index is matched.

Fixes: elastic#52534
@matriv matriv added >docs General docs changes :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 v6.8.7 v7.7.0 v7.6.1 labels Feb 21, 2020
@elasticmachine
Copy link
Collaborator

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

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@@ -42,6 +42,7 @@ tag::allow-no-indices[]
the request does *not* return an error
if a wildcard expression
or `_all` value retrieves only missing or closed indices.
Defaults to `true`.
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid that the default value here depends on the API, or is it true for all APIs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried some of them like HEAD, _mapping, _forgemerge and they behave the same.

Copy link
Member

Choose a reason for hiding this comment

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

The default is API dependant. For example the put mapping api defaults to false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, should we just add a sentence that the default value depends on the API instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can add the default value where the definition is included instead:

include::{docdir}/rest-api/common-parms.asciidoc[tag=allow-no-indices]
+
Defaults to `true`.

Copy link
Member

Choose a reason for hiding this comment

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

@matriv
Copy link
Contributor Author

matriv commented Feb 21, 2020

Checked all usages and added the default value to the doc references.

I have another question though, if you check this: https://github.com/elastic/elasticsearch/pull/52635/files#diff-2e0eb1b85aa4eba2d58663601327caffL39
I removed the reference since the value of the allow_no_indices has not effect.
You get 404 if the the index is not there regardless of the param.

I'm wondering if we should do the same for the index exists api (HEAD /<index-pattern>).
What do you think?

Comment on lines -39 to -40
include::{docdir}/rest-api/common-parms.asciidoc[tag=allow-no-indices]

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% sure.
The mechanism is different, but the outcome the same (status 200 or 404).
If allow_no_indices=false and no index is matched then we get an exception at the index name resolution phase which is translated to 404.
If allow_no_indices=true and no index is matched then no aliases are found and this is again translated to 404.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should probably remove documentation for this parameter. It's not useful to the end user and the outcome will be the same regardless. Thanks for catching this @matriv.

@matriv
Copy link
Contributor Author

matriv commented Feb 21, 2020

@jrodewig FYI, I added also this: 9902b34#diff-72be49b35e089e3c18cadc2619d1d933R99

@matriv matriv requested a review from jrodewig February 21, 2020 19:23
Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @matriv!

@matriv matriv merged commit 2eb9864 into elastic:master Feb 24, 2020
@matriv matriv deleted the fix-52534 branch February 24, 2020 10:37
matriv added a commit to matriv/elasticsearch that referenced this pull request Feb 24, 2020
Add default value to each one of the usages of `allow_no_indices`
since it differs between different APIs.

Relates to: elastic#52534

(cherry picked from commit 2eb9864)
@matriv matriv removed the v6.8.7 label Feb 24, 2020
matriv added a commit that referenced this pull request Feb 24, 2020
Add default value to each one of the usages of `allow_no_indices`
since it differs between different APIs.

Relates to: #52534

(cherry picked from commit 2eb9864)
matriv added a commit that referenced this pull request Feb 24, 2020
Add default value to each one of the usages of `allow_no_indices`
since it differs between different APIs.

Relates to: #52534

(cherry picked from commit 2eb9864)
@jakelandis jakelandis removed the v8.0.0 label Jul 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >docs General docs changes v7.6.1 v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants