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