Yair Zaslavsky has posted comments on this change.
Change subject: engine : Introduction of CommandManager
......................................................................
Patch Set 2: (6 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CacheWrapper.java
Line 9: V get(K key);
Line 10:
Line 11: void remove(final K key);
Line 12:
Line 13: public boolean containsKey(NGuid stepId);
why is the type of stepId NGuid and K? Why is stepId called "stepId" and not
key?
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/CommandManager.java
Line 41:
Line 42: private CommandManager() {
Line 43: //Create a singleton CacheManager using defaults
Line 44: CacheManager manager = CacheManager.create();
Line 45: manager.addCache(initGetCache(EXECUTION_CONTEXT_NAME));
I would like if possible to have more separation here between the cache
provider (ehcache) and the CommandManager. For example - consider having
CacheProviderFactory which will return the CacheWrapper, and if needed - add
more methods to CacheWrapper.
Did you take a look at DataGrid/Cache JSRs? can we use them?
In addition, I prefer to read the cache initialization info from the
configuration.
Line 46: executionContextMap = new EhcacheWrapper<NGuid,
ExecutionContext>(manager, EXECUTION_CONTEXT_NAME);
Line 47: }
Line 48:
Line 49: private Cache initGetCache(String name) {
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/tasks/EhcacheWrapper.java
Line 38: return cacheManager.getEhcache(cacheName);
Line 39: }
Line 40:
Line 41: @Override
Line 42: public boolean containsKey(NGuid stepId) {
Same question as before about NGuid.
Line 43: return getCache().isKeyInCache(stepId) ||
getCache().get(stepId) != null;
Line 44: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/interfaces/CallBack.java
Line 3: import org.ovirt.engine.core.common.action.VdcActionParametersBase;
Line 4: import org.ovirt.engine.core.common.action.VdcActionType;
Line 5: import org.ovirt.engine.core.common.action.VdcReturnValueBase;
Line 6: import org.ovirt.engine.core.compat.NGuid;
Line 7:
We still need to discuss how we're going to use the scheduling framrwork
presented by Saggi, and if there will be changes in it.
Line 8: public interface CallBack {
Line 9: public VdcReturnValueBase endAction(NGuid stepId, VdcActionType
actionType, VdcActionParametersBase actionParameters);
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/interfaces/TaskHelper.java
Line 1: /*
Line 2: * To change this template, choose Tools | Templates
Remove the comment above package
Line 3: * and open the template in the editor.
Line 4: */
Line 5: package org.ovirt.engine.core.common.interfaces;
Line 6:
Line 16: import org.ovirt.engine.core.common.vdscommands.VDSReturnValue;
Line 17: import org.ovirt.engine.core.compat.Guid;
Line 18: import org.ovirt.engine.core.compat.NGuid;
Line 19:
Line 20: public interface TaskHelper {
I think some comments here on why you want TaskHelper - can help.
Line 21: Step addTaskStep(ExecutionContext context, StepEnum stepName,
String description);
Line 22: void endTaskStep(NGuid stepId, JobExecutionStatus exitStatus);
Line 23: void updateStepExternalId(Step step, Guid externalId,
ExternalSystemType systemType);
Line 24: boolean acquireLockAsyncTask();
--
To view, visit http://gerrit.ovirt.org/13152
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibdd7585bfcfa6adeb761a8532218ba1aaa5e3c5d
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Ravi Nori <[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
