theoryxu commented on code in PR #5629:
URL: https://github.com/apache/gravitino/pull/5629#discussion_r1851936397
##########
authorizations/authorization-ranger/src/main/java/org/apache/gravitino/authorization/ranger/RangerHelper.java:
##########
@@ -480,8 +505,8 @@ protected void updatePolicyOwnerRole(RangerPolicy policy,
String ownerRoleName)
policyItem
.getAccesses()
.add(new
RangerPolicy.RangerPolicyItemAccess(ownerPrivilege.getName()));
- if (!policyItem.getRoles().contains(ownerRoleName)) {
- policyItem.getRoles().add(ownerRoleName);
+ if
(!policyItem.getRoles().contains(rangerRoleName(ownerRoleName))) {
+ policyItem.getRoles().add(rangerRoleName(ownerRoleName));
Review Comment:
Thank you for your advice. I get it now and agree that it's a better
pattern.
However, there are a lot of places that would need to be rewritten (not only
roles but also users, groups, policy items, etc.).
I'd like to focus on the core feature in this PR and handle the refactoring
in a separate one.
--
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]
