-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Add parameter to exclude indices in a snapshot from response #86269
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
|
Pinging @elastic/es-distributed (Team:Distributed) |
|
Hi @original-brownbear, I've created a changelog YAML for you. |
| final boolean includeIndices = randomBoolean(); | ||
| final List<SnapshotInfo> defaultSorting = clusterAdmin().prepareGetSnapshots(repoName) | ||
| .setOrder(order) | ||
| .setIndices(includeIndices) |
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.
In my opinion both indices and includeIndices sounds quiet generic and might be confusing.
Should they be called includeIndexNames?
|
|
||
| private boolean verbose = DEFAULT_VERBOSE_MODE; | ||
|
|
||
| private boolean indices = true; |
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.
I think this is very generic and confusing outside of the issue context.
| return this; | ||
| } | ||
|
|
||
| public GetSnapshotsRequestBuilder setIndices(boolean indices) { |
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.
Do you mind renaming this one as well?
|
@elasticmachine update branch |
henningandersen
left a comment
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.
Just a drive-by comment. Can we also update the PR description to mention what this PR changes?
Co-authored-by: Henning Andersen <[email protected]>
|
Description added. |
henningandersen
left a comment
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.
LGTM.
|
Thanks Ievgen + Henning! |
Adds a parameter
index_namesto the get snapshots API so that users may exclude the potentially very long index name lists when listing out snapshots.Not super optimized but good enough to fix the REST response in currently broken cases and generally make the API a little lighter to use for debugging common issues.
closes #82937