mneethiraj commented on code in PR #595:
URL: https://github.com/apache/ranger/pull/595#discussion_r2302562771
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -103,6 +103,7 @@ public class RangerBasePlugin {
private RangerRoles roles;
private boolean
isUserStoreEnricherAddedImplcitly;
private Map<String, String> serviceConfigs;
+ private boolean synchronousPolicyRefreshFlag;
Review Comment:
`Flag` in the name is unnecessary. I suggest renaming to
`isSynchronousPolicyRefresh`.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -735,6 +749,11 @@ public Collection<RangerAccessResult>
isAccessAllowed(Collection<RangerAccessReq
}
public RangerAccessResult evalDataMaskPolicies(RangerAccessRequest
request, RangerAccessResultProcessor resultProcessor) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> evalDataMaskPolicies
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -765,6 +784,11 @@ public RangerAccessResult
evalDataMaskPolicies(RangerAccessRequest request, Rang
}
public RangerAccessResult evalRowFilterPolicies(RangerAccessRequest
request, RangerAccessResultProcessor resultProcessor) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> evalRowFilterPolicies
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -1455,4 +1492,18 @@ private static final class LogHistory {
long lastLogTime;
int counter;
}
+
+ private boolean isSynchronousPolicyCacheUpdateEnabled() {
+ boolean ret = false;
+ if (this.getServiceConfigs() != null && this.pluginConfig != null &&
this.pluginConfig.getServiceType() != null) {
+ String configPolicyCacheRefreshMode =
"ranger.plugins.conf.policy.cache.refresh.mode";
Review Comment:
This configuration is used in the plugin side (and not in Ranger admin
server). Hence, I suggest using existing convention in naming configurations -
`"ranger.plugin." + serviceType + ".policy.refresh.synchronous"`.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -653,6 +657,11 @@ public Collection<RangerAccessResult>
isAccessAllowed(Collection<RangerAccessReq
}
public RangerAccessResult isAccessAllowed(RangerAccessRequest request,
RangerAccessResultProcessor resultProcessor) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> isAccessAllowed
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -817,6 +846,11 @@ public RangerResourceACLs
getResourceACLs(RangerAccessRequest request) {
}
public RangerResourceACLs getResourceACLs(RangerAccessRequest request,
Integer policyType) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> getResourceACLs
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -795,6 +819,11 @@ public RangerAccessResult
evalRowFilterPolicies(RangerAccessRequest request, Ran
}
public void evalAuditPolicies(RangerAccessResult result) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> evalAuditPolicies
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
##########
agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java:
##########
@@ -694,6 +703,11 @@ public RangerAccessResult
isAccessAllowed(RangerAccessRequest request, RangerAcc
}
public Collection<RangerAccessResult>
isAccessAllowed(Collection<RangerAccessRequest> requests,
RangerAccessResultProcessor resultProcessor) {
+ if (this.synchronousPolicyRefreshFlag) {
+ LOG.debug("==> isAccessAllowed
isSynchronousPolicyCacheUpdateEnabled = {}", this.synchronousPolicyRefreshFlag);
Review Comment:
Is this log necessary, especially given the flag will always be `true` here.
I suggest removing the log here, and add inside `refreshPoliciesAndTags()` if
its already not present.
--
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: dev-unsubscr...@ranger.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
