Alona Kaplan has posted comments on this change. Change subject: engine: Implement HostNetworkQosValidator ......................................................................
Patch Set 3: (3 comments) http://gerrit.ovirt.org/#/c/34123/3/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/host/SetupNetworksHelper.java: Line 257: } Line 258: Line 259: NetworkQosValidator qosValidator = new NetworkQosValidator(iface.getQos()); Line 260: if (qosValidator.requiredValuesPresent() != ValidationResult.VALID) { Line 261: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES, Why are you overriding the error message returned from the validator? Is there any other use case for the HostNetworkQosValidator except here? If not, the validator should contain the correct message (i.e ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES) Line 262: iface.getNetworkName()); Line 263: } Line 264: if (qosValidator.peakConsistentWithAverage() != ValidationResult.VALID) { Line 265: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_INCONSISTENT_VALUES, Line 261: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_MISSING_VALUES, Line 262: iface.getNetworkName()); Line 263: } Line 264: if (qosValidator.peakConsistentWithAverage() != ValidationResult.VALID) { Line 265: addViolation(VdcBllMessages.ACTION_TYPE_FAILED_HOST_NETWORK_QOS_SETUP_NETWORKS_INCONSISTENT_VALUES, You're still using the NetworkQosValidator (and not HostNetworkQosValidator) but the error regards to HostNetworkQosValidator.valuesConsistent(). Please rearrange your patches so adding HostNetworkQos to VdsNetworkInterface (instead of NetworkQos) would be in one of the first patches- before you change logic which is based on it. You can first just add HostNetworkQos to VdsNetworkInterface (without removing NetworkQos) and just after you"ll fix all the related logic, to remove NetworkQos. Line 266: iface.getNetworkName()); Line 267: } Line 268: } Line 269: } http://gerrit.ovirt.org/#/c/34123/3/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/HostNetworkQosValidatorTest.java File backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/validator/HostNetworkQosValidatorTest.java: Line 24: private HostNetworkQosValidator nullValidator; Line 25: Line 26: @Before Line 27: public void setup() { Line 28: qos = mock(HostNetworkQos.class); You can add @RunWith(MockitoJUnitRunner.class) to the class, and then you"ll be able to use @mock on the declaration instead of this initialization. But it is just a matter of style, so as you wish. Line 29: validator = new HostNetworkQosValidator(qos); Line 30: nullValidator = new HostNetworkQosValidator(null); Line 31: } Line 32: -- To view, visit http://gerrit.ovirt.org/34123 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibf156e27381111efb3d9abac3e6e37ccdd8f7643 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
