> On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote: > > plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java > > Lines 74 (patched) > > <https://reviews.apache.org/r/72272/diff/2/?file=2215813#file2215813line76> > > > > Since 'useUgi' is set only in the constructor, consider marking this > > as a 'final'. Same applies for 'rangerPlugin' as well. Please review.
that won't work for useUgi. Declaring it final thecompiler complains that it cannot set it. > On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote: > > plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java > > Lines 157 (patched) > > <https://reviews.apache.org/r/72272/diff/2/?file=2215813#file2215813line166> > > > > result.isRowFilterEnabled() already checks if the filter-expr is empty > > or not; so 'StringUtils.isNotEmpty(result.getFilterExpr());' is not needed > > here. Please review. came from the Hive Authorizer. Removed > On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote: > > ranger-presto-plugin-shim/pom.xml > > Lines 93 (patched) > > <https://reviews.apache.org/r/72272/diff/2/?file=2215820#file2215820line93> > > > > The changes in this module don't seem to refer hadoop-hdfs library > > contents directly. Is it necessary to explicitly add this dependency? I'm not getting it into the assembly otherwise for auditing purposes? please advise. > On April 5, 2020, 6:09 p.m., Madhan Neethiraj wrote: > > ranger-presto-plugin-shim/pom.xml > > Lines 134 (patched) > > <https://reviews.apache.org/r/72272/diff/2/?file=2215820#file2215820line134> > > > > The changes in this module don't seem to refer protobuf-java library > > contents directly. Is it necessary to explicitly add this dependency? Idem as above. How to get it in the assembly? It seems not to be included otherwise (struggling with that overall btw) - Bolke ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/72272/#review220223 ----------------------------------------------------------- On March 30, 2020, 5:20 p.m., Bolke de Bruin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/72272/ > ----------------------------------------------------------- > > (Updated March 30, 2020, 5:20 p.m.) > > > Review request for ranger, Madhan Neethiraj, Mehul Parikh, Pradeep Agrawal, > and Ramesh Mani. > > > Bugs: https://issues.apache.org/jira/browse/RANGER-2754 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/RANGER-2754 > > > Repository: ranger > > > Description > ------- > > Upgrade and improve Presto plugin > - Presto SQL 331 has changed its security API and has Row level / column > masking functionality > - Upgraded Hadoop dependency to 3.1.3 (from 3.1.1) due to improved security > handling > - New features like session properties and system properties > > > Diffs > ----- > > agents-common/src/main/resources/service-defs/ranger-servicedef-presto.json > 56a8f5ac0 > distro/src/main/assembly/plugin-presto.xml d2075bfe7 > plugin-presto/pom.xml b63f7dede > > plugin-presto/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java > 3ab63f590 > > plugin-presto/src/test/java/org/apache/ranger/authorization/presto/authorizer/RangerAdminClientImpl.java > PRE-CREATION > > plugin-presto/src/test/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControlTest.java > PRE-CREATION > plugin-presto/src/test/resources/log4j.properties PRE-CREATION > plugin-presto/src/test/resources/presto-policies.json PRE-CREATION > plugin-presto/src/test/resources/ranger-presto-security.xml PRE-CREATION > pom.xml 22926fd7d > ranger-presto-plugin-shim/pom.xml d8ff88d0f > > ranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerConfig.java > 67b0d2434 > > ranger-presto-plugin-shim/src/main/java/org/apache/ranger/authorization/presto/authorizer/RangerSystemAccessControl.java > e89f646e1 > > > Diff: https://reviews.apache.org/r/72272/diff/2/ > > > Testing > ------- > > - New Unit tests added > - Tested locally in production > > > Thanks, > > Bolke de Bruin > >
