----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63552/#review190104 -----------------------------------------------------------
agents-audit/README.txt Lines 25 (patched) <https://reviews.apache.org/r/63552/#comment267351> We have to specify that the owner should be the same as the service process user. E.g. for HiveServer2, the owner should be "hive" agents-audit/README.txt Lines 28 (patched) <https://reviews.apache.org/r/63552/#comment267353> I assume, which this option, memory buffer won't be used. agents-audit/README.txt Lines 40 (patched) <https://reviews.apache.org/r/63552/#comment267352> We need to check with the ORC team to see what would be the ideal (raw) file size. agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java Lines 33 (patched) <https://reviews.apache.org/r/63552/#comment267355> Should I assume that this is JSONWriter? agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java Line 272 (original), 158 (patched) <https://reviews.apache.org/r/63552/#comment267356> Should we make this public, any derived class can override this needed. agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java Line 278 (original), 164 (patched) <https://reviews.apache.org/r/63552/#comment267357> This works good for writers with first class support. But can we give another option to just provide custom class here? agents-audit/src/main/java/org/apache/ranger/audit/utils/AuditWriter.java Lines 42 (patched) <https://reviews.apache.org/r/63552/#comment267358> Should we rename HDFS with WRITER? agents-audit/src/main/java/org/apache/ranger/audit/utils/AuditWriter.java Lines 97 (patched) <https://reviews.apache.org/r/63552/#comment267359> This class seems more HDFS specific. Should we just call it HDFSAuditWriter? I think, we have two variables: 1. FileSystem (Local File, HDFS, S3, etc) 2. Format/encoding (JSON, ORC, Parquet, Avro, etc.) We should be able to mix and match where possible. agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java Lines 96 (patched) <https://reviews.apache.org/r/63552/#comment267360> Assuming there are no performance issues, can we somehow generalize this? E.g. the AuthzAuditEvent class can provide an method to get the fields and their types. Then this code can just iterate over it and construct these objects. The reason being, as we support more types or users writing custom Writers, we and they don't need to keep these code in sync with any changes done to any new audit fields we add or modify. agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java Lines 154 (patched) <https://reviews.apache.org/r/63552/#comment267361> Should we log and ignore this Exception agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java Lines 167 (patched) <https://reviews.apache.org/r/63552/#comment267362> Can we make this a class constant? agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java Lines 172 (patched) <https://reviews.apache.org/r/63552/#comment267363> Can we get the structure from AuditEvent class? agents-audit/src/main/java/org/apache/ranger/audit/utils/TextWriter.java Lines 108 (patched) <https://reviews.apache.org/r/63552/#comment267365> General comment. In infrastructure code, we shouldn't use private method, unless absoluted needed. Making it protected gives an option for derived class to override the default implementation. agents-audit/src/main/java/org/apache/ranger/audit/utils/TextWriter.java Lines 122 (patched) <https://reviews.apache.org/r/63552/#comment267364> Ideally, all these should be done in base class, except for operations which are specific of the writer. Which can be abstracted out as abstract methods - Don Bosco Durai On Nov. 4, 2017, 12:38 a.m., Ramesh Mani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63552/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2017, 12:38 a.m.) > > > Review request for ranger, Don Bosco Durai and Madhan Neethiraj. > > > Repository: ranger > > > Description > ------- > > RANGER-1837:Enhance Ranger Audit to HDFS to support ORC file format > > > Diffs > ----- > > agents-audit/README.txt PRE-CREATION > agents-audit/pom.xml c8bd1d8 > > agents-audit/src/main/java/org/apache/ranger/audit/destination/HDFSAuditDestination.java > 66d8504 > > agents-audit/src/main/java/org/apache/ranger/audit/provider/AuditFileCacheProvider.java > 314b130 > agents-audit/src/main/java/org/apache/ranger/audit/provider/MiscUtil.java > eff3824 > agents-audit/src/main/java/org/apache/ranger/audit/utils/AuditWriter.java > PRE-CREATION > agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCFileUtil.java > PRE-CREATION > agents-audit/src/main/java/org/apache/ranger/audit/utils/ORCWriter.java > PRE-CREATION > agents-audit/src/main/java/org/apache/ranger/audit/utils/TextWriter.java > PRE-CREATION > agents-audit/src/main/java/org/apache/ranger/audit/utils/Writer.java > PRE-CREATION > pom.xml 589cd6a > src/main/assembly/hbase-agent.xml 3ebc334 > src/main/assembly/hdfs-agent.xml 5279a9a > src/main/assembly/hive-agent.xml ca65c80 > src/main/assembly/knox-agent.xml 8357d49 > src/main/assembly/plugin-atlas.xml fd98811 > src/main/assembly/plugin-kafka.xml 95855d9 > src/main/assembly/plugin-kms.xml 6d15f2a > src/main/assembly/plugin-solr.xml de30bfb > src/main/assembly/plugin-sqoop.xml d2bd69a > src/main/assembly/plugin-yarn.xml c6a48e8 > src/main/assembly/storm-agent.xml 64224ec > > > Diff: https://reviews.apache.org/r/63552/diff/3/ > > > Testing > ------- > > Testing done in local > > > Thanks, > > Ramesh Mani > >
