Moti Asayag has posted comments on this change. Change subject: core: CommandBase - avoid inserting placeholders for dc without spm ......................................................................
Patch Set 1: Code-Review-1 (2 comments) https://gerrit.ovirt.org/#/c/41691/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 46: import org.ovirt.engine.core.bll.tasks.interfaces.SPMTask; Line 47: import org.ovirt.engine.core.bll.utils.Injector; Line 48: import org.ovirt.engine.core.bll.utils.PermissionSubject; Line 49: import org.ovirt.engine.core.common.AuditLogType; Line 50: import org.ovirt.engine.core.common.FeatureSupported; i might be missing something, but can't find where this is being used. Line 51: import org.ovirt.engine.core.common.VdcObjectType; Line 52: import org.ovirt.engine.core.common.action.LockProperties; Line 53: import org.ovirt.engine.core.common.action.LockProperties.Scope; Line 54: import org.ovirt.engine.core.common.action.VdcActionParametersBase; Line 1410: * This method is called before executeAction to insert the async task Line 1411: * placeholders for the child commands. Line 1412: */ Line 1413: protected void insertAsyncTaskPlaceHolders() { Line 1414: if (getStoragePool() != null && !isDataCenterWithSpm()) { 1. there is no implementation of isDataCenterWithSpm() in this patch. 2. This seems like a violation of separation of concerns: the command base shouldn't be aware of the system entities. It shouldn't be concerned by the meaning of DataCenter or any other business logic. If required specific behavior, we can refactor this method to allow it to be overridden easily. Line 1415: return; Line 1416: } Line 1417: Line 1418: TransactionSupport.executeInScope(TransactionScopeOption.Required, -- To view, visit https://gerrit.ovirt.org/41691 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If764d57a46e712aa4c6eb9f50a3d6699d27c89a8 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: Ravi Nori <rn...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
