Oved Ourfali has posted comments on this change.

Change subject: wip: async vdscommand
......................................................................


Patch Set 1:

(3 comments)

That looks awesome!
Some comments and suggestions to make it cleaner.

https://gerrit.ovirt.org/#/c/40565/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java:

Line 487:                 return (VDSAsyncReturnValue) value;
Line 488:             }
Line 489:         }
Line 490: 
Line 491:         return null;
consider throwing an exception in this case.
Line 492:     }
Line 493: 
Line 494:     public <P extends VdsIdVDSCommandParametersBase> 
FutureVDSCall<VDSReturnValue> runFutureVdsCommand(final FutureVDSCommandType 
commandType,
Line 495:             final P parameters) {


https://gerrit.ovirt.org/#/c/40565/1/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageInfoVDSCommand.java
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/GetImageInfoVDSCommand.java:

Line 45: {
what about the proceedProxtReturnValue(). Don't we need it anymore?


Line 62: setAsyncResult
Great progress!

However, I would want to have as little to do in the command itself.

Perhaps we can have a setResult as part of the IrsBrokerCommandInterface?

I mean that we will always construct a callback for async executions, that will 
delegate to the command itself, and call command.process() or 
command.setResult() or whatever we call it.

Also, sync executions will also call this method, but not from within a 
callback interface, but directly.

So, IrsBrokerCommand will do:
                if (getIrsProxy() != null) {
                    executeIrsBrokerCommand();
                    if (async) {
                        create the AsyncCallback that calls setResult()
                    } else {
                        setResult();
                    }
...
...
..


That way, the code of the vds commands will be sync/async agnostic.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1c8a05c286a9008ef9a08ac7da02bc7211c4af9
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Piotr Kliczewski <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Oved Ourfali <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to