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
Engine-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel