Yair Zaslavsky has posted comments on this change.

Change subject: core: user who added external events does not...
......................................................................


Patch Set 1:

(1 comment)

....................................................
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/auditloghandling/AuditLogDirector.java
Line 932:                     severity,
Line 933:                     resolvedMessage,
Line 934:                     auditLogable.getUserId(),
Line 935:                     auditLogable.getUserId() != null ? 
getDbFacadeInstance().getDbUserDao().get(auditLogable.getUserId()).getLoginName()
 : null,
Line 936:                     auditLogable.getVmIdRef(),
Hi,
Why couldn't you just use, for example getVmName - in the code it checks if the 
cached version field - mVmName is null -if null, getches the Vm from db, and so 
on.. pretty much the same as you did here.
Am I missing something?
I think this might be relevant to the other calls, and even if not, since such 
getXXXName() methods exist and AuditLoggableBase, other getXXXName() methods 
should reside there as well.
Line 937:                     auditLogable.getVmIdRef() != null ? 
getDbFacadeInstance().getVmDao().get(auditLogable.getVmIdRef()).getName() : 
null,
Line 938:                     auditLogable.getVdsIdRef(),
Line 939:                     auditLogable.getVdsIdRef() != null ? 
getDbFacadeInstance().getVdsDao().get(auditLogable.getVdsIdRef()).getName() : 
null,
Line 940:                     auditLogable.getVmTemplateIdRef(),


-- 
To view, visit http://gerrit.ovirt.org/17924
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I750fb1873d730c14ddab761062c3ec85157f0a77
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to