Gilad Chaplik has posted comments on this change. Change subject: core: Add CRUD commands and queries for QoS objects (storage) ......................................................................
Patch Set 6: (6 comments) patch will be uploaded soon. http://gerrit.ovirt.org/#/c/27096/6/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddStorageQosCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/qos/AddStorageQosCommand.java: Line 18: } Line 19: Line 20: @Override Line 21: protected QosValidator<StorageQos> getQosValidator(StorageQos qos) { Line 22: return new QosValidator<StorageQos>(qos) { > how do you plan to add unit-tests for an anonymous class ? Done Line 23: Line 24: @Override Line 25: protected QosDao<StorageQos> getQosDao() { Line 26: return AddStorageQosCommand.this.getQosDao(); Line 22: return new QosValidator<StorageQos>(qos) { Line 23: Line 24: @Override Line 25: protected QosDao<StorageQos> getQosDao() { Line 26: return AddStorageQosCommand.this.getQosDao(); > why prefix "AddStorageQosCommand.this." required ? Done Line 27: } Line 28: Line 29: @Override Line 30: public ValidationResult allValuesPresent() { http://gerrit.ovirt.org/#/c/27096/6/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 8: import org.ovirt.engine.core.common.errors.VdcBllMessages; Line 9: import org.ovirt.engine.core.dao.qos.QosDao; Line 10: Line 11: public abstract class QosValidator<T extends BaseQos> { Line 12: > please add unit-tests. in a later patch (will add the link later) Line 13: private final T qos; Line 14: private T oldQos; Line 15: private List<T> allQos; Line 16: Line 68: Line 69: /** Line 70: * Verify that a QoS entity's name hasn't changed. This assumes that QoS entity has been verified to exist. Line 71: */ Line 72: public ValidationResult nameNotChangedOrNotTaken() { > couldn't found usages for this method. Done Line 73: if (!getOldQos().getName().equals(qos.getName())) { Line 74: return nameNotTakenInDc(); Line 75: } else { Line 76: return ValidationResult.VALID; http://gerrit.ovirt.org/#/c/27096/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/AuditLogType.java: Line 957: USER_FAILED_TO_REMOVE_NETWORK_QOS(10103, AuditLogSeverity.ERROR), Line 958: USER_UPDATED_NETWORK_QOS(10104), Line 959: USER_FAILED_TO_UPDATE_NETWORK_QOS(10105, AuditLogSeverity.ERROR), Line 960: Line 961: // Qos > the ids of the audit-log types identifies the entries type in the event log you have another good comment somewhere else bout logging. will open a general bug to enhance logging for QoS (and include both points). Line 962: USER_ADDED_QOS(10100), Line 963: USER_FAILED_TO_ADD_QOS(10101, AuditLogSeverity.ERROR), Line 964: USER_REMOVED_QOS(10102), Line 965: USER_FAILED_TO_REMOVE_QOS(10103, AuditLogSeverity.ERROR), http://gerrit.ovirt.org/#/c/27096/6/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/VdcQueryType.java: Line 315: GetAllProviders, Line 316: GetAllNetworksForProvider, Line 317: Line 318: //Network QoS Line 319: GetAllNetworkQosByStoragePoolId, > please add a space line Done Line 320: // QoS Line 321: GetQosById, Line 322: GetAllQosByStoragePoolIdAndType, Line 323: -- To view, visit http://gerrit.ovirt.org/27096 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c00522ddb Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Doron Fediuck <[email protected]> Gerrit-Reviewer: Gilad Chaplik <[email protected]> Gerrit-Reviewer: Kobi Ianko <[email protected]> Gerrit-Reviewer: Liron Ar <[email protected]> Gerrit-Reviewer: Moti Asayag <[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
