Arik Hadas has posted comments on this change. Change subject: engine: support for adding network qos for rest-api ......................................................................
Patch Set 1: (3 comments) http://gerrit.ovirt.org/#/c/31868/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/QosValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/QosValidator.java: Line 41: /** Line 42: * Verify that the QoS entity had previously existed in the database. Line 43: */ Line 44: public ValidationResult qosExists() { Line 45: return (qos != null && getOldQos() == null) why is it important to check 'qos != null' ? can't we use the parent check? Line 46: ? new ValidationResult(VdcBllMessages.ACTION_TYPE_FAILED_QOS_NOT_FOUND) Line 47: : ValidationResult.VALID; Line 48: } Line 49: http://gerrit.ovirt.org/#/c/31868/1/backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java File backend/manager/modules/vdsbroker/src/test/java/org/ovirt/engine/core/vdsbroker/vdsbroker/SetupNetworksVDSCommandTest.java: Line 189: new NetworkQosMapper(networkStruct, VdsProperties.HOST_QOS_INBOUND, VdsProperties.HOST_QOS_OUTBOUND); Line 190: NetworkQoS deserialize = qosMapper.deserialize(); Line 191: if (deserialize != null) { Line 192: deserialize.setId(expectedQos.getId()); Line 193: } The new network QOS compares IDs in the equal method, VURTI is using the deserialize method which doesn't set the ID, so please verify that nothing is broken Line 194: assertEquals(expectedQos, deserialize); Line 195: } Line 196: Line 197: @Test http://gerrit.ovirt.org/#/c/31868/1/packaging/dbscripts/upgrade/03_06_0320_refactor_network_qos.sql File packaging/dbscripts/upgrade/03_06_0320_refactor_network_qos.sql: Line 14: please remove trailing spaces -- To view, visit http://gerrit.ovirt.org/31868 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I34f43f9edc10b7b52e096b2b6f1f43d19f129ed5 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Doron Fediuck <[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
