Yaniv Bronhaim has posted comments on this change.

Change subject: engine : Refactoring Async tasks code
......................................................................


Patch Set 2: (2 inline comments)

If you have singleton for creating tasks do we still need TaskHandlerCommand? I 
don't see the reason for this interface.. Seems like we have a lot of mess 
around the tasks. How can we have specific implementation for SPMAsyncTasks? 
Don't we have only SPM async tasks? what other AsyncTasks can we have?

A class diagram or short explanation of how and why the implementation is right 
now as it is might help to proceed and decide about the separation of the code.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 1303:      */
Line 1304:     protected Guid createTask(AsyncTaskCreationInfo 
asyncTaskCreationInfo,
Line 1305:             VdcActionType parentCommand,
Line 1306:             String description, VdcObjectType entityType, Guid... 
entityIds) {
Line 1307:         return TasksUtil.createTask(this, asyncTaskCreationInfo, 
parentCommand, description, entityType, entityIds);
why do we need this tunnel? if we want to call createTask we can use the 
tasksUtil, we don't need it also as part of commandBase, its redundant.
Same for concreteCreateTask, CancelTasks, and all the others below - that's the 
reason for the TasksUtil, to separate it from the command.
Line 1308:     }
Line 1309: 
Line 1310:     public SPMAsyncTask concreteCreateTask(
Line 1311:             AsyncTaskCreationInfo asyncTaskCreationInfo,


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/TasksUtil.java
Line 32: /**
Line 33:  *
Line 34:  * @author rnori
Line 35:  */
Line 36: public class TasksUtil {
I know we have asyncTaskManger already, but this object looks more like General 
Tasks Manager\Controller\Handler than utility. AsyncTaskManager context can 
move to TasksUtil or the other way around. TaskManager should include treatment 
also for Async and Sync tasks.
Line 37:     public static final TasksUtil instance = new TasksUtil();
Line 38:     public static final Log log = 
LogFactory.getLog(instance.getClass());
Line 39:     /**
Line 40:      * Use this method in order to create task in the AsyncTaskManager 
in a safe way. If you use this method within a


--
To view, visit http://gerrit.ovirt.org/12561
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie3839ed09825f1e0db914f55f5ee358b6b3c0449
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to