Yair Zaslavsky has posted comments on this change.

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


Patch Set 2: (2 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 1226:      * This method is called before executeAction and after 
checkCanDoAction
Line 1227:      * to insert the async task place holders for the child commands.
Line 1228:      */
Line 1229:     protected void insertAsyncTaskPlaceHolders() {
Line 1230:         TransactionSupport.executeInNewTransaction(new 
TransactionMethod<Void>() {
No.
if buildChildCommands is implemented at "child command" insertPlaceHolder is 
implemented as well, so you  don't need to run in a new transaction there.
Line 1231:             @Override
Line 1232:             public Void runInTransaction() {
Line 1233:                 Map<Guid, CommandBase<?>> childCommands = 
buildChildCommands();
Line 1234:                 for (CommandBase<?> command : 
childCommands.values()) {


Line 1259:      * @param runAsInternal
Line 1260:      * @param context
Line 1261:      * @return
Line 1262:      */
Line 1263:     protected CommandBase<?> constructCommand(VdcActionType 
actionType,
This is a method that can be used in buildChildCommands() implementations to 
construct the child command.
I disagree that command should be executed only from backend - I know this is 
the current case, but I don't think we should stick to this principal if we 
need to create command internally for some other purpose at engine side.
Would u prefer to have a 
Backend.createCommand(VdcActionType,VdcActionParametersBase) instead - so we 
will get the command object, and then we will run createPlaceHolders on  all 
the commands (we must run them in transaction) , and then for each command we 
will run execute? IMHO this is an overkill.
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