> On Aug. 16, 2019, 4:20 p.m., Pradeep Agrawal wrote: > > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > > Line 2054 (original), 2058 (patched) > > <https://reviews.apache.org/r/71217/diff/1/?file=2160907#file2160907line2058> > > > > There is already a deletePolicy(RangerPolicy policy, RangerService > > service) method there. Possibly you can use that by passing the service > > object value as null. > > Nikhil P wrote: > we are not adding a new method but changing existing method in order to > avoid extra DB call for get policy, so I feel its better to get rid of the > method with Id param which gives additional DB call to get policy which is > already present with the caller method of it.Also there would be no backward > compatability issue as this is not the REST layer method.
Won't a call to deletePolicy(RangerPolicy policy, RangerService service) method will serve the purpose here ? if yes, then either you can keep this method as is "deletePolicy(Long policyId)" or remove the implementation of this method completly(to avoid pmd issues). - Pradeep ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71217/#review217241 ----------------------------------------------------------- On Aug. 19, 2019, 9:51 a.m., Nikhil P wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71217/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2019, 9:51 a.m.) > > > Review request for ranger, Gautam Borad, Abhay Kulkarni, Madhan Neethiraj, > Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja Polavarapu, and > Velmurugan Periasamy. > > > Bugs: RANGER-2535 > https://issues.apache.org/jira/browse/RANGER-2535 > > > Repository: ranger > > > Description > ------- > > Good coding practices for storing and retrieving data history in ranger > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/store/ServiceStore.java > 2af5845e2 > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 113e727c4 > security-admin/src/main/java/org/apache/ranger/rest/ServiceREST.java > e7b317265 > > security-admin/src/main/java/org/apache/ranger/service/RangerDataHistService.java > 7bd0681b1 > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelHelper.java > PRE-CREATION > > security-admin/src/main/java/org/apache/ranger/service/RangerPolicyLabelsService.java > 1a1b56e4c > security-admin/src/test/java/org/apache/ranger/biz/TestServiceDBStore.java > 2be371892 > > > Diff: https://reviews.apache.org/r/71217/diff/2/ > > > Testing > ------- > > 1.Tested If data is getting stored and retrieved properly in data history > table i.e. x_data_hist. > > > Thanks, > > Nikhil P > >