Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/21613 )
Change subject: [log] Support logging audit logs to a separate file ...................................................................... Patch Set 8: (7 comments) http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger-test.cc File src/kudu/master/audit_logger-test.cc: http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger-test.cc@65 PS8, Line 65: OVERRIDE nit: override is OK. OVERRIDE is always translated to override in Kudu. http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc File src/kudu/master/audit_logger.cc: http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@80 PS8, Line 80: + nit: add 2 spaces before and after '+'. http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@83 PS8, Line 83: << '-' The Kudu log dosen't have such a '-', please keep consistent to the regular log. http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@96 PS8, Line 96: const char* /*base_filename*/, int /*line*/, : const google::LogMessageTime& /*logmsgtime*/, We can obtain time, base filename, file number from the parameters, not needed to obtain manually. http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@100 PS8, Line 100: oss.fill('0'); : oss << google::GetLogSeverityName(severity)[0] Wrap this to FormattedDebugInfo() as well ? http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/master-test.cc@412 PS8, Line 412: const Schema kTableSchema({ ColumnSchema("key", INT32), We can simplify use GetSimpleTestSchema() in src/kudu/common/wire_protocol-test-util.h http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/master-test.cc@433 PS8, Line 433: dump_count, 1 nit: swap the order to make the expected value as the first parameter. -- To view, visit http://gerrit.cloudera.org:8080/21613 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie5323361befb456d91a12da7273865542f1d2430 Gerrit-Change-Number: 21613 Gerrit-PatchSet: 8 Gerrit-Owner: KeDeng <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: KeDeng <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Mon, 23 Sep 2024 06:35:51 +0000 Gerrit-HasComments: Yes
