On 03/27/2012 08:18 AM, Itamar Heim wrote: > On 03/26/2012 09:53 PM, Saggi Mizrahi wrote: >> 1. The column listing value is reloadable is redundant. Whether or not >> a value is "reloadable" or not is defined in the actual Engine code. >> Values that are currently not "reloadable" might become reloadable in >> the future. Putting this is the db just means you have to sync between >> the engine logic and the DB. I think this value should be obtained >> from the Engine dynamically and not persisted to disk. >> 2. Why don't configuration changes happen only through the Engine REST >> API? That way you always know when values change and you don't have to >> periodically reload. For cases where someone want's to change the >> values bypassing the API just make them use `service ovirt-engine >> reload` like every other daemon. >> 3. Values that cannot be refreshed without a restart will print a >> warning wither in the API response or to stderr for the service call. >> something in the form of: >> WARNING option XXXXX changed a reboot is required for it to take affect. >> 4. I also don't see why you write the data type to the DB (config >> type, column type). It's not like you are saving the values in binary >> format. It's just another case when this information is already
Agreed. Current code in engine-core uses a generic method to get the value, and uses @TypeConverterAttribute annotation to give indication to which type the data should be "cast" to. See DbConfigUtils.parseValue(String value, String name, java.lang.Class<?> fieldType) >> written in code anyway and when you change it in code you will just >> have to do the book keeping and needlessly change it again in the DB. >> As a rule try and not use db values to document the code. This is what >> documentation is for. > > a different take on this - why not just annotate the enums with this > information? +1 On annotation idea - although this introduces "information on behavior" to code, and any change will require re-compiling, I don't see why anyone would like to change the behavior. > also, not sure configuration should be reloaded per change > automatically. maybe user wants it to happen on next restart, maybe some > changes must happen together. > i.e., I'd expect a specific 'reconfigure' call to make it happen rather > than poll for it. > >> These are things I'm not against I just want more explanation about: >> 1. Why even put the configuration in the DB? Data bases are >> notoriously strict and once you have schema you are committed to it. >> Having a regular config file might be simpler for users to edit and >> for you to maintain. The only reasons to use a DB as opposed to a flat >> file are ACID and scaleability. None of these are actually important >> for configuration. >> 2. The definition to what might not be "reloadable" seem arbitrary to >> me (eg. "Quartz services that are setup on startup and cannot be >> changed afterwords."). I admit I don't really know what Quartz >> services are, but I don't imagine it's impossible to write one that >> has at least one "reloadable" configuration value. Muli - are you sure about Quartz? Have you checked it out? >> >> ----- Original Message ----- >>> From: "Muli Salem"<[email protected]> >>> To: [email protected] >>> Sent: Monday, March 26, 2012 1:10:10 PM >>> Subject: [Engine-devel] Reloadable Configuration - Wiki Page >>> >>> Hi All, >>> >>> Below please find a wiki page regarding the design of the Reloadable >>> Configuration feature. >>> You are more than welcome to review and comment. >>> >>> http://www.ovirt.org/wiki/Features/ReloadableConfiguration >>> >>> Thanks, >>> Muli >>> _______________________________________________ >>> 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 > > _______________________________________________ > 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
