singhpk234 commented on code in PR #3228:
URL: https://github.com/apache/polaris/pull/3228#discussion_r2604026293
##########
extensions/auth/opa/impl/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java:
##########
@@ -95,6 +95,21 @@ public OpaPolarisAuthorizer(
this.objectMapper = objectMapper;
}
+ @Override
+ public boolean requiresPrincipalRoles() {
+ return false;
+ }
+
+ @Override
+ public boolean requiresCatalogRoles() {
+ return false;
+ }
+
+ @Override
+ public boolean requiresResolvedEntities() {
+ return false;
+ }
Review Comment:
> strong reasons to avoid introducing even byte-code compatible changes
TBH i don't have strong concerns for introducing byte-code compatibility
changes, my main consideration were the version upgrade should be smooth and
the API should well documented so the downstream users can override it
accordingly which i think is already addressed in the recent commit so I am
good from that POV as this is definitely an integration point.
> I'd proposed to move the methods from SupportsDelegatedAuthZ into a (new)
AuthorizationHints interface to be produced from PolarisAuthorizerFactory and
used at call sites that can optimize execution based on the information the new
interface provides.
I leave this to the your better judgement ! I don't have a strong for or
against opinion on this ! since the concerns i had are already addressed.
cc @HonahX can you also please weigh in on this discussion
--
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]
