Re: [Engine-devel] Using config values

----- Original Message ----- > From: "Omer Frenkel" <ofrenkel(a)redhat.com&gt; > To: "Eli Mesika" <emesika(a)redhat.com&gt; > Cc: "Dusmant Kumar Pati" <dpati(a)redhat.com&gt;, "engine-devel" <engine-devel(a)ovirt.org&gt; > Sent: Sunday, December 1, 2013 11:02:46 AM > Subject: Re: [Engine-devel] Using config values > > > > ----- Original Message ----- >> From: "Eli Mesika" <emesika(a)redhat.com&gt; >> To: "Dusmant Kumar Pati" <dpati(a)redhat.com&gt; >> Cc: "engine-devel" <engine-devel(a)ovirt.org&gt; >> Sent: Saturday, November 30, 2013 10:58:35 PM >> Subject: Re: [Engine-devel] Using config values >> >> >> >> ----- Original Message ----- >>> From: "Dusmant Kumar Pati" <dpati(a)redhat.com&gt; >>> To: "Kanagaraj" <kmayilsa(a)redhat.com&gt;, "engine-devel" >>> <engine-devel(a)ovirt.org&gt; >>> Sent: Friday, November 29, 2013 1:40:09 PM >>> Subject: Re: [Engine-devel] Using config values >>> >>> On 11/29/2013 01:46 PM, Kanagaraj wrote: >>> >>> >>> Hi All, >>> >>> The are some issues arising in configurations whenever we move up on the >>> versions(3.3 => 3.4), because of the way we store and interpret them. >>> >>> Whenever there is a new cluster level, you will need to add a new entry >>> for >>> all(most) of the configuration. Mostly a copy paste if you see from 3.2 >>> to >>> 3.3, except some CPU/PM type related configurations. >>> Better option would be to have the defaul config value in >>> ConfigValues.java >>> and the overrides will go to config.sql. In this approach you don't need >>> a >>> new entries to config.sql when there is a new cluster level. >>> >>> Lets take an exmaple, "SupportForceCreateVG" - This is supported from 3.1 >>> onwards, >>> >>> If you look at config.sql, you will see following entries >>> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); >>> select fn_db_add_config_value('SupportForceCreateVG','true','3.1'); >>> select fn_db_add_config_value('SupportForceCreateVG','true','3.2'); >>> select fn_db_add_config_value('SupportForceCreateVG','true','3.3'); >>> >>> And in ConfigValues.java >>> >>> @TypeConverterAttribute(Boolean.class) >>> @DefaultValueAttribute("false") >>> SupportForceCreateVG, >>> >>> Now if there is 3.4 and 3.5, the user needs to add 2 more entries, which >>> i >>> feel is redundant. >>> >>> Instead we can make >>> >>> @TypeConverterAttribute(Boolean.class) >>> @DefaultValueAttribute("true") >>> SupportForceCreateVG, >>> >>> and have only the following in config.sql >>> select fn_db_add_config_value('SupportForceCreateVG','false','3.0'); >>> >>> if a particular value(for a specific cluster level) is not found in >>> Config.sql, the fallback is to use the value available in >>> ConfigValues.java. >>> >>> Please share your thoughts on this. >> >> Hi >> >> First of all I think its a good idea >> I have 2 questions >> >> 1) Which value will be stored as default in the java class for >> configuration >> values that are not a boolean, that represents if a feature is active or >> not. >> Is that the latest version value ? sounds not obvious to me >> > > i guess this will have to have a configuration values for each version in the > db. > >> 2) There are some configuration values that are exposed to the user via the >> engine-config tool, how this will work, we can not remove the entries their >> since the user may change and override those values. >> > > in your suggestion below, there is the same issue, > if user want to change the 3.3 value, engine config will fail with "No such > entry" > because 3.3 will not be in the db.. > so if we are not fixing this, i think using the current implementation is > good enough, In this case engine-config will add this key with the given value for that version, so , I see no problem in that... In addition the getConfigValue will lookup for version matching only the first time, so , if it was given a 3.4 version for key K and found a matching only for 3.2 , it will add to the cache K with the same value for version 3.4