-
Notifications
You must be signed in to change notification settings - Fork 103
Add index block removal API #4684
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
Following you can find the validation changes for the APIs you have modified.
You can validate these APIs yourself by using the |
It's breaking but avoids duplicating the type names, which is forbidden.
} | ||
} | ||
|
||
export class IndicesBlockStatus { | ||
export class AddIndicesBlockStatus { |
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.
Thinking about future API extensions: If there is the chance for a get_index_block_status
endpoint in the future, it would probably make sense to have this type in the _types
namespace.
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 don't think that's super relevant, as an API like that will always return which type of block exists on the index, rather than just a boolean.
specification/indices/remove_block/examples/response/IndicesAddBlockResponseExample1.yaml
Outdated
Show resolved
Hide resolved
* @rest_spec_name indices.remove_block | ||
* @availability stack since=9.1.0 stability=stable | ||
* @availability serverless stability=stable visibility=public | ||
* @doc_id index-block-remove |
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.
Are there any index_privileges or cluster_privileges we need to specify? these will show up in the docs.
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.
Good question, I'm not familiar with privileges myself, so I'll ask around.
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 added the manage
index privilege. We'll have to add it to IndicesAddBlockRequest
as well, but I'll do that in a follow-up PR.
@pquentin I'm not sure how to resolve the conflicts with |
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.
Thanks! LGTM. Since everything in output/
is generated, I fixed the conflicts by running make contrib
.
* Add index block removal API Follow-up of elastic/elasticsearch#129128 * Fix block enum * Add doc-id to table * Fix lint * Move IndicesBlockOptions and IndicesBlockStatus to indices/_types It's breaking but avoids duplicating the type names, which is forbidden. * Separate BlockStatus as it is different * Rename response example file * Fix response fields * Add index privilege --------- Co-authored-by: Quentin Pradet <[email protected]> (cherry picked from commit e668a98)
* Add index block removal API Follow-up of elastic/elasticsearch#129128 * Fix block enum * Add doc-id to table * Fix lint * Move IndicesBlockOptions and IndicesBlockStatus to indices/_types It's breaking but avoids duplicating the type names, which is forbidden. * Separate BlockStatus as it is different * Rename response example file * Fix response fields * Add index privilege --------- (cherry picked from commit e668a98) Co-authored-by: Niels Bauman <[email protected]> Co-authored-by: Quentin Pradet <[email protected]>
Follow-up of elastic/elasticsearch#129128