[Kimchi-devel] [PATCH v2][Wok 2/8] Changes server behavior to use pluginsmanager
Rodrigo Trujillo
rodrigo.trujillo at linux.vnet.ibm.com
Wed Jun 8 22:18:50 UTC 2016
On 06/08/2016 06:18 PM, Aline Manera wrote:
>
>
> 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.
You are not understanding the problem. "auth" is imported by Server.py
BEFORE server class be initialized
then 'tabs' have to be set, which means that PluginsManager will be
instantiated and initialized before the
server, what is wrong. Server should initiate the Plugins Manager.
>
>>>
>>>> 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?
>
I understood that, just did not see the multiple calls problem before. I
changed to fix bad architectural imports made!
>>
>>>
>>>> '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
>>
>
> _______________________________________________
> 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