Yevgeny Zaspitsky has posted comments on this change. Change subject: engine: replace NetworkUtils.getDefaultManagementNetwork calls ......................................................................
Patch Set 15: (5 comments) http://gerrit.ovirt.org/#/c/34243/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkCommand.java: Line 301: return ValidationResult.VALID; Line 302: } Line 303: Line 304: public ValidationResult notRenamingManagementNetwork() { Line 305: return managementNetworkUtil.isManagementNetwork(network.getId()) > You will throw a validation error in any case management network is updated Done Line 306: ? new ValidationResult(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK) Line 307: : ValidationResult.VALID; Line 308: } Line 309: http://gerrit.ovirt.org/#/c/34243/15/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/UpdateNetworkToVdsInterfaceCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/UpdateNetworkToVdsInterfaceCommand.java: Line 227: getReturnValue().getCanDoActionMessages() Line 228: .add(VdcBllMessages.NETWORK_DEFAULT_UPDATE_NAME_INVALID.toString()); Line 229: final Network managementNetwork = managementNetworkUtil.getManagementNetwork(clusterId); Line 230: getReturnValue().getCanDoActionMessages().add(String.format("$NetworkName %1$s", Line 231: managementNetwork.getName())); > You can use- getParameters().getOldNetworkName(), no need to query the db. Done Line 232: return false; Line 233: } Line 234: Line 235: if (StringUtils.isNotEmpty(getParameters().getGateway())) { http://gerrit.ovirt.org/#/c/34243/15/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/network/dc/UpdateNetworkValidatorTest.java: Line 131: } Line 132: Line 133: @Test Line 134: public void managementNetworkRenamed() { Line 135: when(managementNetworkUtil.isManagementNetwork(NETWORK_ID)).thenReturn(true); > Notice there is a comment about the renaming in the commands- please rewrit Done Line 136: Line 137: assertThat(validator.notRenamingManagementNetwork(), Line 138: failsWith(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK)); Line 139: } Line 138: failsWith(VdcBllMessages.NETWORK_CAN_NOT_REMOVE_DEFAULT_NETWORK)); Line 139: } Line 140: Line 141: @Test Line 142: public void managementNetworkNotRenamed() { > same. Done Line 143: when(managementNetworkUtil.isManagementNetwork(NETWORK_ID)).thenReturn(false); Line 144: Line 145: assertThat(validator.notRenamingManagementNetwork(), isValid()); Line 146: } http://gerrit.ovirt.org/#/c/34243/15/backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java File backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/NetworkUtils.java: Line 27: * majority of the cases. Line 28: * Line 29: * @return the default management network name. Line 30: */ Line 31: public static String getDefaultManagementNetworkName() { > I would move this method to ManagementNetowrkUtil Done in a separate commit Line 32: return Config.<String> getValue(ConfigValues.DefaultManagementNetwork); Line 33: } Line 34: Line 35: public static Integer getDefaultMtu() { -- To view, visit http://gerrit.ovirt.org/34243 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib0bd50e16b577b3f31765011d9b16501149c2479 Gerrit-PatchSet: 15 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@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
