Moti Asayag has posted comments on this change. Change subject: core: Improved message when MAC Pool cannot be deleted ......................................................................
Patch Set 9: Code-Review-1 (6 comments) http://gerrit.ovirt.org/#/c/30663/9/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MacPoolValidatorTest.java: Line 38: public void setUp() throws Exception { Line 39: macPool = new MacPool(); Line 40: Line 41: final DbFacade dbFacadeMock = mock(DbFacade.class); Line 42: DbFacadeLocator.setDbFacade(dbFacadeMock); I'd refrain from using that setter. It was originally added as a hack [1] and seems that can be safely removed from DbFacadeLocator. [1] http://gerrit.ovirt.org/#/c/12385/3/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dal/dbbroker/DbFacadeLocator.java,cm Line 43: Line 44: when(dbFacadeMock.getMacPoolDao()).thenReturn(macPoolDaoMock); Line 45: when(dbFacadeMock.getStoragePoolDao()).thenReturn(storagePoolDao); Line 46: } http://gerrit.ovirt.org/#/c/30663/9/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/VdcBllMessages.java: Line 4: public enum VdcBllMessages { Line 5: Unassigned, Line 6: VAR__TYPE__HOST, Line 7: VAR__ENTITIES__HOSTS, Line 8: VAR__ENTITIES__STORAGE_POOLS, s/VAR__ENTITIES__STORAGE_POOLS/VAR__ENTITIES__DATA_CENTERS DATA_CENTERS will be proper in this context. Line 9: VAR__TYPE__VM, Line 10: VAR__ENTITIES__VMS, Line 11: VAR__TYPE__QUOTA, Line 12: VAR__TYPE__VM__CLUSTER, http://gerrit.ovirt.org/#/c/30663/9/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties File backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties: Line 8: IRS_NETWORK_ERROR=Storage Manager Service not responding. Line 9: IRS_PROTOCOL_ERROR=Storage Manager protocol error. Line 10: IRS_RESPONSE_ERROR=Storage Manager response error. Line 11: MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. Line 12: ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${ENTITIES_USING_NETWORK_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again. shouldn't it be DATACENTERS_USING_MAC_POOL_COUNTER instead of ENTITIES_USING_NETWORK_COUNTER Line 13: VAR__ENTITIES__STORAGE_POOLS=$entities Data Centers Line 14: ACTION_TYPE_FAILED_CANNOT_REMOVE_DEFAULT_MAC_POOL=Cannot ${action} ${type}. Default ${type} cannot be removed. Line 15: ACTION_TYPE_FAILED_MAC_POOL_DOES_NOT_EXIST=Cannot ${action} ${type}. ${type} does not exist. Line 16: ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Changing default ${type} is not supported. Line 9: IRS_PROTOCOL_ERROR=Storage Manager protocol error. Line 10: IRS_RESPONSE_ERROR=Storage Manager response error. Line 11: MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES=Not enough MAC addresses left in MAC Address Pool. Line 12: ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${ENTITIES_USING_NETWORK_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again. Line 13: VAR__ENTITIES__STORAGE_POOLS=$entities Data Centers s/VAR__ENTITIES__STORAGE_POOLS/VAR__ENTITIES__DATA_CENTERS also in the rest of the relevent files in this patch. Line 14: ACTION_TYPE_FAILED_CANNOT_REMOVE_DEFAULT_MAC_POOL=Cannot ${action} ${type}. Default ${type} cannot be removed. Line 15: ACTION_TYPE_FAILED_MAC_POOL_DOES_NOT_EXIST=Cannot ${action} ${type}. ${type} does not exist. Line 16: ACTION_TYPE_FAILED_CHANGING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Changing default ${type} is not supported. Line 17: ACTION_TYPE_FAILED_SETTING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Setting default ${type} is not supported. http://gerrit.ovirt.org/#/c/30663/9/backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java File backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/StoragePoolDAOTest.java: Line 353: Line 354: @Test Line 355: public void testGetAllDataCentersByMacPoolId() { Line 356: assertThat(dao.getAllDataCentersByMacPoolId(FixturesTool.DEFAULT_MAC_POOL_ID).size(), is(6)); Line 357: } would you like to add a test for a non-existing mac pool id ? Line 358: Line 359: private void assertStorageTypes(List<StorageType> typesList, StorageType... expectedTypes) { Line 360: assertEquals("Number of storage types different than expected storage type in the pool", typesList.size(), expectedTypes.length); Line 361: http://gerrit.ovirt.org/#/c/30663/9/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java File frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java: Line 36: Line 37: @DefaultStringValue("Not enough MAC addresses left in MAC Address Pool.") Line 38: String MAC_POOL_NOT_ENOUGH_MAC_ADDRESSES(); Line 39: Line 40: @DefaultStringValue("ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=Cannot ${action} ${type}. Several ${entities} (${ENTITIES_USING_NETWORK_COUNTER}) are using this ${type}:\n${DATACENTERS_USING_MAC_POOL}\n - Please remove it from all ${entities} that are using it and try again.") the prefix "ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL=" should not be part of the message in this context. Line 41: String ACTION_TYPE_FAILED_CANNOT_REMOVE_STILL_USED_MAC_POOL(); Line 42: Line 43: @DefaultStringValue("$entities Data Centers") Line 44: String VAR__ENTITIES__STORAGE_POOLS(); -- To view, visit http://gerrit.ovirt.org/30663 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I647420d7cc43853eddd2b17091cd627b9088a34d Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Martin Mucha <mmu...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
