Maor Lipchuk has posted comments on this change. Change subject: core: support for querying connections of domains in given statuses ......................................................................
Patch Set 1: (3 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. You are fetching StorageServerConnections by status, it does not mean that they are connectable. The name should be changed to getStorageConnectionsByStorageTypeAndStatus Line 88: Set<StorageDomainStatus> statuses); Line 89: /** Line 90: * Retrieves all connections for the specified volume group. Line 91: * 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 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 is also includes comma. Potentially you can use more then 6 statuses. 2) The name of the SP should also be changed to indicate status as so: GetConnectableStorageConnectionsByStorageTypeAndStatus 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: 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
