On 05/31/2016 02:51 PM, Rodrigo Trujillo wrote: > Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; > --- > src/wok/pluginsmanager.py | 133 > ++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 133 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..b7bb92b > --- /dev/null > +++ b/src/wok/pluginsmanager.py > @@ -0,0 +1,133 @@ > +# > +# Project Wok > +# > +# Copyright IBM Corp, 2016 > +# > +# Code derived from Project Kimchi > +# This is a new code, right? So not derived from Kimchi

> +# 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 copy > +import os > +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 > + > + > +wok_log = cherrypy.log.error_log > + > + > +class Plugins(): > + __metaclass__ = Singleton > + > + def __init__(self): > + # { '<PLUGIN_NAME>': { > + # config: <PLUGIN_CONFIG>, > + # enabled: <TRUE/FALSE>, > + # uri: <PLUGIN_URI_FROM_CONFIG>, > + # plugin_class: <PLUGIN_CLASS_FROM_CONFIG> plugin_class is not part of plugin config anymore.

> + self._plugins_dict = {} > + self._init_all_plugins() > + > + def _load_plugin_conf(self, name): > + plugin_conf = PluginPaths(name).conf_file > + ret = {} > + if not os.path.exists(plugin_conf): > + cherrypy.log.error_log.error("Plugin configuration file %s" > + " doesn't exist." % > plugin_conf) > + else: > + try: > + ret = Parser().dict_from_file(plugin_conf) > + except ValueError as e: > + msg = "Failed to load plugin conf from %s: %s" > + cherrypy.log.error_log.error(msg % (plugin_conf, > e.message)) > + ret.update(PluginConfig(name)) > + return ret > + > + def _init_all_plugins(self): > + 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)): > + wok_log.info("Loading plugin: %s" % name) > + cfg = self._load_plugin_conf(name) > + plugin_cls = cfg.get('wok', {}).get('plugin_class', '') Same I commented above. plugin_class is not used anymore.

> + if plugin_cls == '': > + plugin_cls = ('plugins.%s.%s' % (name, > name[0].upper() + > + name[1:])) > + self._plugins_dict[name] = { > + 'config': cfg, > + 'enabled': cfg.get('wok', {}).get('enable', False), > + 'uri': cfg.get('wok', {}).get('uri', ''), > + 'plugin_class': plugin_cls, > + 'conf_file': PluginPaths(name).conf_file > + } > + > + if 'wok' in self._plugins_dict[name]['config']: > + del self._plugins_dict[name]['config']['wok'] > + Usually the cherrypy configuration for a plugin can not be changed, so IMO it should not be part of plugin configuration file. If plugin code is responsible to loads its configuraiton, we don't need to merge the plugin configuration settings with cherrypy configuration and the above lines can be removed.

> + def get_all_plugins_info(self): > + return copy.deepcopy(self._plugins_dict) > + > + def get_plugin_info(self, name): > + return copy.deepcopy(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): > + self._change_plugin_state(plugin, True) > + > + def disable_plugin(self, plugin): > + self._change_plugin_state(plugin, False) _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

On 06/01/2016 04:30 PM, Rodrigo Trujillo wrote: > > > On 06/01/2016 12:08 AM, Aline Manera wrote: >> >> >> On 05/31/2016 02:51 PM, Rodrigo Trujillo wrote: >>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo(a)linux.vnet.ibm.com&gt; >>> --- >>> src/wok/pluginsmanager.py | 133 >>> ++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 133 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..b7bb92b >>> --- /dev/null >>> +++ b/src/wok/pluginsmanager.py >>> @@ -0,0 +1,133 @@ >>> +# >>> +# Project Wok >>> +# >>> +# Copyright IBM Corp, 2016 >>> +# >> >>> +# Code derived from Project Kimchi >>> +# >> >> This is a new code, right? So not derived from Kimchi > ack >> >>> +# 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 copy >>> +import os >>> +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 >>> + >>> + >>> +wok_log = cherrypy.log.error_log >>> + >>> + >>> +class Plugins(): >>> + __metaclass__ = Singleton >>> + >>> + def __init__(self): >>> + # { '<PLUGIN_NAME>': { >>> + # config: <PLUGIN_CONFIG>, >>> + # enabled: <TRUE/FALSE>, >>> + # uri: <PLUGIN_URI_FROM_CONFIG>, >> >>> + # plugin_class: <PLUGIN_CLASS_FROM_CONFIG> >> >> plugin_class is not part of plugin config anymore. > I was not aware of this. So what is the idea ? What will be the rule ? > The plugin main class must ALWAYS be the plugin name ? Yeap! With the first letter in upper case.

>> >>> + self._plugins_dict = {} >>> + self._init_all_plugins() >>> + >>> + def _load_plugin_conf(self, name): >>> + plugin_conf = PluginPaths(name).conf_file >>> + ret = {} >>> + if not os.path.exists(plugin_conf): >>> + cherrypy.log.error_log.error("Plugin configuration >>> file %s" >>> + " doesn't exist." % >>> plugin_conf) >>> + else: >>> + try: >>> + ret = Parser().dict_from_file(plugin_conf) >>> + except ValueError as e: >>> + msg = "Failed to load plugin conf from %s: %s" >>> + cherrypy.log.error_log.error(msg % (plugin_conf, >>> e.message)) >>> + ret.update(PluginConfig(name)) >>> + return ret >>> + >>> + def _init_all_plugins(self): >>> + 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)): >>> + wok_log.info("Loading plugin: %s" % name) >>> + cfg = self._load_plugin_conf(name) >> >>> + plugin_cls = cfg.get('wok', >>> {}).get('plugin_class', '') >> >> Same I commented above. plugin_class is not used anymore. > ok >> >>> + if plugin_cls == '': >>> + plugin_cls = ('plugins.%s.%s' % (name, >>> name[0].upper() + >>> + name[1:])) >>> + self._plugins_dict[name] = { >>> + 'config': cfg, >>> + 'enabled': cfg.get('wok', {}).get('enable', >>> False), >>> + 'uri': cfg.get('wok', {}).get('uri', ''), >>> + 'plugin_class': plugin_cls, >>> + 'conf_file': PluginPaths(name).conf_file >>> + } >>> + >> >>> + if 'wok' in self._plugins_dict[name]['config']: >>> + del self._plugins_dict[name]['config']['wok'] >>> + >> >> Usually the cherrypy configuration for a plugin can not be changed, >> so IMO it should not be part of plugin configuration file. >> If plugin code is responsible to loads its configuraiton, we don't >> need to merge the plugin configuration settings with cherrypy >> configuration and the above lines can be removed. >> > Did not understand your point here, can you explain better ? > What do you mean by "plugin code is responsible to loads its > configuration" ? > > Historically, the plugin configuration would contain information about the plugin itself (if it is enabled or not, the plugin base URI) and any other cherrypy configuration needed by the plugin. But after Wok creation, the cherrypy configuration needed by plugin was moved to the python code (WokRoot.get_custom_conf) - the class derived from WokRoot has a method called get_custom_conf() responsible to load the plugin configuration. So today, the plugin configuration file should only contain data about the plugin: if it is enabled or not, plugin base URI and any other information. For example, Kimchi has a [kimchi] section with other parameters. But the cherrypy configuration will be done on get_custom_conf() method. Said that, we don't need to merge the content of the plugin configuration file with the cherrypy configuration while loading the plugin as there is nothing there related to cherrypy. Does that make sense? I think Ginger is the unique plugin still using the plugin configuration file to handle cherrypy configuration. So if you decide to go what I suggested, you need to update the Ginger code.

>>> + def get_all_plugins_info(self): >>> + return copy.deepcopy(self._plugins_dict) >>> + >>> + def get_plugin_info(self, name): >>> + return copy.deepcopy(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): >>> + self._change_plugin_state(plugin, True) >>> + >>> + def disable_plugin(self, plugin): >>> + self._change_plugin_state(plugin, False) >> >> _______________________________________________ >> Kimchi-devel mailing list >> Kimchi-devel(a)ovirt.org >> http://lists.ovirt.org/mailman/listinfo/kimchi-devel >> > > _______________________________________________ > Kimchi-devel mailing list > Kimchi-devel(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel > _______________________________________________ Kimchi-devel mailing list Kimchi-devel(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel