KeDeng 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 9: (7 comments) Thanks for your reviews. 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: ir = log > nit: override is OK. OVERRIDE is always translated to override in Kudu. Done 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: l > nit: add 2 spaces before and after '+'. Done http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@83 PS8, Line 83: << set > The Kudu log dosen't have such a '-', please keep consistent to the regular Done http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@96 PS8, Line 96: google::LogSeverity severity, const char* /*full_filename*/, : const char* /*base_filename*/, int /*line*/, > We can obtain time, base filename, file number from the parameters, not nee Yes, the variable names have been commented out here just to keep the API's parameter types for successful compilation, as this function is overridden. These variables are not used inside the function. http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/audit_logger.cc@100 PS8, Line 100: ostringstream oss; : oss << FormattedDebugInfo(severity); > Wrap this to FormattedDebugInfo() as well ? Done 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 char *kTableName = "test_table"; > We can simplify use GetSimpleTestSchema() in src/kudu/common/wire_protocol- Done http://gerrit.cloudera.org:8080/#/c/21613/8/src/kudu/master/master-test.cc@433 PS8, Line 433: > nit: swap the order to make the expected value as the first parameter. Done -- 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: 9 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 07:47:42 +0000 Gerrit-HasComments: Yes
