[Kimchi-devel] [PATCH v2][Wok 2/8] Changes server behavior to use pluginsmanager
Aline Manera
alinefm at linux.vnet.ibm.com
Thu Jun 9 01:02:02 UTC 2016
On 06/08/2016 08:31 PM, Aline Manera wrote:
>
>
> On 06/08/2016 07:18 PM, Rodrigo Trujillo wrote:
>>
>>
>> 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.
>>
>
> I agree that probably I am not understanding all that cyclic imports.
>
> My point is to do not call a function to read a configuration file on
> every request.
>
All that discussion can be solved by adding:
plugins_tabs = get_all_tabs()
after the import session on pluginsmanager.py and use
from wok.pluginsmanager import plugins_tabs
when you need to use this value.
That way we guarantee the plugins configuration files will be parsed once.
>>
>>>
>>>>>
>>>>>> 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!
>
> ... by adding a different issue.
>
> Maybe we need to think in a better solution.
>
>>
>>>>
>>>>>
>>>>>> '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
>>>
>>
>> _______________________________________________
>> 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