Arik Hadas has posted comments on this change.

Change subject: core: Allow skipping command execution
......................................................................


Patch Set 1: Code-Review+1

(4 comments)

I think it's a good solution for the problem, minor comments inside

http://gerrit.ovirt.org/#/c/34935/1//COMMIT_MSG
Commit Message:

Line 14: 2. add code in can-do-action and execute to skip execution
Line 15: 
Line 16: this patch adds an easy way to skip command execution,
Line 17: by overriding 'shouldSkipCommandExecution' method, we can mark the
Line 18: command no to run can-do-action and execute code.
s\no\not
Line 19: the command will just finish as succeeded and operation will be logged.
Line 20: 
Line 21: in the following patch i have an example of usage for the 'stop down 
vm'
Line 22: scenario.


Line 17: by overriding 'shouldSkipCommandExecution' method, we can mark the
Line 18: command no to run can-do-action and execute code.
Line 19: the command will just finish as succeeded and operation will be logged.
Line 20: 
Line 21: in the following patch i have an example of usage for the 'stop down 
vm'
s\the following patch\change-id\
Line 22: scenario.
Line 23: 
Line 24: Change-Id: Ib86226c3af0c245ec01c59ac650c9282d58f22bc


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() {
s\getCachedShouldSkipCommandExecution\isCachedShouldSkipCommandExecution\
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() {
I think it is better to think about the case where the command should be 
executed instead, that means to return true by-default call the method 
'isCommandExecutionRequired' or something like this, but that's subjective so 
for your consideration
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: 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

Reply via email to