Skip to content

Add parameter to exclude indices in a snapshot from response#86269

Merged
original-brownbear merged 8 commits intoelastic:masterfrom
original-brownbear:82937
Apr 29, 2022
Merged

Add parameter to exclude indices in a snapshot from response#86269
original-brownbear merged 8 commits intoelastic:masterfrom
original-brownbear:82937

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear commented Apr 28, 2022

Adds a parameter index_names to 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

@original-brownbear original-brownbear added >enhancement :Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v8.3.0 labels Apr 28, 2022
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Apr 28, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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)
Copy link
Copy Markdown
Contributor

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;
Copy link
Copy Markdown
Contributor

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.

@@ -143,7 +143,7 @@ public GetSnapshotsRequestBuilder setOrder(SortOrder order) {
}

public GetSnapshotsRequestBuilder setIndices(boolean indices) {
Copy link
Copy Markdown
Contributor

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?

@original-brownbear
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

Copy link
Copy Markdown
Contributor

@henningandersen henningandersen left a 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 <33268011+henningandersen@users.noreply.github.com>
@original-brownbear
Copy link
Copy Markdown
Contributor Author

Description added.

Copy link
Copy Markdown
Contributor

@henningandersen henningandersen left a comment

Choose a reason for hiding this comment

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

LGTM.

@original-brownbear
Copy link
Copy Markdown
Contributor Author

Thanks Ievgen + Henning!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >enhancement Team:Distributed Meta label for distributed team. v8.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Parameter to not Return Index Name Lists in GET Snapshots API

5 participants