Martin Mucha has posted comments on this change. Change subject: core: added missing logging + refactoring ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/29244/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java: Line 112: AuditLogSeverity severity = logType.getSeverity(); Line 113: Line 114: //TODO MM: shouldn't this be rather warning? This assumption is hardly valid. Line 115: //TODO MM: there's NO such situation when this can happen; and since we find this invalid anyway, it should be 'blocked' in AuditLogType constructor. Line 116: if (severity == null) { > I can be wrong, but how's that possible? Severity is set via one of constru I'm not sure about previous code of this patch and one this patch needs, but now it's really not possible that severity is not null. Todos and check are removed, so I'm just closing this unanswered questions. Done. Line 117: severity = AuditLogSeverity.NORMAL; Line 118: log.infoFormat("No severity for {0} audit log type, assuming Normal severity", logType); Line 119: } Line 120: AuditLog auditLog = createAuditLog(auditLogable, logType, loggerString, severity); Line 143: logMessage(severity, getMessageToLog(loggerString, auditLog)); Line 144: } Line 145: } Line 146: Line 147: private static void logMessage(AuditLogSeverity severity, String logMessage) { > ok, I've overlooked that this is bound to gwt. That it has to stay in this Done Line 148: //TODO MM: rather than this, there could(should?) be OO approach: introducing 'log' method on AuditLogSeverity. Line 149: switch (severity) { Line 150: case NORMAL: Line 151: log.info(logMessage); -- To view, visit http://gerrit.ovirt.org/29244 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic737ace1808e1f242d0eb08ee458869a89be500e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> Gerrit-Reviewer: Martin Mucha <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
