Moti Asayag has posted comments on this change.

Change subject: db, core: aggregate qos and storage qos impl
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.ovirt.org/#/c/27094/8/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/QosBaseDaoFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/qos/QosBaseDaoFacadeImpl.java:

Line 22:     /**
Line 23:      * @param map
Line 24:      * @param obj - derived entity (T)
Line 25:      */
Line 26:     protected abstract void 
updatePartialParametersMapper(MapSqlParameterSource map, T obj);
this method doesn't required.

you can have a method to expose the base parameters source, and call it from 
all of the successors, i.e.:

    @Override
    protected abstract MapSqlParameterSource createBaseParametersMapper(T obj) {
        MapSqlParameterSource map = createIdParameterMapper(obj.getId())
                .addValue("qos_type", getQosType())
                .addValue("name", obj.getName())
                .addValue("description", obj.getDescription())
                .addValue("storage_pool_id", obj.getStoragePoolId());
        return map;
    }

and in the successors, you should use it:

@override
protected MapSqlParameterSource createFullParametersMapper(T obj) {
        MapSqlParameterSource map = createBaseParametersMapper(T obj);
        map.addValue("max_throughput", obj.getMaxThroughput());
        map.addValue("max_read_throughput", obj.getMaxReadThroughput());
        map.addValue("max_write_throughput", obj.getMaxWriteThroughput());
        map.addValue("max_iops", obj.getMaxIops());
        map.addValue("max_read_iops", obj.getMaxReadIops());
        map.addValue("max_write_iops", obj.getMaxWriteIops());
    return map;
}
Line 27: 
Line 28:     /**
Line 29:      * @return qos type for derived qos dao
Line 30:      */


Line 41:                 createEntityRowMapper(),
Line 42:                 parameterSource);
Line 43:     }
Line 44: 
Line 45:     @Override
per comment above, this could either be removed or changed to abstract.
Line 46:     protected MapSqlParameterSource createFullParametersMapper(T obj) {
Line 47:         MapSqlParameterSource map = 
createIdParameterMapper(obj.getId())
Line 48:                 .addValue("qos_type", getQosType())
Line 49:                 .addValue("name", obj.getName())


Line 59:         return getCustomMapSqlParameterSource()
Line 60:                 .addValue("id", guid);
Line 61:     }
Line 62: 
Line 63:     protected static abstract class QosBaseDaoFacadaeImplMapper<T 
extends QosBase> implements RowMapper<T> {
using T as type confuses with the type T on the class level.
Line 64:         @Override
Line 65:         public T mapRow(ResultSet rs, int rowNum) throws SQLException {
Line 66:             T entity = createQosEntity(rs);
Line 67:             entity.setId(getGuid(rs, "id"));


http://gerrit.ovirt.org/#/c/27094/8/packaging/dbscripts/upgrade/03_05_0610_qos_and_storage_impl.sql
File packaging/dbscripts/upgrade/03_05_0610_qos_and_storage_impl.sql:

Line 7:   id uuid NOT NULL,
Line 8:   qos_type SMALLINT NOT NULL,
Line 9:   name VARCHAR(50) NOT NULL,
Line 10:   description TEXT,
Line 11:   storage_pool_id uuid NOT NULL,
what about renaming the columns to data_center_id ?
Line 12:   max_throughput INTEGER,
Line 13:   max_read_throughput INTEGER,
Line 14:   max_write_throughput INTEGER,
Line 15:   max_iops INTEGER,


-- 
To view, visit http://gerrit.ovirt.org/27094
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1a9af59277b5055453159f002f19046c0051d63b
Gerrit-PatchSet: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Gilad Chaplik <gchap...@redhat.com>
Gerrit-Reviewer: Kobi Ianko <k...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@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

Reply via email to