Skip to content

Return Total Result Count and Remaining Count in Get Snapshots Response#76150

Merged
original-brownbear merged 3 commits intoelastic:masterfrom
original-brownbear:track-total-count
Aug 9, 2021
Merged

Return Total Result Count and Remaining Count in Get Snapshots Response#76150
original-brownbear merged 3 commits intoelastic:masterfrom
original-brownbear:track-total-count

Conversation

@original-brownbear
Copy link
Copy Markdown
Contributor

@original-brownbear original-brownbear commented Aug 5, 2021

Add total result count and remaining count to get snapshots response.

Add total result count and remaining count to get snapshots response.
final SnapshotsInRepo snInfos = sortSnapshots(allSnapshots, sortBy, after, size, order);
final List<SnapshotInfo> snapshotInfos = snInfos.snapshotInfos;
final int remaining;
final boolean hasMore = snInfos.hasMore || responses.stream().anyMatch(r -> r.v2() != null && r.v2().hasMore);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The logic for all this counting could be made for nicer obviously, but for now I went with the shortest route to getting this implemented on top of the existing functionality as we will be changing how this works under the hood soon anyway.

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.

Will remaining not anyway become 0 if hasMore==false? Meaning that we can skip calculating this boolean and just sum up remaining always? And then use that instead of hasMore below? Just seems nicer without too much rewrite.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Right, dropped hasMore completely now :)

@original-brownbear original-brownbear marked this pull request as ready for review August 5, 2021 11:32
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team. label Aug 5, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

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, left a few minor comments, really all optional.

final SnapshotsInRepo snInfos = sortSnapshots(allSnapshots, sortBy, after, size, order);
final List<SnapshotInfo> snapshotInfos = snInfos.snapshotInfos;
final int remaining;
final boolean hasMore = snInfos.hasMore || responses.stream().anyMatch(r -> r.v2() != null && r.v2().hasMore);
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.

Will remaining not anyway become 0 if hasMore==false? Meaning that we can skip calculating this boolean and just sum up remaining always? And then use that instead of hasMore below? Just seems nicer without too much rewrite.

@original-brownbear
Copy link
Copy Markdown
Contributor Author

Thanks Henning!

@original-brownbear original-brownbear merged commit a4983f5 into elastic:master Aug 9, 2021
@original-brownbear original-brownbear deleted the track-total-count branch August 9, 2021 09:34
original-brownbear added a commit that referenced this pull request Aug 14, 2021
* Return Total Result Count and Remaining Count in Get Snapshots Response (#76150)

Add total result count and remaining count to get snapshots response.

* Implement Numeric Offset Parameter in Get Snapshots API (#76233)

Add numeric offset parameter to this API.

Relates #74350
@original-brownbear original-brownbear restored the track-total-count branch April 18, 2023 20:54
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. v7.15.0 v8.0.0-alpha2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants