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()). -- Endi S. Dewata