[Kimchi-devel] [PATCH 1/2] Add static directory configurations for plugin's ui
Mark Wu
wudxw at linux.vnet.ibm.com
Tue Mar 4 08:45:29 UTC 2014
On 03/04/2014 03:51 PM, Zhou Zheng Sheng wrote:
> on 2014/02/28 17:59, Mark Wu wrote:
>> Plugin needs static directory configuration for ui requirements,
>> such as css, js, etc, as what we do for kimchi base. So this
>> patch refactors the server configuration to make plugin can share
>> the same code with kimchi. To avoid the warning messages reported from
>> cherrypy's path checker, it also adds empty ui directory for the
>> sample plugin, which also can demonstrate plugin's ui directory
>> sturcture. We can add actual ui files under those direcotries later.
>> ---
>> plugins/sample/ui/css/.gitignore | 0
>> plugins/sample/ui/images/.gitignore | 0
>> plugins/sample/ui/js/.gitignore | 0
>> plugins/sample/ui/libs/.gitignore | 0
>> src/kimchi/config.py.in | 79 +++++++++++++++++++++++++++++++++++++
>> src/kimchi/server.py | 77 ++----------------------------------
>> 6 files changed, 82 insertions(+), 74 deletions(-)
>> create mode 100644 plugins/sample/ui/css/.gitignore
>> create mode 100644 plugins/sample/ui/images/.gitignore
>> create mode 100644 plugins/sample/ui/js/.gitignore
>> create mode 100644 plugins/sample/ui/libs/.gitignore
>>
>> diff --git a/plugins/sample/ui/css/.gitignore b/plugins/sample/ui/css/.gitignore
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/plugins/sample/ui/images/.gitignore b/plugins/sample/ui/images/.gitignore
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/plugins/sample/ui/js/.gitignore b/plugins/sample/ui/js/.gitignore
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/plugins/sample/ui/libs/.gitignore b/plugins/sample/ui/libs/.gitignore
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>> index 2747164..f599c4d 100644
>> --- a/src/kimchi/config.py.in
>> +++ b/src/kimchi/config.py.in
>> @@ -127,6 +127,85 @@ class PluginPaths(Paths):
>> self.conf_file = os.path.join(self.conf_dir, '%s.conf' % name)
>>
>>
>> +class AppConfig(dict):
>> +
>> + # expires is one year.
>> + CACHEEXPIRES = 31536000
>> +
>> + def _get_ui_configs(self, paths):
>> + ui_configs = {}
>> + for sub_dir in ('css', 'js', 'libs', 'images'):
>> + ui_configs['/' + sub_dir] = {
>> + 'tools.staticdir.on': True,
>> + 'tools.staticdir.dir': os.path.join(paths.ui_dir, sub_dir),
>> + 'tools.nocache.on': False}
>> + if sub_dir != 'images':
>> + ui_configs['/' + sub_dir].update({
>> + 'tools.expires.on': True,
>> + 'tools.expires.secs': AppConfig.CACHEEXPIRES})
>> + return ui_configs
>> +
> Hi,
>
> The patch is good, but I don't like the way it uses inheritance. The
> KimchiConfig and PluginConfig don't have to be the subclass of
> AppConfig, and AppConfig is actual just a container for UI related
> config, but not general configurations. Inheritance expresses a strong
> "is-a-kind-of" relationship. It seems an overkill to use object
> encapsulation and inheritance here. May I propose a better way to model
> these three configs as follow?
>
> Basically, the concept is rather simple:
>
> kimchi_config = basic_kimchi_config + ui_config(kimchi_paths)
> plugin_config = basic_plugin_config + ui_config(plugin_paths)
>
> So in my opinion it should be a use case of composition, not inheritance.
>
> Pseudo code is like this.
>
> def _get_ui_config(paths):
> ui_configs = {}
> for sub_dir in ('css', 'js', 'libs', 'images'):
> ...
> # almose the same as AppConfig.__init__ in you code.
> return ui_configs
>
> def _get_kimchi_config():
> c = {'/': {'tools.trailing_slash.on': False, ...}
> c.update(_get_ui_config(paths))
> return c
>
> def _get_plugin_ui_config(plugin_name):
> c = {'/ui/config/tab-ext.xml': {...
> c.update(_get_ui_config(PluginPaths(plugin_name)))
> return c
>
> class Server(object):
> def __init__(self, options):
> ...
> self.configObj = _get_kimchi_config()
>
> def _load_plugins(self):
> ...
> for plugin_name, plugin_config in get_enabled_plugins():
> ...
> plugin_config.update(_get_plugin_ui_config(plugin_name))
>
>
> If you prefer the objective way. Maybe you can consider the following
> object model.
Thanks for the review.
I think you're right. It makes more sense to use composite than
Inheritance here.
Let's do the objective way because in future we could add more common
configurations for
kimchi and plugins. After we add that kind of configurations, we could
change to Inheritance because
the attributes are in parent object.
>
> class UIConfig(dict):
> def __init__(self, paths):
> for sub_dir in ('css', 'js', 'libs', 'images'):
> ...
> # almose the same as AppConfig.__init__ in you code.
>
> class KimchiConfig(dict):
> def __init__(self):
> super(KimchiConfig, self).__init__()
> kimchi_config = {...}
> self.update(self.kimchi_config)
> self.update(UIConfig(paths))
>
> class PluginConfig(dict):
> def __init__(self):
> super(PluginConfig, self).__init__()
> plugin_config = {...}
> self.update(plugin_config)
> self.update(UIConfig(PluginPaths(plugin_name)))
>
>> +
>> +class KimchiConfig(AppConfig):
>> + kimchi_config = {
>> + '/': {'tools.trailing_slash.on': False,
>> + 'request.methods_with_bodies': ('POST', 'PUT'),
>> + 'tools.nocache.on': True,
>> + 'tools.sessions.on': True,
>> + 'tools.sessions.name': 'kimchi',
>> + 'tools.sessions.httponly': True,
>> + 'tools.sessions.locking': 'explicit',
>> + 'tools.sessions.storage_type': 'ram',
>> + 'tools.kimchiauth.on': False},
>> + '/data/screenshots': {
>> + 'tools.staticdir.on': True,
>> + 'tools.staticdir.dir': get_screenshot_path(),
>> + 'tools.nocache.on': False
>> + },
>> + '/data/debugreports': {
>> + 'tools.staticdir.on': True,
>> + 'tools.staticdir.dir': get_debugreports_path(),
>> + 'tools.nocache.on': False,
>> + 'tools.kimchiauth.on': True,
>> + 'tools.staticdir.content_types': {'xz': 'application/x-xz'}
>> + },
>> + '/config/ui/tabs.xml': {
>> + 'tools.staticfile.on': True,
>> + 'tools.staticfile.filename': '%s/config/ui/tabs.xml' %
>> + paths.prefix,
>> + 'tools.nocache.on': True
>> + },
>> + '/favicon.ico': {
>> + 'tools.staticfile.on': True,
>> + 'tools.staticfile.filename': '%s/images/logo.ico' % paths.ui_dir
>> + },
>> + '/help': {
>> + 'tools.staticdir.on': True,
>> + 'tools.staticdir.dir': '%s/ui/pages/help' % paths.prefix,
>> + 'tools.nocache.on': False
>> + }
>> + }
>> +
>> + def __init__(self):
>> + super(KimchiConfig, self).__init__(self)
>> + self.update(self.kimchi_config)
>> + self.update(self._get_ui_configs(paths))
>> +
>> +
>> +class PluginConfig(AppConfig):
>> + def __init__(self, plugin_name):
>> + super(PluginConfig, self).__init__(self)
>> + plugin_config = {
>> + '/ui/config/tab-ext.xml': {
>> + 'tools.staticfile.on': True,
>> + 'tools.staticfile.filename':
>> + os.path.join(PluginPaths(plugin_name).ui_dir,
>> + 'config/tab-ext.xml'),
>> + 'tools.nocache.on': True}}
>> + self.update(plugin_config)
>> + self.update(self._get_ui_configs(PluginPaths(plugin_name)))
>> +
>> +
>> def _get_config():
>> config = SafeConfigParser()
>> config.add_section("server")
>> diff --git a/src/kimchi/server.py b/src/kimchi/server.py
>> index 5475b92..0d02868 100644
>> --- a/src/kimchi/server.py
>> +++ b/src/kimchi/server.py
>> @@ -29,7 +29,7 @@ from kimchi import config
>> from kimchi.model import model
>> from kimchi import mockmodel
>> from kimchi import vnc
>> -from kimchi.config import paths, PluginPaths
>> +from kimchi.config import paths, KimchiConfig, PluginConfig
>> from kimchi.control import sub_nodes
>> from kimchi.root import KimchiRoot
>> from kimchi.utils import get_enabled_plugins, import_class
>> @@ -57,73 +57,6 @@ def set_no_cache():
>>
>>
>> class Server(object):
>> - # expires is one year.
>> - CACHEEXPIRES = 31536000
>> - configObj = {
>> - '/': {'tools.trailing_slash.on': False,
>> - 'request.methods_with_bodies': ('POST', 'PUT'),
>> - 'tools.nocache.on': True,
>> - 'tools.sessions.on': True,
>> - 'tools.sessions.name': 'kimchi',
>> - 'tools.sessions.httponly': True,
>> - 'tools.sessions.locking': 'explicit',
>> - 'tools.sessions.storage_type': 'ram',
>> - 'tools.kimchiauth.on': False},
>> - '/css': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': '%s/ui/css' % paths.prefix,
>> - 'tools.expires.on': True,
>> - 'tools.expires.secs': CACHEEXPIRES,
>> - 'tools.nocache.on': False
>> - },
>> - '/js': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': '%s/ui/js' % paths.prefix,
>> - 'tools.expires.on': True,
>> - 'tools.expires.secs': CACHEEXPIRES,
>> - 'tools.nocache.on': False
>> - },
>> - '/libs': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': '%s/ui/libs' % paths.prefix,
>> - 'tools.expires.on': True,
>> - 'tools.expires.secs': CACHEEXPIRES,
>> - 'tools.nocache.on': False,
>> - },
>> - '/images': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': '%s/ui/images' % paths.prefix,
>> - 'tools.nocache.on': False
>> - },
>> - '/data/screenshots': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': config.get_screenshot_path(),
>> - 'tools.nocache.on': False
>> - },
>> - '/data/debugreports': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': config.get_debugreports_path(),
>> - 'tools.nocache.on': False,
>> - 'tools.kimchiauth.on': True,
>> - 'tools.staticdir.content_types': {'xz': 'application/x-xz'}
>> - },
>> - '/config/ui/tabs.xml': {
>> - 'tools.staticfile.on': True,
>> - 'tools.staticfile.filename': '%s/config/ui/tabs.xml' %
>> - paths.prefix,
>> - 'tools.nocache.on': True
>> - },
>> - '/favicon.ico': {
>> - 'tools.staticfile.on': True,
>> - 'tools.staticfile.filename': '%s/images/logo.ico' % paths.ui_dir
>> - },
>> - '/help': {
>> - 'tools.staticdir.on': True,
>> - 'tools.staticdir.dir': '%s/ui/pages/help' % paths.prefix,
>> - 'tools.nocache.on': False
>> - }
>> - }
>> -
>> def __init__(self, options):
>> make_dirs = [
>> os.path.dirname(os.path.abspath(options.access_log)),
>> @@ -137,6 +70,7 @@ class Server(object):
>> if not os.path.isdir(directory):
>> os.makedirs(directory)
>>
>> + self.configObj = KimchiConfig()
>> cherrypy.tools.nocache = cherrypy.Tool('on_end_resource', set_no_cache)
>> cherrypy.tools.kimchiauth = cherrypy.Tool('before_handler',
>> auth.kimchiauth)
>> @@ -214,12 +148,7 @@ class Server(object):
>> script_name = plugin_config['kimchi']['uri']
>> del plugin_config['kimchi']
>>
>> - plugin_config['/ui/config/tab-ext.xml'] = {
>> - 'tools.staticfile.on': True,
>> - 'tools.staticfile.filename':
>> - os.path.join(PluginPaths(plugin_name).ui_dir,
>> - 'config/tab-ext.xml'),
>> - 'tools.nocache.on': True}
>> + plugin_config.update(PluginConfig(plugin_name))
>> except KeyError:
>> continue
>>
>
More information about the Kimchi-devel
mailing list