> On March 31, 2017, 3:31 p.m., Colm O hEigeartaigh wrote: > > I'm wondering if it makes sense to make the default > > getPolicyEngineOptions() configured via "configureDelegateAdmin"? Why not > > just let ServiceREST do that part? > > Zsombor Gegesy wrote: > I thought about creating a constructor with all the fields, and using > that in various places, but it seemed a bit harder to follow the logic with > one string field, and 6 booleans in the arguments of the constructor. > > Colm O hEigeartaigh wrote: > What I was suggested was to remove the new "default" behaviour in > RangerPolicyEngineCache for > "opts.configureDelegateAdmin(RangerConfiguration.getInstance(), > propertyPrefix);" and instead move this call to > getDelegatedAdminPolicyEngine. Why should the default behaviour in > RangerPolicyEngineCache be the delegated configuration?
I see. Unfortunately when the control returns to the getDelegatedAdminPolicyEngine, it just gets a RangerPolicyEngine (interface), and the actual implementation RangerPolicyEngineImpl doesn't even refer to the RangerPolicyEngineOptions, as all the usage is limited inside to the constructor call. Other solution could be, that these defaults passed in to the getPolicyEngine call as arguments ... I'm going to submit a new patch version with it - Zsombor ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57987/#review170728 ----------------------------------------------------------- On April 21, 2017, 12:17 p.m., Zsombor Gegesy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57987/ > ----------------------------------------------------------- > > (Updated April 21, 2017, 12:17 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-1478 > https://issues.apache.org/jira/browse/RANGER-1478 > > > Repository: ranger > > > Description > ------- > > RangerPolicyEngineOptions has a lot of public fields, which is written from > various places from the code base, which should be avoided. That object is > configured from RangerConfiguration, but in the middle of the plugin > initialization code, which makes this a bit more complex, than it should be. > Suggestions: > > RangerConfiguration should be treated as an object, not a static facade > for a couple of config values > RangerPolicyEngineOptions should get his configuration from directly the > RangerConfiguration, in an explicit, encapsulated way. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineCache.java > 5376b52 > > agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyEngineOptions.java > a9027bc > > agents-common/src/main/java/org/apache/ranger/plugin/service/RangerBasePlugin.java > 7010b43 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > b9f1832 > > > Diff: https://reviews.apache.org/r/57987/diff/3/ > > > Testing > ------- > > > Thanks, > > Zsombor Gegesy > >
