Liron Ar has posted comments on this change.

Change subject: core: support for querying connections of domains in given 
statuses
......................................................................


Patch Set 1:

(4 comments)

http://gerrit.ovirt.org/#/c/27521/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAO.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAO.java:

Line 83:      * @param statuses
Line 84:      *          the applicable statuses
Line 85:      * @return the list of connections
Line 86:      */
Line 87:     public List<StorageServerConnections> 
getConnectableStorageConnectionsByStorageTypeAndStatus(Guid pool, StorageType 
storageType,
> I don't think that the name of the method is accurate.
Done
Line 88:                                                                        
                          Set<StorageDomainStatus> statuses);
Line 89:     /**
Line 90:      * Retrieves all connections for the specified volume group.
Line 91:      *


Line 84:      *          the applicable statuses
Line 85:      * @return the list of connections
Line 86:      */
Line 87:     public List<StorageServerConnections> 
getConnectableStorageConnectionsByStorageTypeAndStatus(Guid pool, StorageType 
storageType,
Line 88:                                                                        
                          Set<StorageDomainStatus> statuses);
> Looks weird in gerrit - please check it's just a gerrit foobar.
Done
Line 89:     /**
Line 90:      * Retrieves all connections for the specified volume group.
Line 91:      *
Line 92:      * @param group


http://gerrit.ovirt.org/#/c/27521/1/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java
File 
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/StorageServerConnectionDAODbFacadeImpl.java:

Line 69:         List<String> statusesVals = new LinkedList<>();
Line 70:         for (StorageDomainStatus status : statuses) {
Line 71:             statusesVals.add(Integer.toString(status.getValue()));
Line 72:         }
Line 73:         return 
getCallsHandler().executeReadList("GetConnectableStorageConnectionsByStorageType",
> The name of the sp should also be changed same as the API
Done
Line 74:                 mapper,
Line 75:                 getCustomMapSqlParameterSource()
Line 76:                         .addValue("storage_pool_id", pool)
Line 77:                         .addValue("storage_type", (storageType != 
null) ? storageType.getValue() : null)


http://gerrit.ovirt.org/#/c/27521/1/packaging/dbscripts/storages_san_sp.sql
File packaging/dbscripts/storages_san_sp.sql:

Line 530: LANGUAGE plpgsql;
Line 531: 
Line 532: 
Line 533: 
Line 534: Create or replace FUNCTION 
GetConnectableStorageConnectionsByStorageType(v_storage_pool_id UUID, 
v_storage_type integer, v_statuses varchar(10))
> 1) I think it is better to use a bigger size for the v_statuses, since this
Done
Line 535: RETURNS SETOF storage_server_connections STABLE
Line 536:    AS $procedure$
Line 537: DECLARE
Line 538:   statuses int[];


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie05c371b37fb0cb2c723e6aee3d416abd0d8eb20
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@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