Moti Asayag has posted comments on this change. Change subject: core: Allow skipping command execution ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/34935/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 763: } Line 764: return returnValue; Line 765: } Line 766: Line 767: protected final boolean getCachedShouldSkipCommandExecution() { > i really think using get here sounds better, let see what others think why do you need to cache it ? from command perspective, this should be called once. don't see any reason to temper the code with caching. If needed specifically in certain flow - you can have it there. Line 768: if (skipCommandExecution == null) { Line 769: skipCommandExecution = shouldSkipCommandExecution(); Line 770: } Line 771: return skipCommandExecution; Line 770: } Line 771: return skipCommandExecution; Line 772: } Line 773: Line 774: protected boolean shouldSkipCommandExecution() { > same here, this way makes more sense to me, leaving for others to comment a same conditional execution exists for UpdateNetworkCommand.canDoAction(),where canDoAction execution should be skipped if a certain condition met. Hence from commands perspective i'd support having: protected boolean canDoActionRequired() {return true;} protected boolean executeCommandRequired() {return true;} that could be overridden by the derivatives. Line 775: return false; Line 776: } Line 777: Line 778: private boolean internalValidateAndSetQuota() { -- To view, visit http://gerrit.ovirt.org/34935 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib86226c3af0c245ec01c59ac650c9282d58f22bc Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Omer Frenkel <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Roy Golan <[email protected]> Gerrit-Reviewer: Shahar Havivi <[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
