[Kimchi-devel] [PATCH v2][Wok 2/8] Changes server behavior to use pluginsmanager
Aline Manera
alinefm at linux.vnet.ibm.com
Wed Jun 8 23:31:29 UTC 2016
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.
>
>>
>>>>
>>>>> 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
>
More information about the Kimchi-devel
mailing list