Omer Frenkel 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() { > why do you need to cache it ? from command perspective, this should be call the idea is that in the command you might want to check this, for example in the audit log, in order to provide a different message. i can't predict if this code will have some db access logic or even maybe call to vdsm (god forbid) anyway i dont see any harm in providing this, we have caching in commands for most members, and this is the right place to do it, caching in certain flow will cause it to be called at least twice.. 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 conditional execution exists for UpdateNetworkCommand.canDoAction(),wh not sure i understand, the idea here is to provide an easy why to skip the command, according to what you suggest, in order to do so, i will need to override both in each command? not sure it is so nice. i dont think this is the right place to do what you suggest, if you have a condition that makes other validation not necessary, just return true.. 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
