Arik Hadas has posted comments on this change.

Change subject: core: CommandBase - obtain child commands on endAction()
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/41690/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java:

Line 579:                     parameters.add(command.getParameters());
Line 580:                 }
Line 581:             }
Line 582: 
Line 583:             
commandBase.getParameters().setImagesParameters(parameters);
> Thanks for the review Arik, 
generalizing imagesParameters to something like ChildCommandsParameters will be 
a partial solution - it will solve #1 and #2, but not #3.
I think that one of the nice things in the command coordination's design is the 
separation between command and its tasks handling - the latter is handled by 
the callback which is external to the command instance (btw, the same way it 
used to be with async-tasks manager). imo, setting the parameters of the child 
commands should be done in the callback before calling the endAction. I also 
think that if you generalize imageParameters it could be placed in a general 
abstract callback and not in storage-specific callback.
Line 584:         }
Line 585:     }
Line 586: 
Line 587:     protected boolean isEndSuccessfully() {


-- 
To view, visit https://gerrit.ovirt.org/41690
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia563dc1b97e0df852e242420eca4f987a39fc2b9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Arik Hadas <aha...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to