> On March 30, 2017, 6:46 a.m., Colm O hEigeartaigh wrote: > > In HiveClient, the additional properties (+ the existing ones) should all > > be made private. Also remove the default values (= null, = false, etc.). > > Some of the statements are not consistent in terms of whitespace, for > > example "for (String db: dbList)" and "for(String tbl:tblList)". > > The "String errMsg" is defined multiple times in HiveClient. It would be > > better to make it a private static final constant in the class and refer to > > it that way.
Thanks Colm, I have updated the review request with suggested changes. - Ankita ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58068/#review170535 ----------------------------------------------------------- On March 30, 2017, 3:59 a.m., Ankita Sinha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58068/ > ----------------------------------------------------------- > > (Updated March 30, 2017, 3:59 a.m.) > > > Review request for ranger, Don Bosco Durai, Gautam Borad, Abhay Kulkarni, > Madhan Neethiraj, Pradeep Agrawal, Ramesh Mani, Selvamohan Neethiraj, Sailaja > Polavarapu, and Velmurugan Periasamy. > > > Bugs: RANGER-1483 > https://issues.apache.org/jira/browse/RANGER-1483 > > > Repository: ranger > > > Description > ------- > > Currently, ranger hive service definition uses hiveserver2(hs2) jdbc driver > to fetch hive db/table info, which is used to populate drop downs while > creating/updating policies. Adding ranger hive service definition to also use > hive metastore client which read from hive metastore db. > > > Diffs > ----- > > > agents-common/src/main/java/org/apache/ranger/plugin/client/HadoopConfigHolder.java > 8e79706 > > hive-agent/src/main/java/org/apache/ranger/services/hive/client/HiveClient.java > e3074ce > security-admin/pom.xml d2a9e18 > security-admin/src/main/webapp/scripts/views/service/ConfigurationList.js > dcc85ab > > > Diff: https://reviews.apache.org/r/58068/diff/1/ > > > Testing > ------- > > Tested on Simple environment. > Tested on Secure cluster > Tested with SSL > > > Thanks, > > Ankita Sinha > >
