Sahina Bose has posted comments on this change.
Change subject: engine: Get Services Query
......................................................................
Patch Set 3: (10 inline comments)
Minor comments inline
....................................................
File backend/manager/dbscripts/services_sp.sql
Line 5: - services
Line 6: - service_config
Line 7: ----------------------------------------------------------------*/
Line 8:
Line 9:
Should this return a set of parent services only?
Line 10: Create or replace FUNCTION GetAllowedServices()
Line 11: RETURNS SETOF services_master
Line 12: AS $procedure$
Line 13: BEGIN
....................................................
File backend/manager/dbscripts/upgrade/03_02_0400_add_services_table.sql
Line 2: AS $procedure$
Line 3: BEGIN
Line 4:
Line 5: -- Add services master table
Line 6:
May need a sequence column to decide the priority sequence when executing
multiple services
Line 7: IF (NOT EXISTS (SELECT 1 FROM INFORMATION_SCHEMA.TABLES WHERE
table_name='services_master')) THEN
Line 8: BEGIN
Line 9: CREATE TABLE services_master
Line 10: (
Line 37: END IF;
Line 38:
Line 39: DROP INDEX IF EXISTS IDX_services_server_id;
Line 40: CREATE INDEX IDX_services_server_id ON services(server_id);
Line 41: DROP INDEX IF EXISTS IDX_service_unique;
Unique index should be on (server_id,service_master_id)
Line 42: CREATE UNIQUE INDEX IDX_service_unique ON services(server_id, id);
Line 43:
Line 44: -- Add service config table
Line 45: IF (NOT EXISTS (SELECT 1 FROM INFORMATION_SCHEMA.TABLES WHERE
table_name='service_config')) THEN
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/ServiceConfigEntity.java
Line 10: /**
Line 11: * The service config entity.
Line 12: *
Line 13: * @see ServiceEntity
Line 14: */
Should this extend IVdcQueryable? Service configuration options could be
searchable as well.
Line 15: public class ServiceConfigEntity implements Serializable {
Line 16:
Line 17: private static final long serialVersionUID = -585458847720355956L;
Line 18:
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/ServiceEntity.java
Line 28:
Line 29: @NotNull(message = "VALIDATION.SERVICE.SERVER_ID.NOT_NULL", groups
= { CreateEntity.class })
Line 30: private Guid serverId;
Line 31:
Line 32: private Guid serviceId;
A validation on serviceId not null too?
Line 33:
Line 34: private Integer pid;
Line 35:
Line 36: private ServiceStatus status;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/ServiceMasterEntity.java
Line 17: private Guid id;
Line 18:
Line 19: private String serviceName;
Line 20:
Line 21: private Guid parentService;
Maybe a reference to the parentService as ServiceMasterEntity could be useful?
Line 22:
Line 23:
Line 24: public ServiceMasterEntity() {
Line 25: }
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/gluster/ServiceStatus.java
Line 2:
Line 3: /**
Line 4: * Enum for the Service Status.
Line 5: * @see ServiceEntity
Line 6: */
Is NOTAVAILABLE - as a ServiceStatus required to indicate this service is not
installed on a node?
Line 7: public enum ServiceStatus {
Line 8: RUNNING,
Line 9: STOPPED,
Line 10: FAILED;
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/queries/gluster/ServicesQueryParameters.java
Line 4:
Line 5: /**
Line 6: * Parameter class with cluster Id and serverId as parameters. <br>
Line 7: * This will be used by get services query command. <br>
Line 8: */
Is Service Id (service master id) also required as part of this
Line 9: public class ServicesQueryParameters extends GlusterParameters {
Line 10:
Line 11: private static final long serialVersionUID = -8536790613494079492L;
Line 12: private Guid serverId;
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/gluster/ServicesDaoDbFacadeImpl.java
Line 107:
getCustomMapSqlParameterSource().addValue("service_id",
service.getServiceId())));
Line 108: }
Line 109: }
Line 110:
Line 111:
Minor - can rename to fetchServiceConfigEntities
Line 112: private void fetchServiceConfigEntitys(List<ServiceEntity>
services) {
Line 113: for (ServiceEntity service : services) {
Line 114: fetchServiceConfigEntity(service);
Line 115: }
....................................................
File
backend/manager/modules/dal/src/test/java/org/ovirt/engine/core/dao/gluster/ServicesDaoTest.java
Line 60: ServiceEntity service = dao.getById(newService.getId());
Line 61: assertNotNull(service);
Line 62: assertEquals(newService, service);
Line 63: }
Line 64:
Should also test if service config list is returned correctly in the service
Line 65: @Test
Line 66: public void testGetById() {
Line 67: ServiceEntity service = dao.getById(EXISTING_SERVICE_ID);
Line 68:
--
To view, visit http://gerrit.ovirt.org/11147
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I7982faf180f8580838a962a5674e8e9e8982679b
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dhandapani Gopal <[email protected]>
Gerrit-Reviewer: Dhandapani Gopal <[email protected]>
Gerrit-Reviewer: Sahina Bose <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
