Martin Mucha has posted comments on this change. Change subject: core: added missing logging + refactoring ......................................................................
Patch Set 12: Hmm, this is rather nasty way to suppress logging, but ok, accepted. But to be correct in rest of code, there should be declarative way to express which types should be logged, or the test 'should I log you' should be expressed as a method of AuditLogType. I tried to locate place, where UNASSIGNED suppresses audit logging and found none. Only spaghetti bowl, where coincidence of not existing translation of UNASSIGNED AuditLogType and non-external(works ok for external) AuditLoggableBase leads to problems with AuditLog instance creation, thus nothing is 'audit-logged'(stored in db) and warning "Unable to create AuditLog" is logged into application log. If this is how this 'works', it should be fixed like I outlined. It's rather easy fix and will remove esoterial solution from code. Of course (improper) suppressing of logging with UNASSIGNED will remain as it is. It just become more robust. Agreed to fix this? See this draft: http://gerrit.ovirt.org/33359 also, we should reconsider current approach of do not log anything at all just because translation is missing. Other stuff is present and should be sufficient to present just what we have, like AuditLogType, serverity, user ... So quitting altogether seems like too lazy solution. see org.ovirt.engine.core.dal.dbbroker.auditloghandling.AuditLogDirector#createAuditLog -- 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: 12 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: No _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
