[Kimchi-devel] [PATCH] [WoK 09/10] Removing configobj, use file operations to edit conf files
Daniel Henrique Barboza
danielhb at linux.vnet.ibm.com
Wed Jan 25 19:03:09 UTC 2017
On 01/25/2017 03:33 PM, Aline Manera wrote:
>
> So you have created a function that does not work well on patch 2/10
> to rewrite it again here?
Yeah, that was exactly the reason I created this commit.
...
I'll merge all backend patches in a single patch in v3.
>
> On 01/20/2017 03:32 PM, dhbarboza82 at gmail.com wrote:
>> From: Daniel Henrique Barboza <danielhb at linux.vnet.ibm.com>
>>
>> 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:
>>
>>>>> import configobj
>>>>> parser = configobj.ConfigObj('ginger.conf')
>> 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:
>>
>>>>> import configobj
>>>>> parser = configobj.ConfigObj('kimchi.conf')
>>>>> parser.dict()
>> {'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 at linux.vnet.ibm.com>
>> ---
>> 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)
>>
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
More information about the Kimchi-devel
mailing list