Omer Frenkel has posted comments on this change.

Change subject: 5. core: Persist all async task place holders
......................................................................


Patch Set 2: (1 inline comment)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 1259:      * @param runAsInternal
Line 1260:      * @param context
Line 1261:      * @return
Line 1262:      */
Line 1263:     protected CommandBase<?> constructCommand(VdcActionType 
actionType,
i remember when we needed to introduce the 'running internal' logic, it was 
easy and safe to add since we had only 2 place the used to run commands, this 
is one advantage.
another is the consistency, when i want to run command from a command, i know 
to use backend.runInternal()
now there is another way, which may be confusing (to know when to use what)
so adding another way of creating and running commands feels to me like 
exposing something which is pretty internal.
im not saying im totally against, just feels wrong
Line 1264:             VdcActionParametersBase parameters,
Line 1265:             boolean runAsInternal,
Line 1266:             CommandContext context) {
Line 1267:         ExecutionHandler.evaluateCorrelationId(parameters);


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I66628b98298ed1f500946889604ad906f0a68022
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to