Re: [Kimchi-devel] [PATCH v2][Wok 1/8] Creates pluginmanager.py module

On 06/08/2016 03:52 PM, Aline Manera wrote: > > > On 06/06/2016 04:13 PM, Rodrigo Trujillo wrote: >> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; >> --- >> src/wok/pluginsmanager.py | 238 >> ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 238 insertions(+) >> create mode 100644 src/wok/pluginsmanager.py >> >> diff --git a/src/wok/pluginsmanager.py b/src/wok/pluginsmanager.py >> new file mode 100644 >> index 0000000..d799f11 >> --- /dev/null >> +++ b/src/wok/pluginsmanager.py >> @@ -0,0 +1,238 @@ >> +# >> +# Project Wok >> +# >> +# Copyright IBM Corp, 2016 >> +# >> +# This library is free software; you can redistribute it and/or >> +# modify it under the terms of the GNU Lesser General Public >> +# License as published by the Free Software Foundation; either >> +# version 2.1 of the License, or (at your option) any later version. >> +# >> +# This library is distributed in the hope that it will be useful, >> +# but WITHOUT ANY WARRANTY; without even the implied warranty of >> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> +# Lesser General Public License for more details. >> +# >> +# You should have received a copy of the GNU Lesser General Public >> +# License along with this library; if not, write to the Free Software >> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA >> 02110-1301 USA >> +# >> + >> + >> +import cherrypy >> +import os >> +import xml.etree.ElementTree as ET >> +from cherrypy.lib.reprconf import Parser >> +from configobj import ConfigObj >> + >> + >> +from basemodel import Singleton >> +from wok.config import paths, PluginConfig, PluginPaths >> +from wok.exception import OperationFailed >> +from wok.utils import import_class, wok_log >> + >> + >> +class Plugins(): >> + __metaclass__ = Singleton >> + >> + def __init__(self, options=None): >> + # { '<PLUGIN_NAME>': { >> + # config: <PLUGIN_CHERRYPY_CONFIG>, >> + # enabled: <TRUE/FALSE>, >> + # uri: <PLUGIN_URI_FROM_FILE_CONFIG>, >> + # app: <PLUGIN_MAIN_MODULE> >> + # conf_file: <PLUGIN_CONFIGURATION_FILE> >> + self._plugins_dict = {} >> + self.options = options >> + self._init_all_plugins() >> + >> + def _load_plugin_app(self, name): >> + """ >> + Loads the plugin main module >> + """ >> + plugin_class = ('plugins.%s.%s' % (name, name[0].upper() + >> name[1:])) >> + try: >> + plugin_app = import_class(plugin_class)(self.options) >> + except ImportError, e: >> + wok_log.error("Failed to import plugin %s, error: %s" % >> + (plugin_class, e.message)) >> + self._plugins_dict[name]['enabled'] = False >> + self._plugins_dict[name]['app'] = None >> + return >> + self._plugins_dict[name]['app'] = plugin_app >> + >> + def _load_plugin_app_config(self, name): >> + """ >> + Sets the plugin's cherrypy configuration. That is a merge >> between Wok >> + standard configuration plus plugin's self configuration >> + """ >> + app_conf = {} >> + app_conf.update(PluginConfig(name)) >> + plugin_app = self._plugins_dict[name]['app'] >> + if plugin_app is None: >> + return >> + >> + # 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: >> + app_conf.update(get_custom_conf()) >> + >> + # dynamically add tools.wokauth.on = True to extra plugin APIs >> + try: >> + sub_nodes = import_class('plugins.%s.control.sub_nodes' % >> + name) >> + urlSubNodes = {} >> + for ident, node in sub_nodes.items(): >> + if node.url_auth: >> + ident = "/%s" % ident >> + urlSubNodes[ident] = {'tools.wokauth.on': True} >> + app_conf.update(urlSubNodes) >> + except ImportError, e: >> + wok_log.error("Failed to import subnodes for plugin %s, >> error: %s" >> + % (name, e.message)) >> + self._plugins_dict[name]['config'] = app_conf >> + >> + def _load_plugin_config_file(self, name): >> + """ >> + Loads the information from Wok section in <plugin>.conf >> file. Currently >> + two tags/options are required to Wok and section looks like: >> + [wok] >> + enable = True >> + uri = '/plugins/mypluginpath' >> + """ >> + plugin_conf_file = PluginPaths(name).conf_file >> + config = {} I have set "config" here, to avoid the problems you reported below

>> + if not os.path.exists(plugin_conf_file): >> + wok_log.error("Plugin configuration file %s doesn't >> exist." % >> + plugin_conf_file) >> + else: >> + try: >> + config = Parser().dict_from_file(plugin_conf_file) >> + except ValueError as e: >> + msg = "Failed to load plugin conf from %s: %s" >> + wok_log.error(msg % (plugin_conf_file, e.message)) >> + > >> + plugin = self._plugins_dict[name] >> + plugin['conf_file'] = plugin_conf_file >> + plugin['enabled'] = config.get('wok', {}).get('enable', False) >> + plugin['uri'] = config.get('wok', {}).get('uri', 'Unknow') >> + > > This block should be on 'else' statement (as part of the 'try' > block') or add a 'return' in the 'if' statement as the config file > was not found. > Otherwise, it will raise an exception as 'config' may be not defined > if it does not entered on 'else' block. The same will happen if it > goes to the 'except' block. So move it to 'try' block. > > Tip: if you add 'return' in the 'if' statement you don't need the > 'else' anymore and eliminate one indentation level. > See my comment above. I want to load the "possible" plugin as disabled, even if the config file is missing

>> + def _init_all_plugins(self): >> + """ >> + Initializes internal plugin dictionary, searching all >> directories in >> + <install_path>/wok/plugins, each directory should store a >> plugin >> + content. Then loads its configuration file in order to set >> as enabled >> + or disabled. >> + """ >> + plugin_dir = paths.plugins_dir >> + try: >> + dir_contents = os.listdir(plugin_dir) >> + except OSError as e: >> + wok_log.error("Failed to fetch plugins from '%s': %s" % >> + (plugin_dir, e.message)) >> + return >> + for name in dir_contents: >> + if os.path.isdir(os.path.join(plugin_dir, name)): >> + # TODO: >> + # Add command line option to disable plugin by its >> name >> + # >> + self._plugins_dict[name] = {} >> + self._plugins_dict[name]['config'] = {} >> + self._plugins_dict[name]['app'] = None >> + self._load_plugin_config_file(name) >> + > >> + # Disable all plugins but 'sample' in test mode >> + if (self.options is not None) and self.options.test: >> + self._plugins_dict[name]['enabled'] = (name == >> 'sample') >> + > > Why? I could be able to see all plugins running on test mode. > For example, Kimchi provides an MockModel to allow user tests Kimchi > without making any change in the host system. My idea here was only considering wok running its own tests. I my opinion, when wok run its tests, there is no reason to load any extra plugin but sample for testing. Thing is that I forgot to test if this behavior could impact other plugins running their own tests, and indeed (just tested) Kimchi is impacted. So, I would like to propose a differentiation I Wok behavior between running its own tests and running plugins test. For instance we can continue with 'test' option and create 'plugintest' option which should be set by plugins when they are in test mode. Or (2), create a new option "woktest" that will be passed by wok tests only, in order to disable all plugins and enable Sample.

> >> + if not self._plugins_dict[name]['enabled']: >> + wok_log.info("Initializing plugin: %s >> [DISABLED]" % name) >> + else: >> + wok_log.info("Initializing plugin: %s >> [ENABLED]" % name) >> + >> + def _set_cherrypy_app(self, name): >> + """ >> + Load plugin module and cherrypy configuration, then set it >> as cherrypy >> + app. >> + """ >> + self._load_plugin_app(name) >> + self._load_plugin_app_config(name) >> + if self._plugins_dict[name]['enabled']: >> + cherrypy.tree.mount( >> + self._plugins_dict[name]['app'], >> + self._plugins_dict[name]['uri'], >> + self._plugins_dict[name]['config']) >> + wok_log.info("Plugin '%s' loaded" % >> + self._plugins_dict[name]['app']) >> + >> + def load_plugins(self): >> + """ >> + Set enabled plugins into Cherrypy >> + """ >> + for plugin in self.get_enabled_plugins(): >> + self._set_cherrypy_app(plugin) >> + >> + def get_all_plugins_info(self): >> + return self._plugins_dict >> + >> + def get_plugin_info(self, name): >> + return self._plugins_dict.get(name, {}) >> + >> + def get_all_plugins_names(self): >> + ret = self._plugins_dict.keys() >> + ret.sort() >> + return ret >> + >> + def get_enabled_plugins(self): >> + ret = [plugin for plugin in self._plugins_dict.keys() if >> + self._plugins_dict[plugin]['enabled']] >> + ret.sort() >> + return ret >> + >> + def _enable_plugin_conf_file(self, f_conf, enable=True): >> + try: >> + # 'unrepr' makes ConfigObj read and write characters >> like ("'?) >> + conf = ConfigObj(infile=f_conf, unrepr=True) >> + conf['wok']['enable'] = enable >> + with open(f_conf, 'wb') as f: >> + conf.write(f) >> + except Exception as e: >> + wok_log.error('Error updating plugin conf file. ' + >> e.message) >> + raise >> + >> + def _change_plugin_state(self, name, enable=True): >> + plugin = self._plugins_dict[name] >> + if plugin['enabled'] == enable: >> + return >> + try: >> + self._enable_plugin_conf_file(plugin['conf_file'], enable) >> + except: >> + raise OperationFailed('WOKPLUG0002E', {'plugin': name}) >> + plugin['enabled'] = enable >> + >> + def enable_plugin(self, plugin): >> + wok_log.info("PluginsManager: Enabling plugin '%s'" % plugin) >> + self._change_plugin_state(plugin, True) >> + >> + def disable_plugin(self, plugin): >> + wok_log.info("PluginsManager: Disabling plugin '%s'" % plugin) >> + self._change_plugin_state(plugin, False) >> + >> + >> +def get_all_tabs(): >> + files = [] >> + >> + for plugin in Plugins().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 > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >