[Kimchi-devel] [PATCH v2][Wok 2/8] Changes server behavior to use pluginsmanager

Aline Manera alinefm at linux.vnet.ibm.com
Wed Jun 8 21:18:07 UTC 2016



On 06/08/2016 06:06 PM, Rodrigo Trujillo wrote:
>
>
> On 06/08/2016 03:56 PM, Aline Manera wrote:
>>
>>
>> On 06/06/2016 04:13 PM, Rodrigo Trujillo wrote:
>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>> ---
>>>   src/wok/auth.py   | 15 ++++++++-------
>>>   src/wok/root.py   |  8 ++++----
>>>   src/wok/server.py | 57 
>>> ++++++-------------------------------------------------
>>>   src/wok/utils.py  | 52 
>>> --------------------------------------------------
>>>   4 files changed, 18 insertions(+), 114 deletions(-)
>>>
>>> diff --git a/src/wok/auth.py b/src/wok/auth.py
>>> index 0355e86..f8be4b5 100644
>>> --- a/src/wok/auth.py
>>> +++ b/src/wok/auth.py
>>> @@ -35,15 +35,15 @@ import urllib2
>>>   from wok import template
>>>   from wok.config import config
>>>   from wok.exception import InvalidOperation, OperationFailed
>>> -from wok.utils import get_all_tabs, run_command
>>> +from wok.pluginsmanager import get_all_tabs
>>> +from wok.utils import run_command
>>> +
>>>
>>>   USER_NAME = 'username'
>>>   USER_GROUPS = 'groups'
>>>   USER_ROLES = 'roles'
>>>   REFRESH = 'robot-refresh'
>>
>>
>>> -tabs = get_all_tabs()
>>> -
>>
>> Keep it there and update to point to the new function to avoid 
>> multiple file processing for each server request.
>
> This change was necessary because of cyclic imports in the code.
> Backing to previous implementation will require to change other Wok 
> classes initialization.
>

I'd say to go for it (change other Wok classes initialization)
We should not let a code issue impact the server performance on that way.

>>
>>>   def redirect_login():
>>>       url = "/login.html"
>>> @@ -85,7 +85,7 @@ class PAMUser(User):
>>>           # after adding support to change user roles that info 
>>> should be read
>>>           # from a specific objstore and fallback to default only if 
>>> any entry is
>>>           # found
>>
>>> -        self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
>>> +        self.user[USER_ROLES] = dict.fromkeys(get_all_tabs(), 'user')
>>>
>>>       def get_groups(self):
>>>           out, err, rc = run_command(['id', '-Gn', 
>>> self.user[USER_NAME]])
>>> @@ -99,7 +99,7 @@ class PAMUser(User):
>>>               # after adding support to change user roles that info 
>>> should be
>>>               # read from a specific objstore and fallback to 
>>> default only if
>>>               # any entry is found
>>> -            self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
>>> +            self.user[USER_ROLES] = dict.fromkeys(get_all_tabs(), 
>>> 'admin')
>>>
>>>           return self.user[USER_ROLES]
>>>
>>> @@ -195,7 +195,7 @@ class LDAPUser(User):
>>>           self.user[USER_GROUPS] = list()
>>>           # FIXME: user roles will be changed according roles 
>>> assignment after
>>>           # objstore is integrated
>>> -        self.user[USER_ROLES] = dict.fromkeys(tabs, 'user')
>>> +        self.user[USER_ROLES] = dict.fromkeys(get_all_tabs(), 'user')
>>>
>>>       @staticmethod
>>>       def authenticate(username, password):
>>> @@ -236,7 +236,8 @@ class LDAPUser(User):
>>>               "authentication", "ldap_admin_id").strip('"').split(',')
>>>           for admin_id in admin_ids:
>>>               if self.user[USER_NAME] == admin_id.strip():
>>> -                self.user[USER_ROLES] = dict.fromkeys(tabs, 'admin')
>>> +                self.user[USER_ROLES] = dict.fromkeys(get_all_tabs(),
>>> +
>>
>> That way, on each server request the tab file will be parsed. Move it 
>> a global variable as it was before.
>
> Just made a test adding a print in the function and I do not see 
> multiples requests to it. I only see during logging to application.
>

Which means multiple calls to it. =)

Before this patch, independent on how much users were logging into 
application only one file parsing was needed. Why change that?

>
>>
>>> 'admin')
>>>           return self.user[USER_ROLES]
>>>
>>>       def get_user(self):
>>> diff --git a/src/wok/root.py b/src/wok/root.py
>>> index 29ea657..cb8b376 100644
>>> --- a/src/wok/root.py
>>> +++ b/src/wok/root.py
>>> @@ -123,10 +123,10 @@ class Root(Resource):
>>>
>>>           data['scripts'] = []
>>>           for plugin, app in cherrypy.tree.apps.iteritems():
>>> -                if app.root.extends is not None:
>>> -                    scripts = app.root.extends.get(script_name, {})
>>> -                    if page in scripts.keys():
>>> -                        data['scripts'].append(scripts[page])
>>> +            if app.root.extends is not None:
>>> +                scripts = app.root.extends.get(script_name, {})
>>> +                if page in scripts.keys():
>>> +                    data['scripts'].append(scripts[page])
>>>
>>>           if page.endswith('.html'):
>>>               context = template.render('/tabs/' + page, data)
>>> diff --git a/src/wok/server.py b/src/wok/server.py
>>> index 902d4bf..3cb257f 100644
>>> --- a/src/wok/server.py
>>> +++ b/src/wok/server.py
>>> @@ -30,14 +30,14 @@ from string import Template
>>>   from wok import auth
>>>   from wok import config
>>>   from wok.config import config as configParser
>>> -from wok.config import paths, PluginConfig, WokConfig
>>> +from wok.config import paths, WokConfig
>>>   from wok.control import sub_nodes
>>>   from wok.model import model
>>> +from wok.pluginsmanager import Plugins
>>>   from wok.proxy import start_proxy, terminate_proxy
>>>   from wok.reqlogger import RequestLogger
>>>   from wok.root import WokRoot
>>>   from wok.safewatchedfilehandler import SafeWatchedFileHandler
>>> -from wok.utils import get_enabled_plugins, import_class
>>>
>>>
>>>   LOGGING_LEVEL = {"debug": logging.DEBUG,
>>> @@ -171,61 +171,16 @@ class Server(object):
>>>
>>>           self.app = cherrypy.tree.mount(WokRoot(model_instance, 
>>> dev_env),
>>> config=self.configObj)
>>> -        self._load_plugins(options)
>>> +
>>> +        # Initiate and handles plugins
>>> +        plugins = Plugins(options)
>>> +        plugins.load_plugins()
>>>
>>>           # Terminate proxy when cherrypy server is terminated
>>>           cherrypy.engine.subscribe('exit', terminate_proxy)
>>>
>>>           cherrypy.lib.sessions.init()
>>>
>>> -    def _load_plugins(self, options):
>>> -        for plugin_name, plugin_config in get_enabled_plugins():
>>> -            try:
>>> -                plugin_class = ('plugins.%s.%s' %
>>> -                                (plugin_name,
>>> -                                 plugin_name[0].upper() + 
>>> plugin_name[1:]))
>>> -                script_name = plugin_config['wok']['uri']
>>> -                del plugin_config['wok']
>>> -
>>> - plugin_config.update(PluginConfig(plugin_name))
>>> -            except KeyError:
>>> -                continue
>>> -
>>> -            try:
>>> -                plugin_app = import_class(plugin_class)(options)
>>> -            except ImportError, e:
>>> -                cherrypy.log.error_log.error(
>>> -                    "Failed to import plugin %s, "
>>> -                    "error: %s" % (plugin_class, e.message)
>>> -                )
>>> -                continue
>>> -
>>> -            # dynamically extend plugin config with custom data, if 
>>> provided
>>> -            get_custom_conf = getattr(plugin_app, 
>>> "get_custom_conf", None)
>>> -            if get_custom_conf is not None:
>>> -                plugin_config.update(get_custom_conf())
>>> -
>>> -            # dynamically add tools.wokauth.on = True to extra 
>>> plugin APIs
>>> -            try:
>>> -                sub_nodes = 
>>> import_class('plugins.%s.control.sub_nodes' %
>>> -                                         plugin_name)
>>> -
>>> -                urlSubNodes = {}
>>> -                for ident, node in sub_nodes.items():
>>> -                    if node.url_auth:
>>> -                        ident = "/%s" % ident
>>> -                        urlSubNodes[ident] = {'tools.wokauth.on': 
>>> True}
>>> -
>>> -                    plugin_config.update(urlSubNodes)
>>> -
>>> -            except ImportError, e:
>>> -                cherrypy.log.error_log.error(
>>> -                    "Failed to import subnodes for plugin %s, "
>>> -                    "error: %s" % (plugin_class, e.message)
>>> -                )
>>> -
>>> -            cherrypy.tree.mount(plugin_app, script_name, 
>>> plugin_config)
>>> -
>>>       def start(self):
>>>           # Subscribe to SignalHandler plugin
>>>           if hasattr(cherrypy.engine, 'signal_handler'):
>>> diff --git a/src/wok/utils.py b/src/wok/utils.py
>>> index 4b3b0ac..10f2850 100644
>>> --- a/src/wok/utils.py
>>> +++ b/src/wok/utils.py
>>> @@ -32,16 +32,13 @@ import sqlite3
>>>   import subprocess
>>>   import sys
>>>   import traceback
>>> -import xml.etree.ElementTree as ET
>>>   import locale
>>>
>>> -from cherrypy.lib.reprconf import Parser
>>>   from datetime import datetime, timedelta
>>>   from multiprocessing import Process, Queue
>>>   from threading import Timer
>>>
>>>   from wok.asynctask import AsyncTask
>>> -from wok.config import paths, PluginPaths
>>>   from wok.exception import InvalidParameter, TimeoutExpired
>>>
>>>
>>> @@ -76,55 +73,6 @@ def is_digit(value):
>>>           return False
>>>
>>>
>>> -def _load_plugin_conf(name):
>>> -    plugin_conf = PluginPaths(name).conf_file
>>> -    if not os.path.exists(plugin_conf):
>>> -        cherrypy.log.error_log.error("Plugin configuration file %s"
>>> -                                     " doesn't exist." % plugin_conf)
>>> -        return
>>> -    try:
>>> -        return Parser().dict_from_file(plugin_conf)
>>> -    except ValueError as e:
>>> -        cherrypy.log.error_log.error("Failed to load plugin "
>>> -                                     "conf from %s: %s" %
>>> -                                     (plugin_conf, e.message))
>>> -
>>> -
>>> -def get_enabled_plugins():
>>> -    plugin_dir = paths.plugins_dir
>>> -    try:
>>> -        dir_contents = os.listdir(plugin_dir)
>>> -    except OSError:
>>> -        return
>>> -    for name in dir_contents:
>>> -        if os.path.isdir(os.path.join(plugin_dir, name)):
>>> -            plugin_config = _load_plugin_conf(name)
>>> -            try:
>>> -                if plugin_config['wok']['enable']:
>>> -                    yield (name, plugin_config)
>>> -            except (TypeError, KeyError):
>>> -                continue
>>> -
>>> -
>>> -def get_all_tabs():
>>> -    files = []
>>> -
>>> -    for plugin, _ in get_enabled_plugins():
>>> -        files.append(os.path.join(PluginPaths(plugin).ui_dir,
>>> -                     'config/tab-ext.xml'))
>>> -
>>> -    tabs = []
>>> -    for f in files:
>>> -        try:
>>> -            root = ET.parse(f)
>>> -        except (IOError):
>>> -            wok_log.debug("Unable to load %s", f)
>>> -            continue
>>> -        tabs.extend([t.text.lower() for t in 
>>> root.getiterator('title')])
>>> -
>>> -    return tabs
>>> -
>>> -
>>>   def get_plugin_from_request():
>>>       """
>>>       Returns name of plugin being requested. If no plugin, returns 
>>> 'wok'.
>>
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
>
> _______________________________________________
> 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