Alon Bar-Lev has posted comments on this change.

Change subject: core: remove ovirt logger from vdsbroker and friends
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetConfigurationValueQuery.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetConfigurationValueQuery.java:

Line 25:                 }
Line 26:                 returnValue = Config.<Object> getValue(value, version);
Line 27:             } catch (Exception e) {
Line 28:                 log.error("Unable to return config parameter {}: {}", 
getParameters(), e.getMessage());
Line 29:                 log.debug("Exception", e);
> why did you move the exception to debug?
logs should be free of stack trace as much as possible, unless there are really 
important errors we cannot live without, in any other case a system can be put 
in debug mode to get full stack traces.
Line 30:             }
Line 31:         }
Line 32: 
Line 33:         getQueryReturnValue().setReturnValue(returnValue);


http://gerrit.ovirt.org/#/c/34374/3/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/CreateVmVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/CreateVmVDSCommand.java:

Line 96:         Guid guid = getParameters().getVm().getId();
Line 97:         String vmName = getParameters().getVm().getName();
Line 98:         VmDynamic vmDynamicFromDb = 
DbFacade.getInstance().getVmDynamicDao().get(guid);
Line 99:         if 
(ResourceManager.getInstance().IsVmDuringInitiating(getParameters().getVm().getId()))
 {
Line 100:             log.info("Vm Running failed - vm {}:{} already running", 
guid, vmName);
> you missed to change the order of guid:name to name:guid here (as was done 
because the user cares about the name and the developer cares about the guid, 
and user is more important when it comes to log.
Line 101:             
getVDSReturnValue().setReturnValue(vmDynamicFromDb.getStatus());
Line 102:             return false;
Line 103:         } else {
Line 104:             VMStatus vmStatus = vmDynamicFromDb.getStatus();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae367e2c2fe3df862c8a345ca73575198f4c6edd
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Martin PeÅ™ina <[email protected]>
Gerrit-Reviewer: Moti Asayag <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[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

Reply via email to