----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/73344/#review223001 -----------------------------------------------------------
plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Line 122 (original), 132 (patched) <https://reviews.apache.org/r/73344/#comment312155> For consistency with rest of Ranger source code, and better readability, consider moving private methods after all public and protected methods. Here are the coding guidelines for Apache Ranger: https://cwiki.apache.org/confluence/display/RANGER/Coding+guidelines plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Lines 199 (patched) <https://reviews.apache.org/r/73344/#comment312156> Since Scala Set is no more used, consider importing java.util.Set (similar to Map/List/..), and drop the package name from here. ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Line 130 (original), 130 (patched) <https://reviews.apache.org/r/73344/#comment312153> For consistency with rest of Ranger source code, and better readability, consider moving private methods after all public and protected methods. Here are the coding guidelines for Apache Ranger: https://cwiki.apache.org/confluence/display/RANGER/Coding+guidelines ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java Line 149 (original), 144 (patched) <https://reviews.apache.org/r/73344/#comment312154> Shim should simply forward all calls to the implementation instance. If this is not feasible here, please add a comment. Else forward with: try { activatePluginClassLoader(); return rangerKakfaAuthorizerImpl.start(authorizerServerInfo); } finally { deactivatePluginClassLoader(); } - Madhan Neethiraj On May 10, 2021, 9:06 a.m., Chia-Ping Tsai wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/73344/ > ----------------------------------------------------------- > > (Updated May 10, 2021, 9:06 a.m.) > > > Review request for ranger. > > > Repository: ranger > > > Description > ------- > > As described in the KIP, `org.apache.kafka.server.authorizer.Authorizer` is > an improvement over `kafka.security.auth.Authorizer` and it's a pure Java > interface (instead of Scala). > `kafka.security.auth.Authorizer` has been deprecated since December 2019 and > it will be removed in Apache Kafka 3.0 (roughly planned for July/August). > See the KIP for more details: > https://cwiki.apache.org/confluence/display/KAFKA/KIP-504+-+Add+new+Java+Authorizer+Interface > > > Diffs > ----- > > > plugin-kafka/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java > 2a1b812e0 > > ranger-kafka-plugin-shim/src/main/java/org/apache/ranger/authorization/kafka/authorizer/RangerKafkaAuthorizer.java > 9d72ae0c8 > > > Diff: https://reviews.apache.org/r/73344/diff/1/ > > > Testing > ------- > > run `mvn clean test` and all pass on my local. > > > Thanks, > > Chia-Ping Tsai > >
