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() { > the idea is that in the command you might want to check this, for example i why twice ? suppose you have a SomeCommand: class SomeCommand ... { @Override protected boolean executeCommandRequired() { return foo(); } private boolean foo() { // decide if this should be cached on this command } } So if there is a wider use for it to be cached - go for it. Else, i don't see why we should add more code to the base command beyond necessary. 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() { > not sure i understand, the idea here is to provide an easy why to skip the No, that wasn't my intention. You may decide to skip either the canDoAction validation and go straight to the execution (i.e. most update commands should allow updating the description and comment in the update flows, if only those were modified), hence you'd like to skip the complexity of the can-do-action which tend to access the db several times. But you'd still wish to execute the command. Or, you can decide to skip both can-do-action and execution. Leaving aside the can-do-action which isn't relevant to this specific patch: By default, we assume the executeCommand should be invoked, hence he default should be 'true' (same as Arik has suggested). Then, only commands which wish not to execute the executeCommand, will have to override that method. (meaning the reverse condition to what you currently have in the following patch) 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
