On 01/24/2014 09:37 AM, Aline Manera wrote:
On 01/22/2014 03:48 AM, Mark Wu wrote:
This patch reorganizes kimchi's paths generation code to make the user
of path vars can work with both kimchi root app and plugin app.

Signed-off-by: Mark Wu <wudxw@linux.vnet.ibm.com>
---
  src/kimchi/cachebust.py |   4 +-
  src/kimchi/config.py.in | 164 ++++++++++++++++++++----------------------------
  src/kimchi/root.py      |   6 +-
  src/kimchi/server.py    |  10 +--
  src/kimchi/template.py  |  14 +++--
  src/kimchi/utils.py     |   6 +-
  src/kimchid.in          |   7 ++-
  tests/test_plugin.py    |   4 +-
  8 files changed, 100 insertions(+), 115 deletions(-)

diff --git a/src/kimchi/cachebust.py b/src/kimchi/cachebust.py
index 9a71f4f..7844695 100644
--- a/src/kimchi/cachebust.py
+++ b/src/kimchi/cachebust.py
@@ -23,11 +23,11 @@
  import os


-from kimchi.config import get_prefix
+from kimchi.config import paths


  def href(url):
      # for error.html, url is absolute path
-    f = os.path.join(get_prefix(), 'ui', url.lstrip("/"))
+    f = os.path.join(paths.ui_dir, url.lstrip("/"))
      mtime = os.path.getmtime(f)
      return "%s?cacheBust=%s" % (url, mtime)
diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
index 22fe614..53c8e42 100644
--- a/src/kimchi/config.py.in
+++ b/src/kimchi/config.py.in
@@ -28,7 +28,6 @@ import platform


  from ConfigParser import SafeConfigParser
-from glob import iglob


  from kimchi.xmlutils import xpath_get_text
@@ -37,86 +36,24 @@ from kimchi.xmlutils import xpath_get_text
  DEFAULT_LOG_LEVEL = "debug"


-def get_prefix():
-    if __file__[0] == '/':
-        base = os.path.dirname(__file__)
-    else:
-        base = os.path.dirname('./%s' % __file__)
-
-    if os.access('%s/../../src/kimchi/config.py' % base, os.F_OK):
-        return '%s/../..' % base
-    else:
-        return '@pkgdatadir@'
-
-
-def without_installation():
-    return get_prefix() != '@pkgdatadir@'
-
-
-def get_config_dir():
-    prefix = get_prefix()
-
-    if prefix == '@pkgdatadir@':
-        return '@sysconfdir@/kimchi'
-    else:
-        return os.path.join(prefix, 'src')
-
-
-def _get_kimchi_src_dir():
-    prefix = get_prefix()
-    return ('@kimchidir@' if prefix == '@pkgdatadir@'
-            else os.path.join(prefix, 'src/kimchi'))
-
-
-def get_api_schema_file():
-    return os.path.join(_get_kimchi_src_dir(), 'API.json')
-
-
-def get_default_log_dir():
-    prefix = get_prefix()
-
-    if prefix == '@pkgdatadir@':
-        return "@localstatedir@/log/kimchi"
-
-    return os.path.join(prefix, 'log')
-
-
-def get_state_path():
-    prefix = get_prefix()
-
-    if prefix == '@pkgdatadir@':
-        return "@localstatedir@/lib/kimchi"
-
-    return os.path.join(prefix, 'data')
-
-
  def get_session_path():
-    return os.path.join(get_state_path(), 'sessions')
+    return os.path.join(paths.state_dir, 'sessions')


  def get_object_store():
-    return os.path.join(get_prefix(), 'data', 'objectstore')
+    return os.path.join(paths.state_dir, 'objectstore')


  def get_distros_store():
-    return os.path.join(get_config_dir(), 'distros.d')
-
-
-def get_template_path(resource):
-    return '%s/ui/pages/%s.tmpl' % (get_prefix(), resource)
+    return os.path.join(paths.conf_dir, 'distros.d')


  def get_screenshot_path():
-    return "%s/data/screenshots" % get_prefix()
-
+    return os.path.join(paths.state_dir, 'screenshots')

-def get_mo_path():
-    return '%s/mo' % get_prefix()

-
-def get_support_language():
-    mopath = "%s/*" % get_mo_path()
-    return [path.rsplit('/', 1)[1] for path in iglob(mopath)]
+def get_debugreports_path():
+    return os.path.join(paths.state_dir, 'debugreports')


  def find_qemu_binary():
@@ -137,37 +74,74 @@ def find_qemu_binary():
      return location


-def get_debugreports_path():
-    return os.path.join(get_prefix(), 'data', 'debugreports')
+class Paths(object):
+
+    def __init__(self):
+        self.prefix = self.get_prefix()
+        self.installed = (self.prefix == '@pkgdatadir@')
+
+        if self.installed:
+            self.state_dir = '@localstatedir@/lib/kimchi'
+            self.log_dir = '@localstatedir@/log/kimchi'
+            self.conf_dir = '@sysconfdir@/kimchi'
+            self.src_dir = '@kimchidir@'
+            self.plugins_dir = '@kimchidir@'
+        else:
+            self.state_dir = self.add_prefix('data')
+            self.log_dir = self.add_prefix('log')
+            self.conf_dir = self.add_prefix('src')
+            self.src_dir = self.add_prefix('src/kimchi')
+            self.plugins_dir = self.add_prefix('plugins')
+
+        self.ui_dir = self.add_prefix('ui')
+        self.mo_dir = self.add_prefix('mo')
+
+    def get_prefix(self):
+        if __file__[0] == '/':
+            base = os.path.dirname(__file__)
+        else:
+            base = os.path.dirname('./%s' % __file__)
+
+        if os.access('%s/../../src/kimchi/config.py' % base, os.F_OK):
+            return os.path.abspath('%s/../..' % base)
+        else:
+            return '@pkgdatadir@'
+
+    def add_prefix(self, subdir):
+        return os.path.join(self.prefix, subdir)
+
+    def get_template_path(self, resource):
+        return os.path.join(self.ui_dir, 'pages/%s.tmpl' % resource)
+
+

+paths = Paths()
+

Move it to the constants section
We have to put it after the definition of class Paths.  If you insist on it,  we will have to move the class Path to a new module.  But it's still not clean to have paths related definition in two files.


+class PluginPaths(Paths):

-def get_plugins_dir():
-    prefix = get_prefix()
-    if prefix == '@pkgdatadir@':
-        prefix = '@kimchidir@'
-    return os.path.join(prefix, 'plugins')
+    def __init__(self, name):
+        super(PluginPaths, self).__init__()
+        self.plugin_dir = os.path.join('plugins', name)

+        if self.installed:
+            self.conf_dir = '@sysconfdir@/kimchi/plugins.d'
+            self.src_dir = os.path.join('@kimchidir@', self.plugin_dir)
+        else:
+            self.conf_dir = self.add_prefix(self.plugin_dir)
+            self.src_dir = self.add_prefix(self.plugin_dir)

-def get_plugin_config(name):
-    prefix = get_prefix()
-    if prefix == '@pkgdatadir@':
-        base_dir = '@sysconfdir@/kimchi/plugins.d/'
-    else:
-        base_dir = os.path.join(prefix, 'plugins')
-    return os.path.join(base_dir, '%s/%s.conf' % (name, name))
+        self.ui_dir = self.add_prefix(os.path.join(self.plugin_dir, 'ui'))
+        self.mo_dir = self.add_prefix(os.path.join(self.plugin_dir, 'mo'))
+        self.conf_file = os.path.join(self.conf_dir, '%s.conf' % name)


-def _get_plugin_ui_dir(name):
-    prefix = get_prefix()
-    if prefix == '@pkgdatadir@':
-        base_dir = '@pkgdatadir@/plugins/'
-    else:
-        base_dir = os.path.join(prefix, 'plugins')
-    return os.path.join(base_dir, '%s/ui' % name)

+_plugin_paths = {}

Why is it needed?
It was kind of cache to avoid


instantiation many times. but it just have a few, so remove it in v2.




-def get_plugin_tab_xml(name):
-    return os.path.join(_get_plugin_ui_dir(name), 'config/tab-ext.xml')
+def plugin_paths(plugin_name):
+    if plugin_name not in _plugin_paths:
+        _plugin_paths[plugin_name] = PluginPaths(plugin_name)
+    return _plugin_paths[plugin_name]


  def _get_config():
@@ -180,12 +154,12 @@ def _get_config():
      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_dir", paths.log_dir)
      config.set("logging", "log_level", DEFAULT_LOG_LEVEL)
      config.add_section("display")
      config.set("display", "display_proxy_port", "64667")

-    config_file = os.path.join(get_config_dir(), 'kimchi.conf')
+    config_file = os.path.join(paths.conf_dir, 'kimchi.conf')
      if os.path.exists(config_file):
          config.read(config_file)
      return config
@@ -195,4 +169,4 @@ config = _get_config()


  if __name__ == '__main__':
-    print get_prefix()
+    print paths.prefix
diff --git a/src/kimchi/root.py b/src/kimchi/root.py
index 3cc6321..ae9fd0e 100644
--- a/src/kimchi/root.py
+++ b/src/kimchi/root.py
@@ -23,11 +23,12 @@

  import cherrypy
  import json
+import os


  from kimchi import auth
  from kimchi import template
-from kimchi.config import get_api_schema_file
+from kimchi.config import paths
  from kimchi.control.base import Resource
  from kimchi.control.config import Config
  from kimchi.control.debugreports import DebugReports
@@ -67,7 +68,8 @@ class Root(Resource):
          self.host = Host(model)
          self.debugreports = DebugReports(model)
          self.plugins = Plugins(model)
-        self.api_schema = json.load(open(get_api_schema_file()))
+        self.api_schema = json.load(open(os.path.join(paths.src_dir,
+                                                      'API.json')))

      def error_production_handler(self, status, message, traceback, version):
          data = {'code': status, 'reason': message}
diff --git a/src/kimchi/server.py b/src/kimchi/server.py
index b820263..58b082b 100644
--- a/src/kimchi/server.py
+++ b/src/kimchi/server.py
@@ -33,6 +33,7 @@ from kimchi import config
  from kimchi import model
  from kimchi import mockmodel
  from kimchi import vnc
+from kimchi.config import paths, plugin_paths
  from kimchi.root import Root
  from kimchi.utils import get_enabled_plugins, import_class

@@ -63,8 +64,8 @@ class Server(object):
      CACHEEXPIRES = 31536000
      configObj = {
          '/': {'tools.trailing_slash.on': False,
-              'tools.staticdir.root': config.get_prefix(),
-              'tools.staticfile.root': config.get_prefix(),
+              'tools.staticdir.root': paths.prefix,
+              'tools.staticfile.root': paths.prefix,
                'request.methods_with_bodies': ('POST', 'PUT'),
                'tools.nocache.on': True,
                'tools.sessions.on': True,
@@ -211,7 +212,8 @@ class Server(object):
                  plugin_config['/ui/config/tab-ext.xml'] = {
                      'tools.staticfile.on': True,
                      'tools.staticfile.filename':
-                    config.get_plugin_tab_xml(plugin_name),
+                    os.path.join(plugin_paths(plugin_name).ui_dir,
+                                 'config/tab-ext.xml'),
                      'tools.nocache.on': True}
              except KeyError:
                  continue
@@ -233,7 +235,7 @@ class Server(object):
          cert = options.ssl_cert
          key = options.ssl_key
          if not cert or not key:
-            config_dir = config.get_config_dir()
+            config_dir = paths.conf_dir
              cert = '%s/kimchi-cert.pem' % config_dir
              key = '%s/kimchi-key.pem' % config_dir

diff --git a/src/kimchi/template.py b/src/kimchi/template.py
index 1f19c4a..1740997 100644
--- a/src/kimchi/template.py
+++ b/src/kimchi/template.py
@@ -28,9 +28,10 @@ import os


  from Cheetah.Template import Template
+from glob import iglob


-from kimchi import config
+from kimchi.config import paths


  def get_lang():
@@ -54,8 +55,13 @@ def get_lang():
      return langs


+def get_support_languages():
+    mopath = "%s/*" % paths.mo_dir
+    return [path.rsplit('/', 1)[1] for path in iglob(mopath)]
+
+
  def validate_language(langs):
-    supportLangs = config.get_support_language()
+    supportLangs = get_support_languages()
      for lang in langs:
          if lang in supportLangs:
              return lang
@@ -88,12 +94,12 @@ def render(resource, data):
                            separators=(',', ':'),
                            encoding='iso-8859-1')
      elif can_accept_html():
-        filename = config.get_template_path(resource)
+        filename = paths.get_template_path(resource)
          try:
              params = {'data': data}
              lang = validate_language(get_lang())
              gettext_conf = {'domain': 'kimchi',
-                            'localedir': config.get_mo_path(),
+                            'localedir': paths.mo_dir,
                              'lang': [lang]}
              params['lang'] = gettext_conf
              return Template(file=filename, searchList=params).respond()
diff --git a/src/kimchi/utils.py b/src/kimchi/utils.py
index 5d6e8ea..7077b3b 100644
--- a/src/kimchi/utils.py
+++ b/src/kimchi/utils.py
@@ -28,10 +28,10 @@ import urllib2


  from cherrypy.lib.reprconf import Parser
+from kimchi.config import paths, plugin_paths
  from kimchi.exception import TimeoutExpired


-from kimchi import config
  from threading import Timer


@@ -48,7 +48,7 @@ def is_digit(value):


  def _load_plugin_conf(name):
-    plugin_conf = config.get_plugin_config(name)
+    plugin_conf = plugin_paths(name).conf_file
      if not os.path.exists(plugin_conf):
          cherrypy.log.error_log.error("Plugin configuration file %s"
                                       " doesn't exist." % plugin_conf)
@@ -62,7 +62,7 @@ def _load_plugin_conf(name):


  def get_enabled_plugins():
-    plugin_dir = config.get_plugins_dir()
+    plugin_dir = paths.plugins_dir
      try:
          dir_contents = os.listdir(plugin_dir)
      except OSError:
diff --git a/src/kimchid.in b/src/kimchid.in
index 548fa52..eebc017 100644
--- a/src/kimchid.in
+++ b/src/kimchid.in
@@ -30,12 +30,13 @@ sys.path.insert(1, '@pythondir@')

  import kimchi.server
  import kimchi.config
-if kimchi.config.without_installation():
-    sys.path.append(kimchi.config.get_prefix())

-from kimchi.config import config
+from kimchi.config import config, paths
  from optparse import OptionParser

+if not paths.installed:
+    sys.path.append(paths.prefix)
+
  ACCESS_LOG = "kimchi-access.log"
  ERROR_LOG = "kimchi-error.log"

diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 42c87a9..e5c9053 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -32,7 +32,7 @@ from functools import partial
  import kimchi.mockmodel
  import kimchi.server
  import utils
-from kimchi import config
+from kimchi.config import paths


  test_server = None
@@ -48,7 +48,7 @@ def setUpModule():
      host = '127.0.0.1'
      port = utils.get_free_port('http')
      ssl_port = None
-    sys.path.append(config.get_prefix())
+    sys.path.append(paths.prefix)
      test_server = utils.run_server(host, port, ssl_port, test_mode=True,
                                     model=model)