Add Delete Privileges API to HLRC#35454
Conversation
|
Pinging @elastic/es-core-infra |
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
s/delete-role/delete-privilege
There was a problem hiding this comment.
I just saw it and pushed 85bfdc8
There was a problem hiding this comment.
In other APIs, we have allowed null and used RefreshPolicy.getDefault(). I don't have strong feelings for implicit vs explicit in this case but I think consistency helps. Would it make sense to make a decision and use this for all APIs? @hub-cap WDYT ?
There was a problem hiding this comment.
I made it consistent and changed the code to use the RefreshPolicy.getDefault().
We can still change this later if someone thinks it should be immediate too.
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Mental note to possibly replcae this when Put Privilege is in place
There was a problem hiding this comment.
Yes, that's why I asked you to review this :)
bizybot
left a comment
There was a problem hiding this comment.
Few nits and some changes, else LGTM.
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
...rest-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesRequest.java
Outdated
Show resolved
Hide resolved
...rest-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesRequest.java
Outdated
Show resolved
Hide resolved
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Outdated
Show resolved
Hide resolved
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityClient.java
Outdated
Show resolved
Hide resolved
bizybot
left a comment
There was a problem hiding this comment.
LGTM, once the comments are addressed and the build is successful. Thank you.
There was a problem hiding this comment.
Sorry I missed this earlier, I think this should be ensureExpectedToken(XContentParser.Token.START_OBJECT,token, parser::getTokenLocation)
(expected, actual, getTokenLocation or else might result in wrong error message.
...est-high-level/src/main/java/org/elasticsearch/client/security/DeletePrivilegesResponse.java
Outdated
Show resolved
Hide resolved
This commit adds the Delete Privileges API to the high level REST client. Related to elastic#29827
217acac to
e33ac10
Compare
This commit adds the Delete Privileges API to the high level REST client. Related to #29827
This commit adds the Delete Privileges API to the high level REST client.
Related to #29827