Jackie-Jiang commented on code in PR #13195:
URL: https://github.com/apache/pinot/pull/13195#discussion_r1609077063
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java:
##########
@@ -48,7 +50,7 @@ default boolean hasAccess(RequesterIdentity
requesterIdentity) {
*
* @return {@code true} if authorized, {@code false} otherwise
*/
- boolean hasAccess(RequesterIdentity requesterIdentity, BrokerRequest
brokerRequest);
+ AuthorizationResult hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest);
Review Comment:
I don't think we need to change this one with `BrokerRequest` because it is
single-stage engine query.
##########
pinot-broker/src/main/java/org/apache/pinot/broker/api/AccessControl.java:
##########
@@ -48,7 +50,7 @@ default boolean hasAccess(RequesterIdentity
requesterIdentity) {
*
* @return {@code true} if authorized, {@code false} otherwise
*/
- boolean hasAccess(RequesterIdentity requesterIdentity, BrokerRequest
brokerRequest);
+ AuthorizationResult hasAccess(RequesterIdentity requesterIdentity,
BrokerRequest brokerRequest);
Review Comment:
This is a public stable interface. Directly changing the signature can cause
backward incompatibility. You may want to add new APIs and add default impl for
the old API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
