----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38116/#review97729 -----------------------------------------------------------
Ramesh - I am not done with the review yet. Sending the comments for the review done so fa. agents-common/scripts/enable-agent.sh (line 39) <https://reviews.apache.org/r/38116/#comment153740> "cut -d/ -f5" - this extracts the 5th field. What is the expected input to this method? Does it assume a specific directory level? ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 37) <https://reviews.apache.org/r/38116/#comment153741> If urls is not needed as a class member, it will be good to remove. ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 69) <https://reviews.apache.org/r/38116/#comment153742> I think it will be good to propagate the exception to the caller (in this case namenode). Please review. ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 88) <https://reviews.apache.org/r/38116/#comment153743> Handle rangerHdfsAuthorizerImpl == null. ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java (line 91) <https://reviews.apache.org/r/38116/#comment153744> Consider the following pattern: if(rangerHdfsAuthorizerImpl != null) { rangerPluginClassLoader.activate(); try { rangerHdfsAuthorizerImpl.start(); } finally { rangerPluginClassLoader.deactivate(); } } activate() and deactivate() should set Thread.currentThread().setContextLoader() appropriately. The same pattern can be used for all calls that get forwarded to rangerHdfsAuthorizerImpl. ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoaderUtil.java (line 35) <https://reviews.apache.org/r/38116/#comment153774> "instance" sounds a better name for this member ("config"). ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoaderUtil.java (line 66) <https://reviews.apache.org/r/38116/#comment153775> Should this method be "public"? If not needed, please make this a private method. - Madhan Neethiraj On Sept. 4, 2015, 12:53 a.m., Ramesh Mani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38116/ > ----------------------------------------------------------- > > (Updated Sept. 4, 2015, 12:53 a.m.) > > > Review request for ranger, Don Bosco Durai, Madhan Neethiraj, Selvamohan > Neethiraj, and Velmurugan Periasamy. > > > Repository: ranger > > > Description > ------- > > Ranger plugins should not add dependent libraries to component's > CLASSPATH-hdfs-plugin-patch > > > Diffs > ----- > > agents-common/scripts/enable-agent.sh 16efe74 > > agents-common/src/main/java/org/apache/ranger/authorization/hadoop/config/RangerConfiguration.java > 0a8907c > > hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java > fa2155c > > hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizerImpl.java > PRE-CREATION > pom.xml 4b73a61 > ranger-hdfs-plugin-shim/dependency-reduced-pom.xml PRE-CREATION > ranger-hdfs-plugin-shim/pom.xml PRE-CREATION > ranger-hdfs-plugin-shim/resources/META-INF/MANIFEST.MF PRE-CREATION > > ranger-hdfs-plugin-shim/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java > PRE-CREATION > ranger-hdfs-plugin-shim/src/main/resources/META-INF/MANIFEST.MF > PRE-CREATION > ranger-plugin-classloader/pom.xml PRE-CREATION > > ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoader.java > PRE-CREATION > > ranger-plugin-classloader/src/main/java/org/apache/ranger/plugin/classloader/RangerPluginClassLoaderUtil.java > PRE-CREATION > > ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestChildFistClassLoader.java > PRE-CREATION > > ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPluginImpl.java > PRE-CREATION > > ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/Impl/TestPrint.java > PRE-CREATION > > ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPlugin.java > PRE-CREATION > > ranger-plugin-classloader/src/test/java/org/apache/ranger/plugin/classloader/test/TestPrint.java > PRE-CREATION > src/main/assembly/hdfs-agent.xml 2c18001 > > Diff: https://reviews.apache.org/r/38116/diff/ > > > Testing > ------- > > Manual Testing done > > > Thanks, > > Ramesh Mani > >
