Add Sort By Shard Count and Failed Shard Count to Get Snapshots API#77011
Add Sort By Shard Count and Failed Shard Count to Get Snapshots API#77011original-brownbear merged 2 commits intoelastic:masterfrom original-brownbear:support-sort-by-shard-counts
Conversation
It's in the title. As requested by the Kibana team, adding these two additional sort columns.
|
Pinging @elastic/es-distributed (Team:Distributed) |
arteam
left a comment
There was a problem hiding this comment.
LGTM, I've left a couple of docs/naming comments.
| Sort snapshots by the number of shards they contain and break ties by snapshot name. | ||
|
|
||
| `failed_shards_count`:: | ||
| Sort snapshots by the number of shards that they failed to snapshot break ties by snapshot name. |
There was a problem hiding this comment.
that they failed to snapshot
I believe they is redundant here: the number of shards that failed to snapshot sounds more natural to me.
failed to snapshot break ties by snapshot name
I guess and is missed here: failed to snapshot and break ties by snapshot name
| `index_count`:: | ||
| Sort snapshots by the number of indices they contain and break ties by snapshot name. | ||
|
|
||
| `shard_count`:: |
There was a problem hiding this comment.
General comment: I'm not sure if the parameter names are set in stone, but I think it's inconsistent that shard_count is singular and failed_shards_count is plural. E.g. we refer to shard_count in the codebase as SHARDS and SHARDS_COUNT, but in the external API it's shard_count.
There was a problem hiding this comment.
++ that makes good sense, I made the external parameter failed_shard_count now to make it consistent. I think the enum/internal names are ok as is and we wouldn't really want to change them at this point for BwC reasons (at least technically the transport client would have its API broken otherwise).
|
Thanks Henning + Artem! |
It's in the title. As requested by the Kibana team, adding these two additional sort columns.