[PATCH] [WoK 0/9] '/config/plugins' API implementation
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch set implements the '/config/plugins' API. The idea of this API is to replace the current '/plugins' API while adding a new attribute called 'enabled' in its return value, representing the current state of the plug-in. It will also return all the plug-ins, regardless of whether it is loaded or not. New actions /config/plugin/*name*/enable and /config/plugin/*name*/disable were implemented to enable/disable the plug-in *name* by editing the plug-in .conf file. This is the API in action: [danielhb@arthas wok_all_plugins]$ curl -k -u danielhb -H "Content-Type: application/json" -H "Accept: application/json" -X GET 'https://localhost:8001/config/plugins'Enter host password for user 'danielhb': [ { "enabled":true, "name":"gingerbase" }, { "enabled":true, "name":"kimchi" }, { "enabled":true, "name":"ginger" } ][danielhb@arthas wok_all_plugins]$ [danielhb@arthas wok_all_plugins]$ curl -k -u danielhb -H "Content-Type: application/json" -H "Accept: application/json" -X POST 'https://localhost:8001/config/plugins/ginger/disable' -d'{}' Enter host password for user 'danielhb': { "enabled":false, "name":"ginger" }[danielhb@arthas wok_all_plugins]$ [danielhb@arthas wok_all_plugins]$ curl -k -u danielhb -H "Content-Type: applicationjson" -H "Accept: application/json" -X POST 'https://localhost:8001/config/plugins/ginger/enable' -d'{}' Enter host password for user 'danielhb': { "enabled":true, "name":"ginger" }[danielhb@arthas wok_all_plugins]$ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Daniel Henrique Barboza (9): /config/plugins API: docs changes /config/plugins API: utils changes /config/plugins: changes in config model /config/plugins API: model changes /config/plugins API: test changes Removing old /plugins API /config/plugins: changing existing UI calls /config/plugins: exclude 'sample' plug-in when not in test_mode Removing configobj, use file operations to edit conf files docs/API/config.md | 31 +++++++++++++++++++++++ docs/API/plugins.md | 13 ---------- src/wok/control/config.py | 24 ++++++++++++++++-- src/wok/control/plugins.py | 29 --------------------- src/wok/i18n.py | 1 + src/wok/model/plugins.py | 32 +++++++++++++++++------ src/wok/server.py | 7 +++++- src/wok/utils.py | 63 ++++++++++++++++++++++++++++++++++++++++------ tests/test_api.py | 47 +++++++++++++++++++++++++++++++++- tests/test_utils.py | 56 ++++++++++++++++++++++++++++++++++++++++- ui/js/src/wok.api.js | 4 +-- ui/js/src/wok.logos.js | 11 +++++--- ui/js/src/wok.main.js | 12 ++++++--- 13 files changed, 258 insertions(+), 72 deletions(-) delete mode 100644 docs/API/plugins.md delete mode 100644 src/wok/control/plugins.py -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch adds a new API called /config/plugins in docs/API/config.md file. A new message was also added in i18n.py. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API/config.md | 31 +++++++++++++++++++++++++++++++ src/wok/i18n.py | 1 + 2 files changed, 32 insertions(+) diff --git a/docs/API/config.md b/docs/API/config.md index 0c273e2..22420f1 100644 --- a/docs/API/config.md +++ b/docs/API/config.md @@ -26,3 +26,34 @@ GET /config websockets_port: 64667, version: 2.0 } + +### Collection: Plugins + +**URI:** /config/plugins + +**Methods:** + +* **GET**: Retrieve a summarized list of all UI Plugins. + +#### Examples +GET /plugins +[{'name': 'pluginA', 'enabled': True}, {'name': 'pluginB', 'enabled': False}] + +### Resource: Plugins + +**URI:** /config/plugins/*:name* + +Represents the current state of a given WoK plug-in. + +**Methods:** + +* **GET**: Retrieve the state of the plug-in. + * name: The name of the plug-in. + * enabled: True if the plug-in is currently enabled in WoK, False otherwise. + +* **POST**: *See Plugin Actions* + +**Actions (POST):** + +* enable: Enable the plug-in in the configuration file. +* disable: Disable the plug-in in the configuration file. diff --git a/src/wok/i18n.py b/src/wok/i18n.py index e454e31..c96d14f 100644 --- a/src/wok/i18n.py +++ b/src/wok/i18n.py @@ -62,4 +62,5 @@ messages = { "WOKRES0001L": _("Request made on resource"), "WOKROOT0001L": _("User '%(username)s' login"), "WOKROOT0002L": _("User '%(username)s' logout"), + "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"), } -- 2.7.4
On 20/01/2017 10:15, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
This patch adds a new API called /config/plugins in docs/API/config.md file. A new message was also added in i18n.py.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API/config.md | 31 +++++++++++++++++++++++++++++++ src/wok/i18n.py | 1 + 2 files changed, 32 insertions(+)
diff --git a/docs/API/config.md b/docs/API/config.md index 0c273e2..22420f1 100644 --- a/docs/API/config.md +++ b/docs/API/config.md @@ -26,3 +26,34 @@ GET /config websockets_port: 64667, version: 2.0 } + +### Collection: Plugins + +**URI:** /config/plugins + +**Methods:** + +* **GET**: Retrieve a summarized list of all UI Plugins. + +#### Examples +GET /plugins +[{'name': 'pluginA', 'enabled': True}, {'name': 'pluginB', 'enabled': False}] + +### Resource: Plugins + +**URI:** /config/plugins/*:name* + +Represents the current state of a given WoK plug-in. + +**Methods:** + +* **GET**: Retrieve the state of the plug-in. + * name: The name of the plug-in. + * enabled: True if the plug-in is currently enabled in WoK, False otherwise. + +* **POST**: *See Plugin Actions* + +**Actions (POST):** + +* enable: Enable the plug-in in the configuration file. +* disable: Disable the plug-in in the configuration file. diff --git a/src/wok/i18n.py b/src/wok/i18n.py index e454e31..c96d14f 100644 --- a/src/wok/i18n.py +++ b/src/wok/i18n.py @@ -62,4 +62,5 @@ messages = { "WOKRES0001L": _("Request made on resource"), "WOKROOT0001L": _("User '%(username)s' login"), "WOKROOT0002L": _("User '%(username)s' logout"), + "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
To not mix with user log messages, add that message before those ending in L. Also, it would be nice to have user log messages for enable and disable plugin actions.
}
-- Lucio Correia
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> The default cherrypy parser WoK uses is unable to edit config files without removing the comments in it. This patch implements the 'set_plugin_state' function by using the default python parser, configobj.ConfigObj, to do the writing of the enabled = True|False entry in the plug-ins conf file. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/utils.py | 45 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/src/wok/utils.py b/src/wok/utils.py index 9a08001..e67b608 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -1,7 +1,7 @@ # # Project Wok # -# Copyright IBM Corp, 2015-2016 +# Copyright IBM Corp, 2015-2017 # # Code derived from Project Kimchi # @@ -35,6 +35,7 @@ import traceback import xml.etree.ElementTree as ET from cherrypy.lib.reprconf import Parser +from configobj import ConfigObj from datetime import datetime, timedelta from multiprocessing import Process, Queue from threading import Timer @@ -57,13 +58,21 @@ def is_digit(value): return False -def _load_plugin_conf(name): +def get_plugin_config_file(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 + return None + return plugin_conf + + +def load_plugin_conf(name): try: + plugin_conf = get_plugin_config_file(name) + if not plugin_conf: + return None + return Parser().dict_from_file(plugin_conf) except ValueError as e: cherrypy.log.error_log.error("Failed to load plugin " @@ -71,7 +80,7 @@ def _load_plugin_conf(name): (plugin_conf, e.message)) -def get_enabled_plugins(): +def get_plugins(enabled_only=False): plugin_dir = paths.plugins_dir try: dir_contents = os.listdir(plugin_dir) @@ -79,14 +88,36 @@ def get_enabled_plugins(): return for name in dir_contents: if os.path.isdir(os.path.join(plugin_dir, name)): - plugin_config = _load_plugin_conf(name) + plugin_config = load_plugin_conf(name) + if not plugin_config: + continue try: - if plugin_config['wok']['enable']: - yield (name, plugin_config) + if plugin_config['wok']['enable'] is None: + continue + + plugin_enabled = plugin_config['wok']['enable'] + if enabled_only and not plugin_enabled: + continue + + yield (name, plugin_config) except (TypeError, KeyError): continue +def get_enabled_plugins(): + return get_plugins(enabled_only=True) + + +def set_plugin_state(name, state): + plugin_conf = get_plugin_config_file(name) + if not plugin_conf: + return + + config = ConfigObj(plugin_conf) + config['wok']['enable'] = str(state) + config.write() + + def get_all_tabs(): files = [os.path.join(paths.ui_dir, 'config/tab-ext.xml')] -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Added the Plugins Collection and Plugin Resource classes in wok/control/config.py. Plugin is a Resource with two actions called 'enable' and 'disable'. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/control/config.py | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/wok/control/config.py b/src/wok/control/config.py index 338306c..c0679f3 100644 --- a/src/wok/control/config.py +++ b/src/wok/control/config.py @@ -1,7 +1,7 @@ # # Project Wok # -# Copyright IBM Corp, 2016 +# Copyright IBM Corp, 2016-2017 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -17,7 +17,7 @@ # License along with this library; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA -from wok.control.base import Resource +from wok.control.base import Collection, Resource from wok.control.utils import UrlSubNode @@ -25,6 +25,26 @@ from wok.control.utils import UrlSubNode class Config(Resource): def __init__(self, model, id=None): super(Config, self).__init__(model, id) + self.plugins = Plugins(self.model) + + @property + def data(self): + return self.info + + +class Plugins(Collection): + def __init__(self, model): + super(Plugins, self).__init__(model) + self.resource = Plugin + + +class Plugin(Resource): + def __init__(self, model, ident=None): + super(Plugin, self).__init__(model, ident) + self.ident = ident + self.uri_fmt = "/config/plugins/%s" + self.enable = self.generate_action_handler('enable') + self.disable = self.generate_action_handler('disable') @property def data(self): -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch changes the existing wok/model/plugins.py classes to match the new API. 'Plugins' is now a standard Collection with 'Plugin' as its Resource. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/model/plugins.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/src/wok/model/plugins.py b/src/wok/model/plugins.py index 1b8ec5e..42ca47b 100644 --- a/src/wok/model/plugins.py +++ b/src/wok/model/plugins.py @@ -1,7 +1,7 @@ # # Project Wok # -# Copyright IBM Corp, 2015-2016 +# Copyright IBM Corp, 2015-2017 # # Code derived from Project Kimchi # @@ -19,10 +19,9 @@ # 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 -from wok.config import get_base_plugin_uri -from wok.utils import get_enabled_plugins +from wok.exception import NotFoundError +from wok.utils import get_plugins, load_plugin_conf, set_plugin_state class PluginsModel(object): @@ -30,7 +29,24 @@ class PluginsModel(object): pass def get_list(self): - # Will only return plugins that were loaded correctly by WOK and are - # properly configured in cherrypy - return [plugin for (plugin, config) in get_enabled_plugins() - if get_base_plugin_uri(plugin) in cherrypy.tree.apps.keys()] + return [plugin for (plugin, config) in get_plugins()] + + +class PluginModel(object): + def __init__(self, **kargs): + pass + + def lookup(self, name): + name = name.encode('utf-8') + + plugin_conf = load_plugin_conf(name) + if not plugin_conf: + raise NotFoundError("WOKPLUGIN0001E", {'name': name}) + + return {"name": name, "enabled": plugin_conf['wok']['enable']} + + def enable(self, name): + set_plugin_state(name, True) + + def disable(self, name): + set_plugin_state(name, False) -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch added new tests in test_api.py and test_utils.py for the new /config/plugins API. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- tests/test_api.py | 43 ++++++++++++++++++++++++++++++++++++++++++- tests/test_utils.py | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 23c263d..1358151 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1,7 +1,7 @@ # # Project Wok # -# Copyright IBM Corp, 2016 +# Copyright IBM Corp, 2016-2017 # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -25,6 +25,8 @@ import utils from functools import partial from wok.asynctask import AsyncTask +from wok.utils import set_plugin_state +from wok.rollbackcontext import RollbackContext test_server = None model = None @@ -53,6 +55,45 @@ class APITests(unittest.TestCase): "server_root"] self.assertEquals(sorted(keys), sorted(conf.keys())) + def test_config_plugins(self): + resp = self.request('/config/plugins') + self.assertEquals(200, resp.status) + + plugins = json.loads(resp.read()) + if len(plugins) == 0: + return + + plugin = plugins[0] + plugin_state = plugin.get('enabled') + plugin_name = plugin.get('name') + + with RollbackContext() as rollback: + rollback.prependDefer(set_plugin_state, plugin_name, + plugin_state) + + resp = self.request('/config/plugins/%s' % plugin_name) + self.assertEquals(200, resp.status) + + resp = self.request('/config/plugins/%s/enable' % plugin_name, + '{}', 'POST') + self.assertEquals(200, resp.status) + + resp = self.request('/config/plugins') + self.assertEquals(200, resp.status) + plugins = json.loads(resp.read()) + plugin = plugins[0] + self.assertEqual(plugin.get('enabled'), True) + + resp = self.request('/config/plugins/%s/disable' % plugin_name, + '{}', 'POST') + self.assertEquals(200, resp.status) + + resp = self.request('/config/plugins') + self.assertEquals(200, resp.status) + plugins = json.loads(resp.read()) + plugin = plugins[0] + self.assertEqual(plugin.get('enabled'), False) + def test_user_log(self): # Login and logout to make sure there there are entries in user log hdrs = {'AUTHORIZATION': '', diff --git a/tests/test_utils.py b/tests/test_utils.py index e7fd264..c0e1009 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -19,10 +19,14 @@ # 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 mock +import os +import tempfile import unittest from wok.exception import InvalidParameter -from wok.utils import convert_data_size +from wok.rollbackcontext import RollbackContext +from wok.utils import convert_data_size, set_plugin_state class UtilsTests(unittest.TestCase): @@ -69,3 +73,48 @@ class UtilsTests(unittest.TestCase): for d in success_data: self.assertEquals(d['got'], d['want']) + + def _get_fake_config_file_content(self, enable=True): + return """\ +[wok] +# Enable plugin on Wok server (values: True|False) +enable = %s + +[fakeplugin] +# Yet another comment on this config file +very_interesting_option = True +""" % str(enable) + + def _create_fake_config_file(self): + _, tmp_file_name = tempfile.mkstemp(suffix='.conf') + + config_contents = self._get_fake_config_file_content() + with open(tmp_file_name, 'w') as f: + f.writelines(config_contents) + + return tmp_file_name + + @mock.patch('wok.utils.get_plugin_config_file') + def test_set_plugin_state(self, mock_config_file): + with RollbackContext() as rollback: + + config_file_name = self._create_fake_config_file() + rollback.prependDefer(os.remove, config_file_name) + + mock_config_file.return_value = config_file_name + + set_plugin_state('pluginA', False) + with open(config_file_name, 'r') as f: + updated_conf = f.read() + self.assertEqual( + updated_conf, + self._get_fake_config_file_content(enable=False) + ) + + set_plugin_state('pluginA', True) + with open(config_file_name, 'r') as f: + updated_conf = f.read() + self.assertEqual( + updated_conf, + self._get_fake_config_file_content(enable=True) + ) -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This API was behaving as a SimpleCollection but with the newer model from the new /config/plugins API. It makes little sense to keep it like that. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- docs/API/plugins.md | 13 ------------- src/wok/control/plugins.py | 29 ----------------------------- tests/test_api.py | 4 ++++ 3 files changed, 4 insertions(+), 42 deletions(-) delete mode 100644 docs/API/plugins.md delete mode 100644 src/wok/control/plugins.py diff --git a/docs/API/plugins.md b/docs/API/plugins.md deleted file mode 100644 index aaa37b5..0000000 --- a/docs/API/plugins.md +++ /dev/null @@ -1,13 +0,0 @@ -## REST API Specification for Plugins - -### Collection: Plugins - -**URI:** /plugins - -**Methods:** - -* **GET**: Retrieve a summarized list names of all UI Plugins - -#### Examples -GET /plugins -[pluginA, pluginB, pluginC] diff --git a/src/wok/control/plugins.py b/src/wok/control/plugins.py deleted file mode 100644 index 57dfa1b..0000000 --- a/src/wok/control/plugins.py +++ /dev/null @@ -1,29 +0,0 @@ -# -# Project Wok -# -# Copyright IBM Corp, 2015-2016 -# -# Code derived from Project 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 - -from wok.control.base import SimpleCollection -from wok.control.utils import UrlSubNode - - -@UrlSubNode("plugins") -class Plugins(SimpleCollection): - def __init__(self, model): - super(Plugins, self).__init__(model) diff --git a/tests/test_api.py b/tests/test_api.py index 1358151..127e94a 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -94,6 +94,10 @@ class APITests(unittest.TestCase): plugin = plugins[0] self.assertEqual(plugin.get('enabled'), False) + def test_plugins_api_404(self): + resp = self.request('/plugins') + self.assertEquals(404, resp.status) + def test_user_log(self): # Login and logout to make sure there there are entries in user log hdrs = {'AUTHORIZATION': '', -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> This patch changes ui/js/src/wok.api.js 'listPlugins' method to use the URL /config/plugins instead of /plugins. With this change, ui/js/src/wok.logos.js and ui/js/src/wok.main.js were also changed to handle the different return value from the /config/plugins API. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- ui/js/src/wok.api.js | 4 ++-- ui/js/src/wok.logos.js | 11 +++++++---- ui/js/src/wok.main.js | 12 ++++++++---- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/ui/js/src/wok.api.js b/ui/js/src/wok.api.js index e2829ab..c465dfb 100644 --- a/ui/js/src/wok.api.js +++ b/ui/js/src/wok.api.js @@ -1,7 +1,7 @@ /* * Project Wok * - * Copyright IBM Corp, 2015-2016 + * Copyright IBM Corp, 2015-2017 * * Code derived from Project Kimchi * @@ -111,7 +111,7 @@ var wok = { listPlugins : function(suc, err, sync) { wok.requestJSON({ - url : 'plugins', + url : '/config/plugins', type : 'GET', contentType : 'application/json', dataType : 'json', diff --git a/ui/js/src/wok.logos.js b/ui/js/src/wok.logos.js index a825108..b4e2d75 100644 --- a/ui/js/src/wok.logos.js +++ b/ui/js/src/wok.logos.js @@ -1,7 +1,7 @@ /* * Project Wok * - * Copyright IBM Corp, 2016 + * Copyright IBM Corp, 2016-2017 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -78,18 +78,21 @@ wok.logos = function(element, powered) { wok.listPlugins(function(plugins) { if(plugins && plugins.length > 0) { $(plugins).each(function(i, p) { + if (p.enabled === false) { + return true; + } var url = wok.substitute(pluginUrl, { - plugin: p + plugin: p.name }); obj[i] = { - name : p + name : p.name } var pluginVersions; pluginVersions = retrieveVersion(url); if(pluginVersions && pluginVersions.length > 0){ obj[i].version = pluginVersions; } - var imagepath = url+'/images/'+p; + var imagepath = url+'/images/'+p.name; if(checkImage(imagepath+'.svg') == 200) { obj[i].image = imagepath+'.svg'; } diff --git a/ui/js/src/wok.main.js b/ui/js/src/wok.main.js index 658974a..30ce5bb 100644 --- a/ui/js/src/wok.main.js +++ b/ui/js/src/wok.main.js @@ -1,7 +1,7 @@ /* * Project Wok * - * Copyright IBM Corp, 2015-2016 + * Copyright IBM Corp, 2015-2017 * * Code derived from Project Kimchi * @@ -143,16 +143,20 @@ wok.main = function() { var tabs = retrieveTabs('wok', wokConfigUrl); wok.listPlugins(function(plugins) { $(plugins).each(function(i, p) { + if (p.enabled === false) { + return true; + } + var url = wok.substitute(pluginConfigUrl, { - plugin: p + plugin: p.name }); var i18nUrl = wok.substitute(pluginI18nUrl, { - plugin: p + plugin: p.name }); wok.getI18n(function(i18nObj){ $.extend(i18n, i18nObj)}, function(i18nObj){ //i18n is not define by plugin }, i18nUrl, true); - var pluginTabs = retrieveTabs(p, url); + var pluginTabs = retrieveTabs(p.name, url); if(pluginTabs.length > 0){ tabs.push.apply(tabs, pluginTabs); } -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> The 'sample' plug-in matches the criteria for a valid disabled plug-in. However 'sample' is a plug-in made for demonstration purposes and it is not supposed to be shown in WoK with other 'real' plug-ins. This patch adds the test_mode option in cherrypy.config inside server.py, making it readable across other modules. This option is then read at 'get_plugins' of utils.py and, if the server isn't running in test_mode, discard the sample plug-in entry. Othewise (unit tests using run_server for example) we can safely show it. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/server.py | 7 ++++++- src/wok/utils.py | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/wok/server.py b/src/wok/server.py index 48f455b..2065dc4 100644 --- a/src/wok/server.py +++ b/src/wok/server.py @@ -1,7 +1,7 @@ # # Project Wok # -# Copyright IBM Corp, 2015-2016 +# Copyright IBM Corp, 2015-2017 # # Code derived from Project Kimchi # @@ -139,6 +139,11 @@ class Server(object): if not dev_env: cherrypy.config.update({'environment': 'production'}) + test_mode = False + if hasattr(options, 'test') and options.test is True: + test_mode = True + cherrypy.config.update({'test_mode': test_mode}) + if hasattr(options, 'model'): model_instance = options.model else: diff --git a/src/wok/utils.py b/src/wok/utils.py index e67b608..d841371 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -88,6 +88,10 @@ def get_plugins(enabled_only=False): return for name in dir_contents: if os.path.isdir(os.path.join(plugin_dir, name)): + test_mode = cherrypy.config.get('test_mode', False) + if name == 'sample' and not test_mode: + continue + plugin_config = load_plugin_conf(name) if not plugin_config: continue -- 2.7.4
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> Before the implementation of the /config/plugins API, WoK used cherrypy.lib.reprconf.Parser to parse the .conf files of the plug-ins. In the implementation of this API we have the need to edit these config files, setting the 'enable = ' line from the WoK section to either True or False. Using this same parser for writing the files presented to be a problem because the writing process were deleting the comments presented in the conf files. No option is available to make the write() call of cherrypy.lib.reprconf.Parser preserve the comments. To solve this, the implementation of /config/plugins introduced configobj.ConfigObj, a python standard parser, to make this write operation in the conf files. The write() call of this parser retains the comments. So we ended up using cherrypy.lib.reprconf.Parser to parse the conf files to the plug-ins and configobj.ConfigObj to edit/write the 'enable =' option. It is worth mentioning that although cherrypy.lib.reprconf.Parser implementation extends configobj.ConfigObj, his write() call extends another class called 'backports.configparser.RawConfigParser'. This explains why the write() call from both parsers differs. A problem in this approach was detected when dealing with the conf file of the Ginger plug-in. This is the relevant portion of the Ginger conf file: [backup] default_include = ['/etc', '/var/spool/cron'] default_exclude = ['/etc/init.d', '/etc/rc.d', '/etc/rc?.d'] This declaration of default_include and default_exclude is invalid to the configobj.ConfigObj parser:
import configobj parser = configobj.ConfigObj('ginger.conf') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/site-packages/configobj.py", line 1229, in __init__ self._load(infile, configspec) File "/usr/lib/python2.7/site-packages/configobj.py", line 1318, in _load raise error configobj.ConfigObjError: Parsing failed with several errors. First error at line 6.
This resulted in the /enable and /disable plug-ins being broken when dealing with Ginger. After experimentation with the ginger.conf format no common ground was found between both parsers to proper handle this list declaration of the Ginger file. One solution that was speculated was to get rid of cherrypy.lib.reprconf.Parser altogether and use configobj.ConfigObj in all cases, meaning that we would use it to parse the conf files of the plug-ins too. At first only the Ginger conf file was going to be changed but then we realized that ConfigObj parses boolean in a different fashion too. This is the parsing of the current kimchi.conf file using both parsers:
import configobj parser = configobj.ConfigObj('kimchi.conf') parser.dict() {'kimchi': {'create_iso_pool': 'True', 'federation': 'False'}, 'wok': {'enable': 'True'}}
from cherrypy.lib.reprconf import Parser Parser().dict_from_file('kimchi.conf') {'kimchi': {'create_iso_pool': True, 'federation': False}, 'wok': {'enable': True}}
cherrypy.lib.reprconf.Parser delivers boolean values in the dict, ConfigObj delivers string values 'True' and 'False'. This means that the change to use ConfigObj in the parsing of the conf files would need either: • change Kimchi and Gingerbase plug-ins to handle the now string values 'True' or 'False' in its backend logic or • WoK would need to convert the string values to boolean before sending it to the plug-ins. This can be annoying considering that the conf file supports multiple sections and subsections. The saner solution this patch implements is to get rid of configobj.ConfigObj to do the write of the conf files. This is now being manually made with standard Python file operations. The file editing required in this feature is too simple to warrant for all this hassle with two differents parsers, different INI formats and so on. A unit test was edited to make the .conf file a little trickier to ensure that our file operation is editing and writing the right attribute of the conf file. Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/utils.py | 22 ++++++++++++++++++---- tests/test_utils.py | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/wok/utils.py b/src/wok/utils.py index d841371..c2d4724 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -35,7 +35,6 @@ import traceback import xml.etree.ElementTree as ET from cherrypy.lib.reprconf import Parser -from configobj import ConfigObj from datetime import datetime, timedelta from multiprocessing import Process, Queue from threading import Timer @@ -117,9 +116,24 @@ def set_plugin_state(name, state): if not plugin_conf: return - config = ConfigObj(plugin_conf) - config['wok']['enable'] = str(state) - config.write() + config_contents = None + + with open(plugin_conf, 'r') as f: + config_contents = f.readlines() + + wok_section_found = False + + for i in range(0, len(config_contents)): + if config_contents[i] == '[wok]\n': + wok_section_found = True + continue + + if config_contents[i].startswith('enable =') and wok_section_found: + config_contents[i] = 'enable = %s\n' % str(state) + break + + with open(plugin_conf, 'w') as f: + f.writelines(config_contents) def get_all_tabs(): diff --git a/tests/test_utils.py b/tests/test_utils.py index c0e1009..6f1a783 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -76,12 +76,17 @@ class UtilsTests(unittest.TestCase): def _get_fake_config_file_content(self, enable=True): return """\ +[a_random_section] +# a random section for testing purposes +enable = 1 + [wok] # Enable plugin on Wok server (values: True|False) enable = %s [fakeplugin] # Yet another comment on this config file +enable = 2 very_interesting_option = True """ % str(enable) -- 2.7.4
On 20/01/2017 10:15, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Before the implementation of the /config/plugins API, WoK used cherrypy.lib.reprconf.Parser to parse the .conf files of the plug-ins. In the implementation of this API we have the need to edit these config files, setting the 'enable = ' line from the WoK section to either True or False. Using this same parser for writing the files presented to be a problem because the writing process were deleting the comments presented in the conf files. No option is available to make the write() call of cherrypy.lib.reprconf.Parser preserve the comments.
To solve this, the implementation of /config/plugins introduced configobj.ConfigObj, a python standard parser, to make this write operation in the conf files. The write() call of this parser retains the comments. So we ended up using cherrypy.lib.reprconf.Parser to parse the conf files to the plug-ins and configobj.ConfigObj to edit/write the 'enable =' option. It is worth mentioning that although cherrypy.lib.reprconf.Parser implementation extends configobj.ConfigObj, his write() call extends another class called 'backports.configparser.RawConfigParser'. This explains why the write() call from both parsers differs.
A problem in this approach was detected when dealing with the conf file of the Ginger plug-in. This is the relevant portion of the Ginger conf file:
[backup] default_include = ['/etc', '/var/spool/cron'] default_exclude = ['/etc/init.d', '/etc/rc.d', '/etc/rc?.d']
This declaration of default_include and default_exclude is invalid to the configobj.ConfigObj parser:
import configobj parser = configobj.ConfigObj('ginger.conf') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/site-packages/configobj.py", line 1229, in __init__ self._load(infile, configspec) File "/usr/lib/python2.7/site-packages/configobj.py", line 1318, in _load raise error configobj.ConfigObjError: Parsing failed with several errors. First error at line 6.
This resulted in the /enable and /disable plug-ins being broken when dealing with Ginger. After experimentation with the ginger.conf format no common ground was found between both parsers to proper handle this list declaration of the Ginger file.
One solution that was speculated was to get rid of cherrypy.lib.reprconf.Parser altogether and use configobj.ConfigObj in all cases, meaning that we would use it to parse the conf files of the plug-ins too. At first only the Ginger conf file was going to be changed but then we realized that ConfigObj parses boolean in a different fashion too.
This is the parsing of the current kimchi.conf file using both parsers:
import configobj parser = configobj.ConfigObj('kimchi.conf') parser.dict() {'kimchi': {'create_iso_pool': 'True', 'federation': 'False'}, 'wok': {'enable': 'True'}}
from cherrypy.lib.reprconf import Parser Parser().dict_from_file('kimchi.conf') {'kimchi': {'create_iso_pool': True, 'federation': False}, 'wok': {'enable': True}}
cherrypy.lib.reprconf.Parser delivers boolean values in the dict, ConfigObj delivers string values 'True' and 'False'. This means that the change to use ConfigObj in the parsing of the conf files would need either:
• change Kimchi and Gingerbase plug-ins to handle the now string values 'True' or 'False' in its backend logic
or
• WoK would need to convert the string values to boolean before sending it to the plug-ins. This can be annoying considering that the conf file supports multiple sections and subsections.
The saner solution this patch implements is to get rid of configobj.ConfigObj to do the write of the conf files. This is now being manually made with standard Python file operations. The file editing required in this feature is too simple to warrant for all this hassle with two differents parsers, different INI formats and so on.
A unit test was edited to make the .conf file a little trickier to ensure that our file operation is editing and writing the right attribute of the conf file.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/utils.py | 22 ++++++++++++++++++---- tests/test_utils.py | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/wok/utils.py b/src/wok/utils.py index d841371..c2d4724 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -35,7 +35,6 @@ import traceback import xml.etree.ElementTree as ET
from cherrypy.lib.reprconf import Parser -from configobj import ConfigObj from datetime import datetime, timedelta from multiprocessing import Process, Queue from threading import Timer @@ -117,9 +116,24 @@ def set_plugin_state(name, state): if not plugin_conf: return
- config = ConfigObj(plugin_conf) - config['wok']['enable'] = str(state) - config.write() + config_contents = None + + with open(plugin_conf, 'r') as f: + config_contents = f.readlines() + + wok_section_found = False + + for i in range(0, len(config_contents)): + if config_contents[i] == '[wok]\n': + wok_section_found = True + continue + + if config_contents[i].startswith('enable =') and wok_section_found: + config_contents[i] = 'enable = %s\n' % str(state) + break + + with open(plugin_conf, 'w') as f: + f.writelines(config_contents)
def get_all_tabs(): diff --git a/tests/test_utils.py b/tests/test_utils.py index c0e1009..6f1a783 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -76,12 +76,17 @@ class UtilsTests(unittest.TestCase):
def _get_fake_config_file_content(self, enable=True): return """\ +[a_random_section] +# a random section for testing purposes +enable = 1 + [wok] # Enable plugin on Wok server (values: True|False) enable = %s
[fakeplugin] # Yet another comment on this config file +enable = 2 very_interesting_option = True """ % str(enable)
I believe we may have complaints about problems when user uses: enabled=1 (without spaces). This is supported by INI files and we should handle it as well. -- Lucio Correia
On 01/20/2017 11:24 AM, Lucio Correia wrote:
On 20/01/2017 10:15, dhbarboza82@gmail.com wrote:
From: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
Before the implementation of the /config/plugins API, WoK used cherrypy.lib.reprconf.Parser to parse the .conf files of the plug-ins. In the implementation of this API we have the need to edit these config files, setting the 'enable = ' line from the WoK section to either True or False. Using this same parser for writing the files presented to be a problem because the writing process were deleting the comments presented in the conf files. No option is available to make the write() call of cherrypy.lib.reprconf.Parser preserve the comments.
To solve this, the implementation of /config/plugins introduced configobj.ConfigObj, a python standard parser, to make this write operation in the conf files. The write() call of this parser retains the comments. So we ended up using cherrypy.lib.reprconf.Parser to parse the conf files to the plug-ins and configobj.ConfigObj to edit/write the 'enable =' option. It is worth mentioning that although cherrypy.lib.reprconf.Parser implementation extends configobj.ConfigObj, his write() call extends another class called 'backports.configparser.RawConfigParser'. This explains why the write() call from both parsers differs.
A problem in this approach was detected when dealing with the conf file of the Ginger plug-in. This is the relevant portion of the Ginger conf file:
[backup] default_include = ['/etc', '/var/spool/cron'] default_exclude = ['/etc/init.d', '/etc/rc.d', '/etc/rc?.d']
This declaration of default_include and default_exclude is invalid to the configobj.ConfigObj parser:
import configobj parser = configobj.ConfigObj('ginger.conf') Traceback (most recent call last): File "<stdin>", line 1, in <module> File "/usr/lib/python2.7/site-packages/configobj.py", line 1229, in __init__ self._load(infile, configspec) File "/usr/lib/python2.7/site-packages/configobj.py", line 1318, in _load raise error configobj.ConfigObjError: Parsing failed with several errors. First error at line 6.
This resulted in the /enable and /disable plug-ins being broken when dealing with Ginger. After experimentation with the ginger.conf format no common ground was found between both parsers to proper handle this list declaration of the Ginger file.
One solution that was speculated was to get rid of cherrypy.lib.reprconf.Parser altogether and use configobj.ConfigObj in all cases, meaning that we would use it to parse the conf files of the plug-ins too. At first only the Ginger conf file was going to be changed but then we realized that ConfigObj parses boolean in a different fashion too.
This is the parsing of the current kimchi.conf file using both parsers:
import configobj parser = configobj.ConfigObj('kimchi.conf') parser.dict() {'kimchi': {'create_iso_pool': 'True', 'federation': 'False'}, 'wok': {'enable': 'True'}}
from cherrypy.lib.reprconf import Parser Parser().dict_from_file('kimchi.conf') {'kimchi': {'create_iso_pool': True, 'federation': False}, 'wok': {'enable': True}}
cherrypy.lib.reprconf.Parser delivers boolean values in the dict, ConfigObj delivers string values 'True' and 'False'. This means that the change to use ConfigObj in the parsing of the conf files would need either:
• change Kimchi and Gingerbase plug-ins to handle the now string values 'True' or 'False' in its backend logic
or
• WoK would need to convert the string values to boolean before sending it to the plug-ins. This can be annoying considering that the conf file supports multiple sections and subsections.
The saner solution this patch implements is to get rid of configobj.ConfigObj to do the write of the conf files. This is now being manually made with standard Python file operations. The file editing required in this feature is too simple to warrant for all this hassle with two differents parsers, different INI formats and so on.
A unit test was edited to make the .conf file a little trickier to ensure that our file operation is editing and writing the right attribute of the conf file.
Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com> --- src/wok/utils.py | 22 ++++++++++++++++++---- tests/test_utils.py | 5 +++++ 2 files changed, 23 insertions(+), 4 deletions(-)
diff --git a/src/wok/utils.py b/src/wok/utils.py index d841371..c2d4724 100644 --- a/src/wok/utils.py +++ b/src/wok/utils.py @@ -35,7 +35,6 @@ import traceback import xml.etree.ElementTree as ET
from cherrypy.lib.reprconf import Parser -from configobj import ConfigObj from datetime import datetime, timedelta from multiprocessing import Process, Queue from threading import Timer @@ -117,9 +116,24 @@ def set_plugin_state(name, state): if not plugin_conf: return
- config = ConfigObj(plugin_conf) - config['wok']['enable'] = str(state) - config.write() + config_contents = None + + with open(plugin_conf, 'r') as f: + config_contents = f.readlines() + + wok_section_found = False + + for i in range(0, len(config_contents)): + if config_contents[i] == '[wok]\n': + wok_section_found = True + continue + + if config_contents[i].startswith('enable =') and wok_section_found: + config_contents[i] = 'enable = %s\n' % str(state) + break + + with open(plugin_conf, 'w') as f: + f.writelines(config_contents)
def get_all_tabs(): diff --git a/tests/test_utils.py b/tests/test_utils.py index c0e1009..6f1a783 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -76,12 +76,17 @@ class UtilsTests(unittest.TestCase):
def _get_fake_config_file_content(self, enable=True): return """\ +[a_random_section] +# a random section for testing purposes +enable = 1 + [wok] # Enable plugin on Wok server (values: True|False) enable = %s
[fakeplugin] # Yet another comment on this config file +enable = 2 very_interesting_option = True """ % str(enable)
I believe we may have complaints about problems when user uses: enabled=1 (without spaces).
This is supported by INI files and we should handle it as well.
I'll add support for this format in the v2
participants (3)
-
Daniel Henrique Barboza -
dhbarboza82＠gmail.com -
Lucio Correia