[Kimchi-devel] [PATCH] [WoK 9/9] Removing configobj, use file operations to edit conf files

From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; Before the implementation of the /config/plugins API, WoK used cherrypy.lib.reprconf.Parser to parse the .conf files of the plug-ins. In the implementation of this API we have the need to edit these config files, setting the 'enable = ' line from the WoK section to either True or False. Using this same parser for writing the files presented to be a problem because the writing process were deleting the comments presented in the conf files. No option is available to make the write() call of cherrypy.lib.reprconf.Parser preserve the comments. To solve this, the implementation of /config/plugins introduced configobj.ConfigObj, a python standard parser, to make this write operation in the conf files. The write() call of this parser retains the comments. So we ended up using cherrypy.lib.reprconf.Parser to parse the conf files to the plug-ins and configobj.ConfigObj to edit/write the 'enable =' option. It is worth mentioning that although cherrypy.lib.reprconf.Parser implementation extends configobj.ConfigObj, his write() call extends another class called 'backports.configparser.RawConfigParser'. This explains why the write() call from both parsers differs. A problem in this approach was detected when dealing with the conf file of the Ginger plug-in. This is the relevant portion of the Ginger conf file: [backup] default_include = ['/etc', '/var/spool/cron'] default_exclude = ['/etc/init.d', '/etc/rc.d', '/etc/rc?.d'] This declaration of default_include and default_exclude is invalid to the configobj.ConfigObj parser: Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/site-packages/configobj.py", line 1229, in __init__ self._load(infile, configspec) File "/usr/lib/python2.7/site-packages/configobj.py", line 1318, in _load raise error configobj.ConfigObjError: Parsing failed with several errors. First error at line 6. This resulted in the /enable and /disable plug-ins being broken when dealing with Ginger. After experimentation with the ginger.conf format no common ground was found between both parsers to proper handle this list declaration of the Ginger file. One solution that was speculated was to get rid of cherrypy.lib.reprconf.Parser altogether and use configobj.ConfigObj in all cases, meaning that we would use it to parse the conf files of the plug-ins too. At first only the Ginger conf file was going to be changed but then we realized that ConfigObj parses boolean in a different fashion too. This is the parsing of the current kimchi.conf file using both parsers: {'kimchi': {'create_iso_pool': 'True', 'federation': 'False'}, 'wok': {'enable': 'True'}} >>> from cherrypy.lib.reprconf import Parser >>> Parser().dict_from_file('kimchi.conf') {'kimchi': {'create_iso_pool': True, 'federation': False}, 'wok': {'enable': True}} cherrypy.lib.reprconf.Parser delivers boolean values in the dict, ConfigObj delivers string values 'True' and 'False'. This means that the change to use ConfigObj in the parsing of the conf files would need either: • change Kimchi and Gingerbase plug-ins to handle the now string values 'True' or 'False' in its backend logic or • WoK would need to convert the string values to boolean before sending it to the plug-ins. This can be annoying considering that the conf file supports multiple sections and subsections. The saner solution this patch implements is to get rid of configobj.ConfigObj to do the write of the conf files. This is now being manually made with standard Python file operations. The file editing required in this feature is too simple to warrant for all this hassle with two differents parsers, different INI formats and so on. A unit test was edited to make the .conf file a little trickier to ensure that our file operation is editing and writing the right attribute of the conf file. Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com&gt; --- src/wok/utils.py | 22 ++++++++++++++++++---- tests/test_utils.py | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/wok/utils.py b/src/wok/utils.py index d841371..c2d4724 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -35,7 +35,6 @@ import traceback import xml.etree.ElementTree as ET from cherrypy.lib.reprconf import Parser -from configobj import ConfigObj from datetime import datetime, timedelta from multiprocessing import Process, Queue from threading import Timer @@ -117,9 +116,24 @@ def set_plugin_state(name, state): if not plugin_conf: return - config = ConfigObj(plugin_conf) - config['wok']['enable'] = str(state) - config.write() + config_contents = None + + with open(plugin_conf, 'r') as f: + config_contents = f.readlines() + + wok_section_found = False + + for i in range(0, len(config_contents)): + if config_contents[i] == '[wok]\n': + wok_section_found = True + continue + + if config_contents[i].startswith('enable =') and wok_section_found: + config_contents[i] = 'enable = %s\n' % str(state) + break + + with open(plugin_conf, 'w') as f: + f.writelines(config_contents) def get_all_tabs(): diff --git a/tests/test_utils.py b/tests/test_utils.py index c0e1009..6f1a783 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -76,12 +76,17 @@ class UtilsTests(unittest.TestCase): def _get_fake_config_file_content(self, enable=True): return """\ +[a_random_section] +# a random section for testing purposes +enable = 1 + [wok] # Enable plugin on Wok server (values: True|False) enable = %s [fakeplugin] # Yet another comment on this config file +enable = 2 very_interesting_option = True """ % str(enable) -- 2.7.4