On 03/04/2012 12:20 PM, Moti Asayag wrote: > On 03/01/2012 12:54 PM, Shireesh Anjal wrote: >> Hi, >> >> In order to identify whether a cluster exposes Gluster / Virtualization >> capabilities, we plan to introduce two boolean columns - virt_service >> and gluster_service in the vds_groups table. As per immediate plans, it >> is intended to support only one service per cluster, meaning only one of >> these two values can be true. > > Couldn't there be additional future services in the future ? In that > case perhaps worth considering enum for services, to be stored in a > single service column, its values are: virt, gluster,...) instead of > extending the vds_group table consistently when introducing new services > (under the assumption no mix of services is allowed). +1 on enum idea. Even if mixture of services is allowed, we can still use this enum.
> >> >> We plan to make following changes as part of the patch: >> >> 1) Introduce an upgrade script for adding following columns to the table >> "vds_groups" >> - virt_service - boolean - NOT_NULL - default value true >> - gluster_service - boolean - NOT_NULL - default value false >> >> => What is the naming convention for naming the upgrade script? Is it >> <major-version>_<minor-version>_<running-sequence-with-step-10>_script_description.sql >> ? By that logic, the name of our script will be something like >> 03_01_0560_add_service_columns_to_vds_groups.sql > > Correct. Make sure to verify script sequence uniqueness (under > ovirt-engine/backend/manager/dbscripts/upgrade) before pushing, since > there is no enforcement for that. > >> >> 2) Modify vds_groups_sp.sql to introduce these arguments / variables in >> save and update methods >> >> 3) Modify the DAO interface and implementation to introduce these >> arguments / variables in the save/update methods and VdsGroupRawMapper. >> >> 4) Modify the DAO JUnit test class to take care of these new fields, if >> required. > > As part of DAO unit-test you should add the new columns and possible > data to > ovirt-engine/backend/manager/modules/dal/src/test/resources/fixtures.xml > >> >> 5) Modify class VDSGroup to introduce the two new boolean variables >> virtService and glusterService and modify the methods equals and >> hashCode to use these. >> >> => One question here. Most of the attributes of class VDSGroup have >> annotations like @XmlElement and @Column. I think these are related to >> jaxb and JPA. Are these annotations really required? If yes, how are >> they useful? >> > > @XmlElement annotations could be deleted from that file as a prior patch > to the patch-set. > > JPA annotations could be ignored as not fully supported nor maintained. > >> 6) REST API : Default populate these variables (virtService = true, >> glusterService = false) before invoking the BLL command >> (AddVdsGroupCommand / UpdateVdsGroupCommand). This makes sure that the >> existing API won't break. No change in api.xsd for now. >> >> 7) webadmin create cluster code: Default populate the new variables, >> same as above. No change in UI screen for now. >> >> => Alternatively, the variable declaration on VDSGroup itself can >> initialize these variables with default values, so that changes to UI / >> REST API code (points 6 and 7) may not be required immediately. What do >> you suggest? >> >> Inputs / comments / suggestions welcome. >> > > _______________________________________________ > Engine-devel mailing list > [email protected] > http://lists.ovirt.org/mailman/listinfo/engine-devel _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
