Re: [Kimchi-devel] [project-kimchi] [PATCH 1/4] Move configuration parsing to config.py

On 12/18/2013 06:31 PM, Royce Lv wrote: > On 2013年12月18日 16:51, Mark Wu wrote: >> The configruation is also needed for other code except starting kimchi >> server. So it should be moved to a separate module, config.py. Then the >> configuration can be accessed directly by importing config module. >> >> Signed-off-by: Mark Wu <wudxw(a)linux.vnet.ibm.com&gt; >> --- >> src/kimchi/config.py.in | 23 ++++++++++++++++++++--- >> src/kimchid.in | 20 +------------------- >> 2 files changed, 21 insertions(+), 22 deletions(-) >> >> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in >> index f3c408a..49d42db 100644 >> --- a/src/kimchi/config.py.in >> +++ b/src/kimchi/config.py.in >> @@ -25,11 +25,9 @@ >> import libvirt >> import os >> import platform >> - >> - >> +from ConfigParser import SafeConfigParser >> from glob import iglob >> >> - >> from kimchi.xmlutils import xpath_get_text >> >> >> @@ -166,5 +164,24 @@ def get_plugin_tab_xml(name): >> return os.path.join(_get_plugin_ui_dir(name), >> 'config/tab-ext.xml') >> >> >> +CONFIG_FILE = "%s/kimchi.conf" % get_config_dir() >> +DEFAULT_LOG_LEVEL = "debug" >> + >> +config = SafeConfigParser() >> +config.add_section("server") >> +config.set("server", "host", "0.0.0.0") >> +config.set("server", "port", "8000") >> +config.set("server", "ssl_port", "8001") >> +config.set("server", "ssl_cert", "") >> +config.set("server", "ssl_key", "") >> +config.set("server", "environment", "development") >> +config.add_section("logging") >> +config.set("logging", "log_dir", get_default_log_dir()) >> +config.set("logging", "log_level", DEFAULT_LOG_LEVEL) >> + >> +if os.path.exists(CONFIG_FILE): >> + config.read(CONFIG_FILE) >> + >> + > I like explicit load rather than put these in top level and generate > a global variable. > Think about two processes(let's say in websockify.py) both use this > module, top level code executed for a second time, the later process > may get a different config from the first. > But developer may fail to notice. > This will cause some trouble which is difficult to diagnose. Nope, I don't think it will cause any race. First of all, no running config happens outside that module. Second, that module will be imported only one time because they're running in the same python process. So do you still think it can cause any problem?

>> if __name__ == '__main__': >> print get_prefix() >> diff --git a/src/kimchid.in b/src/kimchid.in >> index 7865713..548fa52 100644 >> --- a/src/kimchid.in >> +++ b/src/kimchid.in >> @@ -33,31 +33,13 @@ import kimchi.config >> if kimchi.config.without_installation(): >> sys.path.append(kimchi.config.get_prefix()) >> >> -from ConfigParser import SafeConfigParser >> +from kimchi.config import config >> from optparse import OptionParser >> >> ACCESS_LOG = "kimchi-access.log" >> ERROR_LOG = "kimchi-error.log" >> -CONFIG_FILE = "%s/kimchi.conf" % kimchi.config.get_config_dir() >> -DEFAULT_LOG_DIR = kimchi.config.get_default_log_dir() >> -DEFAULT_LOG_LEVEL = "debug" >> >> def main(options): >> - config = SafeConfigParser() >> - config.add_section("server") >> - config.set("server", "host", "0.0.0.0") >> - config.set("server", "port", "8000") >> - config.set("server", "ssl_port", "8001") >> - config.set("server", "ssl_cert", "") >> - config.set("server", "ssl_key", "") >> - config.set("server", "environment", "development") >> - config.add_section("logging") >> - config.set("logging", "log_dir", DEFAULT_LOG_DIR) >> - config.set("logging", "log_level", DEFAULT_LOG_LEVEL) >> - >> - if os.path.exists(CONFIG_FILE): >> - config.read(CONFIG_FILE) >> - >> host = config.get("server", "host") >> port = config.get("server", "port") >> ssl_port = config.get("server", "ssl_port") >