On 3/4/2013 10:03 PM, Abhishek Koneru wrote: > Please review the patch attached for fixing the ticket 493 in Dogtag > 10.0.2. > > Tested in both in both interactive and file-passing (-f option) of > pkispawn. > > --Abhishek Some comments: 1. In set_property() the new code checks whether the new property is sensitive. If it is the value will be added directly into the dict, otherwise it will use the ConfigParser.set(). This code might not be necessary. To my understanding the interpolation will be evaluated during retrieval only (in get() and items()), so changing how the value is stored would not affect the interpolation. 2. In the new code the sensitive_parameters will be parsed each time flatten_master_dict() is called. It might be better to parse the list once right after the file is loaded in read_pki_configuration_file(), and store the parsed value as an attribute so it can be used by other methods without parsing it again. 3. The list_items() could be simplified. It's not necessary to merge the section with the default section because interpolation only works in the same section. I think you could call ConfigParser.items(section, True) to get all (param, raw_value) pairs. If the parameter is not sensitive then call ConfigParser.get(section, param) to get the interpolated value and use it to replace the raw value. Then either return all tuples as a list like the original items() or put them in a dict like in your patch. 4. What does the 'list' in list_items() mean? It might be better to use the same name as the original method it's trying to replace (e.g. items()).

On discussion with Endi and Ade, it has been agreed to use an escape('%') character to allow usage of % in the values of the configuration file. If a configuration file is passed to the pkispawn script, any occurrence of '%' must have a '%' used with it as an escape character. Eg. key=abc%def, must be entered as key=abc%%def => which is read as abc %def. Similarly, key=abc%%def must be key=abc%%%%def I the interactive mode, the escape character is added by the code. User can enter the actual value without the escape character. Please review the attached patch attached for with the required changes.