----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59143/#review174551 -----------------------------------------------------------
Couple of feedback items: 1. I agree with Colm that it is better to have default values in one place (PasswordUtils.java) 2. I noticed that the new method "setPropertiesvalues(String aPassword) in PasswordUtils.java contains very similar logic that is repeated in ServiceDBStore.java and RangerServiceService.java. Can we merge this common logic to one place? 3. One quick question - In RangerServiceService.java - getConfigsWithDecryptedPassword(), I noticed that ServiceDBStore.CRYPT_ALGO is being updated every time. Didn't completely understand the logic on why we need to do this? Can you please provide some details. 4. Are you planning to provide a different patch for ranger-0.7 branch? The current patch fails to apply on ranger-0.7 branch. 5. And in case of upgrade, since these new properties are not going to be there, I am guessing the default values will be used. - Sailaja Polavarapu On May 10, 2017, 1:57 p.m., bhavik patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59143/ > ----------------------------------------------------------- > > (Updated May 10, 2017, 1:57 p.m.) > > > Review request for ranger, Ankita Sinha, Gautam Borad, Abhay Kulkarni, Madhan > Neethiraj, Mehul Parikh, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, > and Velmurugan Periasamy. > > > Bugs: RANGER-1571 > https://issues.apache.org/jira/browse/RANGER-1571 > > > Repository: ranger > > > Description > ------- > > Code Improvement To Follow Best Practices. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java > 29983da > > agents-common/src/main/java/org/apache/ranger/plugin/util/PasswordUtils.java > f32355a > security-admin/src/main/java/org/apache/ranger/biz/ServiceDBStore.java > 33e0e85 > > security-admin/src/main/java/org/apache/ranger/service/RangerServiceService.java > f194b18 > security-admin/src/main/resources/conf.dist/ranger-admin-default-site.xml > 0feecfe > > > Diff: https://reviews.apache.org/r/59143/diff/2/ > > > Testing > ------- > > 1. Tested Ranger Admin & Ranger KMS rest calls. > 2. Verified Test-Connection for plugins on kerberized environment. > > > Thanks, > > bhavik patel > >
