Alona Kaplan has posted comments on this change. Change subject: engine: Add DefaultManagementNetworkFinder ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/33275/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinder.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinder.java: Line 9: * Finds the default management network for the given DC. Line 10: * Line 11: * @param dataCenterId Line 12: * data center id Line 13: * @return Please complete the return statement doc. Line 14: */ Line 15: Network findDefaultManagementNetwork(Guid dataCenterId); Line 16: http://gerrit.ovirt.org/#/c/33275/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinderImpl.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinderImpl.java: Line 23: this.networkDao = networkDao; Line 24: } Line 25: Line 26: @Override Line 27: public Network findDefaultManagementNetwork(Guid dataCenterId) { I think the interface java doc is not informative enough. Please expand it with explanation of how the default network is chosen. Line 28: final Network defaultEngineManagementNetwork = findDefaultEngineManagementNetwork(dataCenterId); Line 29: if (defaultEngineManagementNetwork != null) { Line 30: return defaultEngineManagementNetwork; Line 31: } Line 36: } Line 37: return null; Line 38: } Line 39: Line 40: private Network findDefaultEngineManagementNetwork(Guid dataCenterId) { maybe findConfigDefaultManagementNetwork? Line 41: return networkDao.getByNameAndDataCenter(NetworkUtils.getDefaultManagementNetworkName(), Line 42: dataCenterId); Line 43: } http://gerrit.ovirt.org/#/c/33275/6/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinderImplTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/cluster/DefaultManagementNetworkFinderImplTest.java: Line 28: Line 29: private static final Guid TEST_DC_ID = Guid.Empty; Line 30: Line 31: @Mock Line 32: private IConfigUtilsInterface mockConfigUtils; Please use MockConfigRule, it already contains to logic for mocking config values. You don't need to re-implement it. You can see- NetworkUtilsTest for example of how to use it. Line 33: @Mock Line 34: private NetworkDao mockNetworkDao; Line 35: @Mock Line 36: private Network mockNetwork; -- To view, visit http://gerrit.ovirt.org/33275 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7f8f0d53096523414ceb339d1b43224b540e0f78 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: Alona Kaplan <[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
