----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/12932/#review24061 -----------------------------------------------------------
Hi Mengwei, thank you very much for working on this JIRA! core/src/main/java/org/apache/sqoop/core/AuditLogger.java <https://reviews.apache.org/r/12932/#comment47904> I would suggest to put all the audit related classes to it's own package "org.apache.sqoop.core.audit". core/src/main/java/org/apache/sqoop/core/AuditLoggerManager.java <https://reviews.apache.org/r/12932/#comment47906> I would recommend to not have this fallback solution in case that no audit loggers are defined. Running without audit logs might be a valid use case. core/src/main/java/org/apache/sqoop/core/DefaultAuditLogger.java <https://reviews.apache.org/r/12932/#comment47905> I would suggest to convert this class to a "FileAuditLogger" and put the default configuration into the default sqoop.properties configuration file. Benefit of that will be that user can easily configure two FileAuditLoggers and log to two places if necessary. core/src/main/java/org/apache/sqoop/core/DefaultAuditLogger.java <https://reviews.apache.org/r/12932/#comment47908> Nit the default file path is invalid as the "@LOGDIR" is just a placeholder that should be substituted in the configuration file. I would suggest to simply require specifying the file by user. core/src/main/java/org/apache/sqoop/core/SqoopServer.java <https://reviews.apache.org/r/12932/#comment47907> I would suggest to change the order and put the AuditLogger just after the SqoopConfiguration (as it do not have any further dependencies). Jarcec - Jarek Cecho On July 24, 2013, 11:17 p.m., Mengwei Ding wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/12932/ > ----------------------------------------------------------- > > (Updated July 24, 2013, 11:17 p.m.) > > > Review request for Sqoop, Abraham Elmahrek, Hari Shreedharan, and Jarek Cecho. > > > Bugs: SQOOP-1143 > https://issues.apache.org/jira/browse/SQOOP-1143 > > > Repository: sqoop-sqoop2 > > > Description > ------- > > commit 79fef1510f3a309b582a9f31045237b837703d27 > Author: Mengwei Ding <mengwei.d...@gmail.com> > Date: Tue Jul 23 16:51:53 2013 -0700 > > SQOOP-1143 Sqoop2: Provide server audit log for operations upon metadata > structures > > :000000 100644 0000000... cfcbfd9... A > core/src/main/java/org/apache/sqoop/core/AuditLogger.java > :000000 100644 0000000... 1f9e098... A > core/src/main/java/org/apache/sqoop/core/AuditLoggerConstants.java > :000000 100644 0000000... d48e93c... A > core/src/main/java/org/apache/sqoop/core/AuditLoggerError.java > :000000 100644 0000000... a8f0240... A > core/src/main/java/org/apache/sqoop/core/AuditLoggerManager.java > :000000 100644 0000000... f7b7491... A > core/src/main/java/org/apache/sqoop/core/DefaultAuditLogger.java > :100644 100644 90bb327... 9b02be2... M > core/src/main/java/org/apache/sqoop/core/SqoopServer.java > :100755 100755 76596b4... 2aba0a1... M > dist/src/main/server/conf/sqoop.properties > :100644 100644 038f602... 093ab95... M > server/src/main/java/org/apache/sqoop/handler/ConnectionRequestHandler.java > :100644 100644 4c389cc... 9528564... M > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > :100644 100644 04ffc3c... dbf6496... M > server/src/main/java/org/apache/sqoop/handler/FrameworkRequestHandler.java > :100644 100644 ab3f9d0... 50ccd26... M > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > :100644 100644 65686a8... f15da7b... M > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > :100644 100644 f92d107... 7b59a81... M > server/src/main/java/org/apache/sqoop/handler/VersionRequestHandler.java > > > Diffs > ----- > > core/src/main/java/org/apache/sqoop/core/AuditLogger.java PRE-CREATION > core/src/main/java/org/apache/sqoop/core/AuditLoggerConstants.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/core/AuditLoggerError.java PRE-CREATION > core/src/main/java/org/apache/sqoop/core/AuditLoggerManager.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/core/DefaultAuditLogger.java > PRE-CREATION > core/src/main/java/org/apache/sqoop/core/SqoopServer.java 90bb327 > dist/src/main/server/conf/sqoop.properties 76596b4 > server/src/main/java/org/apache/sqoop/handler/ConnectionRequestHandler.java > 038f602 > server/src/main/java/org/apache/sqoop/handler/ConnectorRequestHandler.java > 4c389cc > server/src/main/java/org/apache/sqoop/handler/FrameworkRequestHandler.java > 04ffc3c > server/src/main/java/org/apache/sqoop/handler/JobRequestHandler.java > ab3f9d0 > server/src/main/java/org/apache/sqoop/handler/SubmissionRequestHandler.java > 65686a8 > server/src/main/java/org/apache/sqoop/handler/VersionRequestHandler.java > f92d107 > > Diff: https://reviews.apache.org/r/12932/diff/ > > > Testing > ------- > > Do some manual test for new functionalities. > > Unit test passed. > > > Thanks, > > Mengwei Ding > >
