11:32 a.m.
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
v2:
- added User Log capabilities on /config/plugins/enable|disable actions
- added 'enable=' as a valid entry in the parsing of the conf file
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 (10):
/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
/config/plugins API: user log and parser improvements
docs/API/config.md | 31 ++++++++++++++++++++
docs/API/plugins.md | 13 ---------
src/wok/control/config.py | 33 +++++++++++++++++++--
src/wok/control/plugins.py | 29 -------------------
src/wok/i18n.py | 4 +++
src/wok/model/plugins.py | 32 +++++++++++++++------
src/wok/server.py | 7 ++++-
src/wok/utils.py | 65 ++++++++++++++++++++++++++++++++++++-----
tests/test_api.py | 47 +++++++++++++++++++++++++++++-
tests/test_utils.py | 72 +++++++++++++++++++++++++++++++++++++++++++++-
ui/js/src/wok.api.js | 4 +--
ui/js/src/wok.logos.js | 11 ++++---
ui/js/src/wok.main.js | 12 +++++---
13 files changed, 288 insertions(+), 72 deletions(-)
delete mode 100644 docs/API/plugins.md
delete mode 100644 src/wok/control/plugins.py
--
2.7.4
11:32 a.m.
New subject: [PATCH] [WoK 01/10] /config/plugins API: docs changes
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 02/10] /config/plugins API: utils changes
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 03/10] /config/plugins: changes in config model
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 04/10] /config/plugins API: model changes
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 05/10] /config/plugins API: test changes
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 06/10] Removing old /plugins API
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:32 a.m.
New subject: [PATCH] [WoK 07/10] /config/plugins: changing existing UI calls
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:23 a.m.
New subject: [PATCH] [WoK 07/10] /config/plugins: changing existing UI calls
Please, merge this patch with 6/10 to have atomic commits.
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)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(a)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);
}
11:32 a.m.
New subject: [PATCH] [WoK 08/10] /config/plugins: exclude 'sample' plug-in when not in test_mode
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:29 a.m.
New subject: [PATCH] [WoK 08/10] /config/plugins: exclude 'sample' plug-in when not in test_mode
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)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(a)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})
+
I don't think we need to use cherrypy to handle a information only
needed for configuraiton.
Why do not pass it as parameter to get_enabled_plugins function() ?
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
12:59 p.m.
New subject: [PATCH] [WoK 08/10] /config/plugins: exclude 'sample' plug-in when not in test_mode
On 01/25/2017 03:29 PM, Aline Manera wrote:
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
> From: Daniel Henrique Barboza <danielhb(a)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(a)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})
> +
I don't think we need to use cherrypy to handle a information only
needed for configuraiton.
Why do not pass it as parameter to get_enabled_plugins function() ?
I did that. I had to pass the same parameter along other functions as
well, get_plugins
for example.
I decided to go with this solution instead because I think it's cleaner
than passing it
as a parameter from server.py. It also enables the info to be fetched
anywhere in the
code, including plug-ins.
Also, the documentation about cherrypy.config states:
"
config: a file or dict containing application config. If this
contains
a [global] section, those entries will be used in the global
(site-wide) config.
"
So I think it's a good place to store the 'test_mode' info.
> 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
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
1:34 p.m.
New subject: [PATCH] [WoK 08/10] /config/plugins: exclude 'sample' plug-in when not in test_mode
On 01/25/2017 04:59 PM, Daniel Henrique Barboza wrote:
On 01/25/2017 03:29 PM, Aline Manera wrote:
>
>
> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>> From: Daniel Henrique Barboza <danielhb(a)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(a)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})
>> +
>
> I don't think we need to use cherrypy to handle a information only
> needed for configuraiton.
> Why do not pass it as parameter to get_enabled_plugins function() ?
I did that. I had to pass the same parameter along other functions as
well, get_plugins
for example.
I decided to go with this solution instead because I think it's
cleaner than passing it
as a parameter from server.py. It also enables the info to be fetched
anywhere in the
code, including plug-ins.
The plugins already have access to this info through server options as
well. It is using that that Kimchi runs on MockModel when on --test.
Also, the documentation about cherrypy.config states:
"
config: a file or dict containing application config. If this
contains
a [global] section, those entries will be used in the global
(site-wide) config.
"
So I think it's a good place to store the 'test_mode' info.
>
>> 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
>
> _______________________________________________
> 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
1:35 p.m.
New subject: [PATCH] [WoK 08/10] /config/plugins: exclude 'sample' plug-in when not in test_mode
On 01/25/2017 04:59 PM, Daniel Henrique Barboza wrote:
On 01/25/2017 03:29 PM, Aline Manera wrote:
>
>
> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>> From: Daniel Henrique Barboza <danielhb(a)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(a)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})
>> +
>
> I don't think we need to use cherrypy to handle a information only
> needed for configuraiton.
> Why do not pass it as parameter to get_enabled_plugins function() ?
I did that. I had to pass the same parameter along other functions as
well, get_plugins
for example.
I decided to go with this solution instead because I think it's
cleaner than passing it
as a parameter from server.py. It also enables the info to be fetched
anywhere in the
code, including plug-ins.
Also, the documentation about cherrypy.config states:
"
config: a file or dict containing application config. If this
contains
a [global] section, those entries will be used in the global
(site-wide) config.
"
How do you set it as global to prevent other plugins to override it?
So I think it's a good place to store the 'test_mode'
info.
>
>> 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
>
> _______________________________________________
> 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
11:32 a.m.
New subject: [PATCH] [WoK 09/10] Removing configobj, use file operations to edit conf files
From: Daniel Henrique Barboza <danielhb(a)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(a)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
11:33 a.m.
New subject: [PATCH] [WoK 09/10] Removing configobj, use file operations to edit conf files
So you have created a function that does not work well on patch 2/10 to
rewrite it again here?
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)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(a)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)
1:03 p.m.
New subject: [PATCH] [WoK 09/10] Removing configobj, use file operations to edit conf files
On 01/25/2017 03:33 PM, Aline Manera wrote:
So you have created a function that does not work well on patch 2/10
to rewrite it again here?
Yeah, that was exactly the reason I created this commit.
...
I'll merge all backend patches in a single patch in v3.
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
> From: Daniel Henrique Barboza <danielhb(a)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(a)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)
>
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
11:32 a.m.
New subject: [PATCH] [WoK 10/10] /config/plugins API: user log and parser improvements
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
- added user log capabilities on 'enable' and 'disable' actions
of /config/plugins/*name* API
- parser now accepts 'enable=' as a valid input to recognize the
plug-in option that enables or disables a plug-in.
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/wok/control/config.py | 9 +++++++++
src/wok/i18n.py | 5 ++++-
src/wok/utils.py | 4 +++-
tests/test_utils.py | 20 ++++++++++++++++++--
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/src/wok/control/config.py b/src/wok/control/config.py
index c0679f3..db61060 100644
--- a/src/wok/control/config.py
+++ b/src/wok/control/config.py
@@ -21,6 +21,14 @@ from wok.control.base import Collection, Resource
from wok.control.utils import UrlSubNode
+PLUGIN_REQUESTS = {
+ 'POST': {
+ 'enable': "WOKPLUGIN0001L",
+ 'disable': "WOKPLUGIN0002L",
+ },
+}
+
+
@UrlSubNode("config")
class Config(Resource):
def __init__(self, model, id=None):
@@ -43,6 +51,7 @@ class Plugin(Resource):
super(Plugin, self).__init__(model, ident)
self.ident = ident
self.uri_fmt = "/config/plugins/%s"
+ self.log_map = PLUGIN_REQUESTS
self.enable = self.generate_action_handler('enable')
self.disable = self.generate_action_handler('disable')
diff --git a/src/wok/i18n.py b/src/wok/i18n.py
index c96d14f..95ec20f 100644
--- a/src/wok/i18n.py
+++ b/src/wok/i18n.py
@@ -55,6 +55,8 @@ messages = {
"WOKUTILS0004E": _("Invalid data value '%(value)s'"),
"WOKUTILS0005E": _("Invalid data unit '%(unit)s'"),
+ "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
+
# These messages (ending with L) are for user log purposes
"WOKASYNC0001L": _("Successfully completed task
'%(target_uri)s'"),
"WOKASYNC0002L": _("Failed to complete task
'%(target_uri)s'"),
@@ -62,5 +64,6 @@ messages = {
"WOKRES0001L": _("Request made on resource"),
"WOKROOT0001L": _("User '%(username)s' login"),
"WOKROOT0002L": _("User '%(username)s' logout"),
- "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
+ "WOKPLUGIN0001L": _("Enabling plug-in %(ident)s."),
+ "WOKPLUGIN0002L": _("Disabling plug-in %(ident)s."),
}
diff --git a/src/wok/utils.py b/src/wok/utils.py
index c2d4724..5e87ee0 100644
--- a/src/wok/utils.py
+++ b/src/wok/utils.py
@@ -128,7 +128,9 @@ def set_plugin_state(name, state):
wok_section_found = True
continue
- if config_contents[i].startswith('enable =') and wok_section_found:
+ if (config_contents[i].startswith('enable =') or
+ config_contents[i].startswith('enable=')) and \
+ wok_section_found:
config_contents[i] = 'enable = %s\n' % str(state)
break
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 6f1a783..06930b8 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -82,6 +82,22 @@ 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)
+
+ def _get_config_file_template(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]
@@ -113,7 +129,7 @@ very_interesting_option = True
updated_conf = f.read()
self.assertEqual(
updated_conf,
- self._get_fake_config_file_content(enable=False)
+ self._get_config_file_template(enable=False)
)
set_plugin_state('pluginA', True)
@@ -121,5 +137,5 @@ very_interesting_option = True
updated_conf = f.read()
self.assertEqual(
updated_conf,
- self._get_fake_config_file_content(enable=True)
+ self._get_config_file_template(enable=True)
)
--
2.7.4
11:34 a.m.
New subject: [PATCH] [WoK 10/10] /config/plugins API: user log and parser improvements
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
- added user log capabilities on 'enable' and 'disable' actions
of /config/plugins/*name* API
- parser now accepts 'enable=' as a valid input to recognize the
plug-in option that enables or disables a plug-in.
Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
---
src/wok/control/config.py | 9 +++++++++
src/wok/i18n.py | 5 ++++-
src/wok/utils.py | 4 +++-
tests/test_utils.py | 20 ++++++++++++++++++--
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/src/wok/control/config.py b/src/wok/control/config.py
index c0679f3..db61060 100644
--- a/src/wok/control/config.py
+++ b/src/wok/control/config.py
@@ -21,6 +21,14 @@ from wok.control.base import Collection, Resource
from wok.control.utils import UrlSubNode
+PLUGIN_REQUESTS = {
+ 'POST': {
+ 'enable': "WOKPLUGIN0001L",
+ 'disable': "WOKPLUGIN0002L",
+ },
+}
+
+
@UrlSubNode("config")
class Config(Resource):
def __init__(self, model, id=None):
@@ -43,6 +51,7 @@ class Plugin(Resource):
super(Plugin, self).__init__(model, ident)
self.ident = ident
self.uri_fmt = "/config/plugins/%s"
+ self.log_map = PLUGIN_REQUESTS
self.enable = self.generate_action_handler('enable')
self.disable = self.generate_action_handler('disable')
diff --git a/src/wok/i18n.py b/src/wok/i18n.py
index c96d14f..95ec20f 100644
--- a/src/wok/i18n.py
+++ b/src/wok/i18n.py
@@ -55,6 +55,8 @@ messages = {
"WOKUTILS0004E": _("Invalid data value '%(value)s'"),
"WOKUTILS0005E": _("Invalid data unit '%(unit)s'"),
+ "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
+
# These messages (ending with L) are for user log purposes
"WOKASYNC0001L": _("Successfully completed task
'%(target_uri)s'"),
"WOKASYNC0002L": _("Failed to complete task
'%(target_uri)s'"),
@@ -62,5 +64,6 @@ messages = {
"WOKRES0001L": _("Request made on resource"),
"WOKROOT0001L": _("User '%(username)s' login"),
"WOKROOT0002L": _("User '%(username)s' logout"),
- "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
+ "WOKPLUGIN0001L": _("Enabling plug-in %(ident)s."),
+ "WOKPLUGIN0002L": _("Disabling plug-in %(ident)s."),
}
diff --git a/src/wok/utils.py b/src/wok/utils.py
index c2d4724..5e87ee0 100644
--- a/src/wok/utils.py
+++ b/src/wok/utils.py
@@ -128,7 +128,9 @@ def set_plugin_state(name, state):
wok_section_found = True
continue
- if config_contents[i].startswith('enable =') and wok_section_found:
+ if (config_contents[i].startswith('enable =') or
+ config_contents[i].startswith('enable=')) and \
+ wok_section_found:
It is safer to use regex here.
config_contents[i] = 'enable = %s\n' %
str(state)
break
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 6f1a783..06930b8 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -82,6 +82,22 @@ 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)
+
+ def _get_config_file_template(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]
@@ -113,7 +129,7 @@ very_interesting_option = True
updated_conf = f.read()
self.assertEqual(
updated_conf,
- self._get_fake_config_file_content(enable=False)
+ self._get_config_file_template(enable=False)
)
set_plugin_state('pluginA', True)
@@ -121,5 +137,5 @@ very_interesting_option = True
updated_conf = f.read()
self.assertEqual(
updated_conf,
- self._get_fake_config_file_content(enable=True)
+ self._get_config_file_template(enable=True)
)
1:03 p.m.
New subject: [PATCH] [WoK 10/10] /config/plugins API: user log and parser improvements
On 01/25/2017 03:34 PM, Aline Manera wrote:
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>
> - added user log capabilities on 'enable' and 'disable' actions
> of /config/plugins/*name* API
>
> - parser now accepts 'enable=' as a valid input to recognize the
> plug-in option that enables or disables a plug-in.
>
> Signed-off-by: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
> ---
> src/wok/control/config.py | 9 +++++++++
> src/wok/i18n.py | 5 ++++-
> src/wok/utils.py | 4 +++-
> tests/test_utils.py | 20 ++++++++++++++++++--
> 4 files changed, 34 insertions(+), 4 deletions(-)
>
> diff --git a/src/wok/control/config.py b/src/wok/control/config.py
> index c0679f3..db61060 100644
> --- a/src/wok/control/config.py
> +++ b/src/wok/control/config.py
> @@ -21,6 +21,14 @@ from wok.control.base import Collection, Resource
> from wok.control.utils import UrlSubNode
>
>
> +PLUGIN_REQUESTS = {
> + 'POST': {
> + 'enable': "WOKPLUGIN0001L",
> + 'disable': "WOKPLUGIN0002L",
> + },
> +}
> +
> +
> @UrlSubNode("config")
> class Config(Resource):
> def __init__(self, model, id=None):
> @@ -43,6 +51,7 @@ class Plugin(Resource):
> super(Plugin, self).__init__(model, ident)
> self.ident = ident
> self.uri_fmt = "/config/plugins/%s"
> + self.log_map = PLUGIN_REQUESTS
> self.enable = self.generate_action_handler('enable')
> self.disable = self.generate_action_handler('disable')
>
> diff --git a/src/wok/i18n.py b/src/wok/i18n.py
> index c96d14f..95ec20f 100644
> --- a/src/wok/i18n.py
> +++ b/src/wok/i18n.py
> @@ -55,6 +55,8 @@ messages = {
> "WOKUTILS0004E": _("Invalid data value
'%(value)s'"),
> "WOKUTILS0005E": _("Invalid data unit
'%(unit)s'"),
>
> + "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
> +
> # These messages (ending with L) are for user log purposes
> "WOKASYNC0001L": _("Successfully completed task
> '%(target_uri)s'"),
> "WOKASYNC0002L": _("Failed to complete task
'%(target_uri)s'"),
> @@ -62,5 +64,6 @@ messages = {
> "WOKRES0001L": _("Request made on resource"),
> "WOKROOT0001L": _("User '%(username)s' login"),
> "WOKROOT0002L": _("User '%(username)s' logout"),
> - "WOKPLUGIN0001E": _("Unable to find plug-in %(name)s"),
> + "WOKPLUGIN0001L": _("Enabling plug-in %(ident)s."),
> + "WOKPLUGIN0002L": _("Disabling plug-in %(ident)s."),
> }
> diff --git a/src/wok/utils.py b/src/wok/utils.py
> index c2d4724..5e87ee0 100644
> --- a/src/wok/utils.py
> +++ b/src/wok/utils.py
> @@ -128,7 +128,9 @@ def set_plugin_state(name, state):
> wok_section_found = True
> continue
>
> - if config_contents[i].startswith('enable =') and
> wok_section_found:
> + if (config_contents[i].startswith('enable =') or
> + config_contents[i].startswith('enable=')) and \
> + wok_section_found:
It is safer to use regex here.
I'll change in v3.
> config_contents[i] = 'enable = %s\n' % str(state)
> break
>
> diff --git a/tests/test_utils.py b/tests/test_utils.py
> index 6f1a783..06930b8 100644
> --- a/tests/test_utils.py
> +++ b/tests/test_utils.py
> @@ -82,6 +82,22 @@ 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)
> +
> + def _get_config_file_template(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]
> @@ -113,7 +129,7 @@ very_interesting_option = True
> updated_conf = f.read()
> self.assertEqual(
> updated_conf,
> - self._get_fake_config_file_content(enable=False)
> + self._get_config_file_template(enable=False)
> )
>
> set_plugin_state('pluginA', True)
> @@ -121,5 +137,5 @@ very_interesting_option = True
> updated_conf = f.read()
> self.assertEqual(
> updated_conf,
> - self._get_fake_config_file_content(enable=True)
> + self._get_config_file_template(enable=True)
> )
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
11:44 a.m.
Reviewed-By: Lucio Correia <luciojhc(a)linux.vnet.ibm.com>
On 20/01/2017 15:32, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
v2:
- added User Log capabilities on /config/plugins/enable|disable actions
- added 'enable=' as a valid entry in the parsing of the conf file
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 (10):
/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
/config/plugins API: user log and parser improvements
docs/API/config.md | 31 ++++++++++++++++++++
docs/API/plugins.md | 13 ---------
src/wok/control/config.py | 33 +++++++++++++++++++--
src/wok/control/plugins.py | 29 -------------------
src/wok/i18n.py | 4 +++
src/wok/model/plugins.py | 32 +++++++++++++++------
src/wok/server.py | 7 ++++-
src/wok/utils.py | 65 ++++++++++++++++++++++++++++++++++++-----
tests/test_api.py | 47 +++++++++++++++++++++++++++++-
tests/test_utils.py | 72 +++++++++++++++++++++++++++++++++++++++++++++-
ui/js/src/wok.api.js | 4 +--
ui/js/src/wok.logos.js | 11 ++++---
ui/js/src/wok.main.js | 12 +++++---
13 files changed, 288 insertions(+), 72 deletions(-)
delete mode 100644 docs/API/plugins.md
delete mode 100644 src/wok/control/plugins.py
--
Lucio Correia
11:37 a.m.
Hi Daniel,
In overall, the patch set looks good but that will not be enough to
close the feature request.
You still need to add support to plugin dependencies. Kimchi and Ginger
depends on Ginger Base and when enable/disabling one, the other should
be enable/disable as well.
We have already talked about a solution to do that and IMO it is simple
to be implemented. Any special reason to do not do that right now?
Also, you should investigate how to enable/disable a plugin on cherrypy
configuration on the fly, without the need to restart Wok server.
Cherrypy handles the API configuration in the form of a dict, maybe only
removing the plugin entry is enough. Have you checked it? Any other
solution/suggestion?
Regards,
Aline Manera
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
v2:
- added User Log capabilities on /config/plugins/enable|disable actions
- added 'enable=' as a valid entry in the parsing of the conf file
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 (10):
/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
/config/plugins API: user log and parser improvements
docs/API/config.md | 31 ++++++++++++++++++++
docs/API/plugins.md | 13 ---------
src/wok/control/config.py | 33 +++++++++++++++++++--
src/wok/control/plugins.py | 29 -------------------
src/wok/i18n.py | 4 +++
src/wok/model/plugins.py | 32 +++++++++++++++------
src/wok/server.py | 7 ++++-
src/wok/utils.py | 65 ++++++++++++++++++++++++++++++++++++-----
tests/test_api.py | 47 +++++++++++++++++++++++++++++-
tests/test_utils.py | 72 +++++++++++++++++++++++++++++++++++++++++++++-
ui/js/src/wok.api.js | 4 +--
ui/js/src/wok.logos.js | 11 ++++---
ui/js/src/wok.main.js | 12 +++++---
13 files changed, 288 insertions(+), 72 deletions(-)
delete mode 100644 docs/API/plugins.md
delete mode 100644 src/wok/control/plugins.py
1:08 p.m.
On 01/25/2017 03:37 PM, Aline Manera wrote:
Hi Daniel,
In overall, the patch set looks good but that will not be enough to
close the feature request.
You still need to add support to plugin dependencies. Kimchi and
Ginger depends on Ginger Base and when enable/disabling one, the other
should be enable/disable as well.
We have already talked about a solution to do that and IMO it is
simple to be implemented. Any special reason to do not do that right now?
I am not sure if it's simple to be implemented but I'll look into it
and, if feasible,
add it in v3.
Also, you should investigate how to enable/disable a plugin on
cherrypy configuration on the fly, without the need to restart Wok
server.
Cherrypy handles the API configuration in the form of a dict, maybe
only removing the plugin entry is enough. Have you checked it? Any
other solution/suggestion?
I haven't found a way for cherrypy to reload a configuration on the fly
without issuing server reload. As far as I've investigated it is not
possible to
retrieve the loaded modules after issuing cherrypy.tree.mount().
Any help is appreciated.
Regards,
Aline Manera
On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>
> v2:
> - added User Log capabilities on /config/plugins/enable|disable actions
> - added 'enable=' as a valid entry in the parsing of the conf file
>
> 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 (10):
> /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
> /config/plugins API: user log and parser improvements
>
> docs/API/config.md | 31 ++++++++++++++++++++
> docs/API/plugins.md | 13 ---------
> src/wok/control/config.py | 33 +++++++++++++++++++--
> src/wok/control/plugins.py | 29 -------------------
> src/wok/i18n.py | 4 +++
> src/wok/model/plugins.py | 32 +++++++++++++++------
> src/wok/server.py | 7 ++++-
> src/wok/utils.py | 65
> ++++++++++++++++++++++++++++++++++++-----
> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
> tests/test_utils.py | 72
> +++++++++++++++++++++++++++++++++++++++++++++-
> ui/js/src/wok.api.js | 4 +--
> ui/js/src/wok.logos.js | 11 ++++---
> ui/js/src/wok.main.js | 12 +++++---
> 13 files changed, 288 insertions(+), 72 deletions(-)
> delete mode 100644 docs/API/plugins.md
> delete mode 100644 src/wok/control/plugins.py
>
_______________________________________________
Kimchi-devel mailing list
Kimchi-devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
1:39 p.m.
On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
On 01/25/2017 03:37 PM, Aline Manera wrote:
> Hi Daniel,
>
> In overall, the patch set looks good but that will not be enough to
> close the feature request.
>
> You still need to add support to plugin dependencies. Kimchi and
> Ginger depends on Ginger Base and when enable/disabling one, the
> other should be enable/disable as well.
> We have already talked about a solution to do that and IMO it is
> simple to be implemented. Any special reason to do not do that right
> now?
I am not sure if it's simple to be implemented but I'll look into it
and, if feasible,
add it in v3.
>
> Also, you should investigate how to enable/disable a plugin on
> cherrypy configuration on the fly, without the need to restart Wok
> server.
> Cherrypy handles the API configuration in the form of a dict, maybe
> only removing the plugin entry is enough. Have you checked it? Any
> other solution/suggestion?
I haven't found a way for cherrypy to reload a configuration on the fly
without issuing server reload. As far as I've investigated it is not
possible to
retrieve the loaded modules after issuing cherrypy.tree.mount().
The right place to look at is: cherrypy.tree.apps. It will be a dict of
plugin URI and plugin Root instance.
Something like:
{/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger: <GingerRoot
instance>, ...}
I have not tested it before, but shouldn't we edit this to remove the
entry we want and add when needed?
Any help is appreciated.
>
> Regards,
> Aline Manera
>
> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>>
>> v2:
>> - added User Log capabilities on /config/plugins/enable|disable actions
>> - added 'enable=' as a valid entry in the parsing of the conf file
>>
>> 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 (10):
>> /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
>> /config/plugins API: user log and parser improvements
>>
>> docs/API/config.md | 31 ++++++++++++++++++++
>> docs/API/plugins.md | 13 ---------
>> src/wok/control/config.py | 33 +++++++++++++++++++--
>> src/wok/control/plugins.py | 29 -------------------
>> src/wok/i18n.py | 4 +++
>> src/wok/model/plugins.py | 32 +++++++++++++++------
>> src/wok/server.py | 7 ++++-
>> src/wok/utils.py | 65
>> ++++++++++++++++++++++++++++++++++++-----
>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>> tests/test_utils.py | 72
>> +++++++++++++++++++++++++++++++++++++++++++++-
>> ui/js/src/wok.api.js | 4 +--
>> ui/js/src/wok.logos.js | 11 ++++---
>> ui/js/src/wok.main.js | 12 +++++---
>> 13 files changed, 288 insertions(+), 72 deletions(-)
>> delete mode 100644 docs/API/plugins.md
>> delete mode 100644 src/wok/control/plugins.py
>>
>
> _______________________________________________
> 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
3:17 p.m.
On 01/25/2017 05:39 PM, Aline Manera wrote:
On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>
>
> On 01/25/2017 03:37 PM, Aline Manera wrote:
>> Hi Daniel,
>>
>> In overall, the patch set looks good but that will not be enough to
>> close the feature request.
>>
>> You still need to add support to plugin dependencies. Kimchi and
>> Ginger depends on Ginger Base and when enable/disabling one, the
>> other should be enable/disable as well.
>> We have already talked about a solution to do that and IMO it is
>> simple to be implemented. Any special reason to do not do that right
>> now?
>
> I am not sure if it's simple to be implemented but I'll look into it
> and, if feasible,
> add it in v3.
>
>>
>> Also, you should investigate how to enable/disable a plugin on
>> cherrypy configuration on the fly, without the need to restart Wok
>> server.
>> Cherrypy handles the API configuration in the form of a dict, maybe
>> only removing the plugin entry is enough. Have you checked it? Any
>> other solution/suggestion?
>
> I haven't found a way for cherrypy to reload a configuration on the fly
> without issuing server reload. As far as I've investigated it is not
> possible to
> retrieve the loaded modules after issuing cherrypy.tree.mount().
>
The right place to look at is: cherrypy.tree.apps. It will be a dict
of plugin URI and plugin Root instance.
Something like:
{/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger: <GingerRoot
instance>, ...}
I have not tested it before, but shouldn't we edit this to remove the
entry we want and add when needed?
Not sure, I can give a try. But all this discussion raises a question.
Back in the RFC, we agreed on a
solution that consists on editing the plug-ins config file and then
reloads WoK. We're now discussing a
way to make cherrypy enable/disable them on the fly. Is this now a part
of this same work?
My initial understanding was that the /config/reload API +
/config/plugins API closes the feature
request.
There's also UI challenges with this 'on the fly' reload. The whole idea
of doing 'on the fly' is
to avoid a full UI reload + relogin when enabling/disabling plug-in. Say
userA is creating a new
VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the fact
that it can take 3 seconds
for the UI to know about it (the Notification interval). In this case,
Kimchi was also disabled, so the
current operation userA was about to do is now invalid. A full UI reload
is now required to
remove both plug-ins from the UI. This is the trivial scenario.
The "bad" scenario is, instead of disabling Gingerbase, adminB disables
Ginger. Should we disrupt
userA work with a full refresh? Kimchi wasn't affected by it. Should we
wait until he does
a refresh window to remove Ginger?
The worst scenario: userA is using Gingers390x, Gingerbase is disabled.
Unless Gingers390x
UI is aware of its full dependency tree, it won't be aware that
disabling Gingerbase cascades
into disabling Ginger, so a full refresh is required because Gingers390x
was disabled too.
Note that all these scenarios can be solved by forcing a UI refresh
every time but, in this case,
the only difference between the 'on the fly' reload versus a reboot
reload is that on the second
you'll need to login again.
To sum up, I am not against investigating the cherrypy on the fly reload
at this moment, but
I advise against pushing it together with this work, without UI changes.
The 'reboot reload'
we proposed in the RFC does not share these problems because when the
browser loses
connectivity it goes back to the login screen.
Daniel
> Any help is appreciated.
>
>
>>
>> Regards,
>> Aline Manera
>>
>> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>>>
>>> v2:
>>> - added User Log capabilities on /config/plugins/enable|disable
>>> actions
>>> - added 'enable=' as a valid entry in the parsing of the conf file
>>>
>>> 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 (10):
>>> /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
>>> /config/plugins API: user log and parser improvements
>>>
>>> docs/API/config.md | 31 ++++++++++++++++++++
>>> docs/API/plugins.md | 13 ---------
>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>> src/wok/control/plugins.py | 29 -------------------
>>> src/wok/i18n.py | 4 +++
>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>> src/wok/server.py | 7 ++++-
>>> src/wok/utils.py | 65
>>> ++++++++++++++++++++++++++++++++++++-----
>>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>>> tests/test_utils.py | 72
>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>> ui/js/src/wok.api.js | 4 +--
>>> ui/js/src/wok.logos.js | 11 ++++---
>>> ui/js/src/wok.main.js | 12 +++++---
>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>> delete mode 100644 docs/API/plugins.md
>>> delete mode 100644 src/wok/control/plugins.py
>>>
>>
>> _______________________________________________
>> 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
>
5:41 a.m.
On 01/25/2017 07:17 PM, Daniel Henrique Barboza wrote:
On 01/25/2017 05:39 PM, Aline Manera wrote:
>
>
> On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>>
>>
>> On 01/25/2017 03:37 PM, Aline Manera wrote:
>>> Hi Daniel,
>>>
>>> In overall, the patch set looks good but that will not be enough to
>>> close the feature request.
>>>
>>> You still need to add support to plugin dependencies. Kimchi and
>>> Ginger depends on Ginger Base and when enable/disabling one, the
>>> other should be enable/disable as well.
>>> We have already talked about a solution to do that and IMO it is
>>> simple to be implemented. Any special reason to do not do that
>>> right now?
>>
>> I am not sure if it's simple to be implemented but I'll look into it
>> and, if feasible,
>> add it in v3.
>>
>>>
>>> Also, you should investigate how to enable/disable a plugin on
>>> cherrypy configuration on the fly, without the need to restart Wok
>>> server.
>>> Cherrypy handles the API configuration in the form of a dict, maybe
>>> only removing the plugin entry is enough. Have you checked it? Any
>>> other solution/suggestion?
>>
>> I haven't found a way for cherrypy to reload a configuration on the
>> fly
>> without issuing server reload. As far as I've investigated it is not
>> possible to
>> retrieve the loaded modules after issuing cherrypy.tree.mount().
>>
>
> The right place to look at is: cherrypy.tree.apps. It will be a dict
> of plugin URI and plugin Root instance.
>
> Something like:
>
> {/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger: <GingerRoot
> instance>, ...}
>
> I have not tested it before, but shouldn't we edit this to remove the
> entry we want and add when needed?
Not sure, I can give a try. But all this discussion raises a question.
Back in the RFC, we agreed on a
solution that consists on editing the plug-ins config file and then
reloads WoK. We're now discussing a
way to make cherrypy enable/disable them on the fly. Is this now a
part of this same work?
My initial understanding was that the /config/reload API +
/config/plugins API closes the feature
request.
There's also UI challenges with this 'on the fly' reload. The whole
idea of doing 'on the fly' is
to avoid a full UI reload + relogin when enabling/disabling plug-in.
Say userA is creating a new
VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the
fact that it can take 3 seconds
for the UI to know about it (the Notification interval). In this case,
Kimchi was also disabled, so the
current operation userA was about to do is now invalid. A full UI
reload is now required to
remove both plug-ins from the UI. This is the trivial scenario.
The "bad" scenario is, instead of disabling Gingerbase, adminB
disables Ginger. Should we disrupt
userA work with a full refresh? Kimchi wasn't affected by it. Should
we wait until he does
a refresh window to remove Ginger?
The worst scenario: userA is using Gingers390x, Gingerbase is
disabled. Unless Gingers390x
UI is aware of its full dependency tree, it won't be aware that
disabling Gingerbase cascades
into disabling Ginger, so a full refresh is required because
Gingers390x was disabled too.
Note that all these scenarios can be solved by forcing a UI refresh
every time but, in this case,
the only difference between the 'on the fly' reload versus a reboot
reload is that on the second
you'll need to login again.
To sum up, I am not against investigating the cherrypy on the fly
reload at this moment, but
I advise against pushing it together with this work, without UI
changes. The 'reboot reload'
we proposed in the RFC does not share these problems because when the
browser loses
connectivity it goes back to the login screen.
Yeap! Maybe I was not clear in my first comment. What you need to do to
get this patch accepted upstream is only the plugin dependencies stuff.
IMO we should have an **investigation** on what is needed to get the
plugin enabling/disabling on the fly to know the next steps.
For example, the UI development needs to wait that decision to avoid
waste of time and code.
All the scenarios you pointed above are still valid for plugin
enabling/disabling through configuration file.
And the need to restart the Wok server is more intrusive as it may cause
issues in the ongoing tasks the plugin is performing. For example, some
leftovers may be on system due that and cause issues on future actions.
IMO it is better to an user get a message "API is no longer available"
then 'break' the system in some way due a web server restart when some
tasks was ongoing.
Think about clone/create/migrate virtual machines, create debug reports,
create/restore backups - which a mess it may cause in the system on a
server restart.
The plugin management on the fly will not interrupt any on going tasks -
they only will not be shown on UI anymore but the background task will
continue to work.
And only future requests will be blocked - avoiding leftover on system.
I personally think the on the fly configuration makes better the user
experience and avoid issues.
Also to have the plugin enabling/disabling on the fly working properly
we will need to have the websockets communication to inform all open
browser sessions to reload the UI.
So, as I said, we should only have in mind what will be the next steps
on that area.
Daniel
>
>> Any help is appreciated.
>>
>>
>>>
>>> Regards,
>>> Aline Manera
>>>
>>> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>>>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>>>>
>>>> v2:
>>>> - added User Log capabilities on /config/plugins/enable|disable
>>>> actions
>>>> - added 'enable=' as a valid entry in the parsing of the conf
file
>>>>
>>>> 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 (10):
>>>> /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
>>>> /config/plugins API: user log and parser improvements
>>>>
>>>> docs/API/config.md | 31 ++++++++++++++++++++
>>>> docs/API/plugins.md | 13 ---------
>>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>>> src/wok/control/plugins.py | 29 -------------------
>>>> src/wok/i18n.py | 4 +++
>>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>>> src/wok/server.py | 7 ++++-
>>>> src/wok/utils.py | 65
>>>> ++++++++++++++++++++++++++++++++++++-----
>>>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>>>> tests/test_utils.py | 72
>>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>>> ui/js/src/wok.api.js | 4 +--
>>>> ui/js/src/wok.logos.js | 11 ++++---
>>>> ui/js/src/wok.main.js | 12 +++++---
>>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>>> delete mode 100644 docs/API/plugins.md
>>>> delete mode 100644 src/wok/control/plugins.py
>>>>
>>>
>>> _______________________________________________
>>> 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
>>
>
6:04 a.m.
Hi Daniel,
I have just tested and
del cherrypy.tree.apps[<plugin-uri>]
Successfully disable a plugin.
And
cherrypy.tree.mount(<plugin-app>, <plugin-uri>, <plugin-config>)
(Check server.py for more details about the parameters to
cherrypy.tree.mount)
Enables a plugin.
So, it would be great if you could add that to your V3 patch as well.
Then, when we have the async notification events using websockets
working we can remove the need to reboot the web server.
What do you think?
On 01/26/2017 09:41 AM, Aline Manera wrote:
On 01/25/2017 07:17 PM, Daniel Henrique Barboza wrote:
>
>
> On 01/25/2017 05:39 PM, Aline Manera wrote:
>>
>>
>> On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 01/25/2017 03:37 PM, Aline Manera wrote:
>>>> Hi Daniel,
>>>>
>>>> In overall, the patch set looks good but that will not be enough
>>>> to close the feature request.
>>>>
>>>> You still need to add support to plugin dependencies. Kimchi and
>>>> Ginger depends on Ginger Base and when enable/disabling one, the
>>>> other should be enable/disable as well.
>>>> We have already talked about a solution to do that and IMO it is
>>>> simple to be implemented. Any special reason to do not do that
>>>> right now?
>>>
>>> I am not sure if it's simple to be implemented but I'll look into
>>> it and, if feasible,
>>> add it in v3.
>>>
>>>>
>>>> Also, you should investigate how to enable/disable a plugin on
>>>> cherrypy configuration on the fly, without the need to restart Wok
>>>> server.
>>>> Cherrypy handles the API configuration in the form of a dict,
>>>> maybe only removing the plugin entry is enough. Have you checked
>>>> it? Any other solution/suggestion?
>>>
>>> I haven't found a way for cherrypy to reload a configuration on
>>> the fly
>>> without issuing server reload. As far as I've investigated it is
>>> not possible to
>>> retrieve the loaded modules after issuing cherrypy.tree.mount().
>>>
>>
>> The right place to look at is: cherrypy.tree.apps. It will be a dict
>> of plugin URI and plugin Root instance.
>>
>> Something like:
>>
>> {/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger:
>> <GingerRoot instance>, ...}
>>
>> I have not tested it before, but shouldn't we edit this to remove
>> the entry we want and add when needed?
>
> Not sure, I can give a try. But all this discussion raises a
> question. Back in the RFC, we agreed on a
> solution that consists on editing the plug-ins config file and then
> reloads WoK. We're now discussing a
> way to make cherrypy enable/disable them on the fly. Is this now a
> part of this same work?
> My initial understanding was that the /config/reload API +
> /config/plugins API closes the feature
> request.
>
> There's also UI challenges with this 'on the fly' reload. The whole
> idea of doing 'on the fly' is
> to avoid a full UI reload + relogin when enabling/disabling plug-in.
> Say userA is creating a new
> VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the
> fact that it can take 3 seconds
> for the UI to know about it (the Notification interval). In this
> case, Kimchi was also disabled, so the
> current operation userA was about to do is now invalid. A full UI
> reload is now required to
> remove both plug-ins from the UI. This is the trivial scenario.
>
> The "bad" scenario is, instead of disabling Gingerbase, adminB
> disables Ginger. Should we disrupt
> userA work with a full refresh? Kimchi wasn't affected by it. Should
> we wait until he does
> a refresh window to remove Ginger?
>
> The worst scenario: userA is using Gingers390x, Gingerbase is
> disabled. Unless Gingers390x
> UI is aware of its full dependency tree, it won't be aware that
> disabling Gingerbase cascades
> into disabling Ginger, so a full refresh is required because
> Gingers390x was disabled too.
>
> Note that all these scenarios can be solved by forcing a UI refresh
> every time but, in this case,
> the only difference between the 'on the fly' reload versus a reboot
> reload is that on the second
> you'll need to login again.
>
>
> To sum up, I am not against investigating the cherrypy on the fly
> reload at this moment, but
> I advise against pushing it together with this work, without UI
> changes. The 'reboot reload'
> we proposed in the RFC does not share these problems because when the
> browser loses
> connectivity it goes back to the login screen.
>
Yeap! Maybe I was not clear in my first comment. What you need to do
to get this patch accepted upstream is only the plugin dependencies
stuff.
IMO we should have an **investigation** on what is needed to get the
plugin enabling/disabling on the fly to know the next steps.
For example, the UI development needs to wait that decision to avoid
waste of time and code.
All the scenarios you pointed above are still valid for plugin
enabling/disabling through configuration file.
And the need to restart the Wok server is more intrusive as it may
cause issues in the ongoing tasks the plugin is performing. For
example, some leftovers may be on system due that and cause issues on
future actions.
IMO it is better to an user get a message "API is no longer available"
then 'break' the system in some way due a web server restart when some
tasks was ongoing.
Think about clone/create/migrate virtual machines, create debug
reports, create/restore backups - which a mess it may cause in the
system on a server restart.
The plugin management on the fly will not interrupt any on going tasks
- they only will not be shown on UI anymore but the background task
will continue to work.
And only future requests will be blocked - avoiding leftover on system.
I personally think the on the fly configuration makes better the user
experience and avoid issues.
Also to have the plugin enabling/disabling on the fly working properly
we will need to have the websockets communication to inform all open
browser sessions to reload the UI.
So, as I said, we should only have in mind what will be the next steps
on that area.
>
> Daniel
>
>
>>
>>> Any help is appreciated.
>>>
>>>
>>>>
>>>> Regards,
>>>> Aline Manera
>>>>
>>>> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>>>>> From: Daniel Henrique Barboza <danielhb(a)linux.vnet.ibm.com>
>>>>>
>>>>> v2:
>>>>> - added User Log capabilities on /config/plugins/enable|disable
>>>>> actions
>>>>> - added 'enable=' as a valid entry in the parsing of the conf
file
>>>>>
>>>>> 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 (10):
>>>>> /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
>>>>> /config/plugins API: user log and parser improvements
>>>>>
>>>>> docs/API/config.md | 31 ++++++++++++++++++++
>>>>> docs/API/plugins.md | 13 ---------
>>>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>>>> src/wok/control/plugins.py | 29 -------------------
>>>>> src/wok/i18n.py | 4 +++
>>>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>>>> src/wok/server.py | 7 ++++-
>>>>> src/wok/utils.py | 65
>>>>> ++++++++++++++++++++++++++++++++++++-----
>>>>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>>>>> tests/test_utils.py | 72
>>>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>>>> ui/js/src/wok.api.js | 4 +--
>>>>> ui/js/src/wok.logos.js | 11 ++++---
>>>>> ui/js/src/wok.main.js | 12 +++++---
>>>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>>>> delete mode 100644 docs/API/plugins.md
>>>>> delete mode 100644 src/wok/control/plugins.py
>>>>>
>>>>
>>>> _______________________________________________
>>>> 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
6:10 a.m.
On 01/26/2017 10:04 AM, Aline Manera wrote:
Hi Daniel,
I have just tested and
del cherrypy.tree.apps[<plugin-uri>]
Successfully disable a plugin.
And
cherrypy.tree.mount(<plugin-app>, <plugin-uri>, <plugin-config>)
(Check server.py for more details about the parameters to
cherrypy.tree.mount)
Enables a plugin.
Interesting.
So, it would be great if you could add that to your V3 patch as well.
Then, when we have the async notification events using websockets
working we can remove the need to reboot the web server.
What do you think?
You're proposing that /config/plugins/*name*/enable|disable would not just
edit the config file but also do the on the fly enable/disablement of
the plug-ins,
even if for now the UI will need to reboot the web server after doing so
because of the pending UI issues?
I think it's ok. I'll get it done in v3.
On 01/26/2017 09:41 AM, Aline Manera wrote:
>
>
> On 01/25/2017 07:17 PM, Daniel Henrique Barboza wrote:
>>
>>
>> On 01/25/2017 05:39 PM, Aline Manera wrote:
>>>
>>>
>>> On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>>>>
>>>>
>>>> On 01/25/2017 03:37 PM, Aline Manera wrote:
>>>>> Hi Daniel,
>>>>>
>>>>> In overall, the patch set looks good but that will not be enough
>>>>> to close the feature request.
>>>>>
>>>>> You still need to add support to plugin dependencies. Kimchi and
>>>>> Ginger depends on Ginger Base and when enable/disabling one, the
>>>>> other should be enable/disable as well.
>>>>> We have already talked about a solution to do that and IMO it is
>>>>> simple to be implemented. Any special reason to do not do that
>>>>> right now?
>>>>
>>>> I am not sure if it's simple to be implemented but I'll look into
>>>> it and, if feasible,
>>>> add it in v3.
>>>>
>>>>>
>>>>> Also, you should investigate how to enable/disable a plugin on
>>>>> cherrypy configuration on the fly, without the need to restart
>>>>> Wok server.
>>>>> Cherrypy handles the API configuration in the form of a dict,
>>>>> maybe only removing the plugin entry is enough. Have you checked
>>>>> it? Any other solution/suggestion?
>>>>
>>>> I haven't found a way for cherrypy to reload a configuration on
>>>> the fly
>>>> without issuing server reload. As far as I've investigated it is
>>>> not possible to
>>>> retrieve the loaded modules after issuing cherrypy.tree.mount().
>>>>
>>>
>>> The right place to look at is: cherrypy.tree.apps. It will be a
>>> dict of plugin URI and plugin Root instance.
>>>
>>> Something like:
>>>
>>> {/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger:
>>> <GingerRoot instance>, ...}
>>>
>>> I have not tested it before, but shouldn't we edit this to remove
>>> the entry we want and add when needed?
>>
>> Not sure, I can give a try. But all this discussion raises a
>> question. Back in the RFC, we agreed on a
>> solution that consists on editing the plug-ins config file and then
>> reloads WoK. We're now discussing a
>> way to make cherrypy enable/disable them on the fly. Is this now a
>> part of this same work?
>> My initial understanding was that the /config/reload API +
>> /config/plugins API closes the feature
>> request.
>>
>> There's also UI challenges with this 'on the fly' reload. The whole
>> idea of doing 'on the fly' is
>> to avoid a full UI reload + relogin when enabling/disabling plug-in.
>> Say userA is creating a new
>> VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the
>> fact that it can take 3 seconds
>> for the UI to know about it (the Notification interval). In this
>> case, Kimchi was also disabled, so the
>> current operation userA was about to do is now invalid. A full UI
>> reload is now required to
>> remove both plug-ins from the UI. This is the trivial scenario.
>>
>> The "bad" scenario is, instead of disabling Gingerbase, adminB
>> disables Ginger. Should we disrupt
>> userA work with a full refresh? Kimchi wasn't affected by it. Should
>> we wait until he does
>> a refresh window to remove Ginger?
>>
>> The worst scenario: userA is using Gingers390x, Gingerbase is
>> disabled. Unless Gingers390x
>> UI is aware of its full dependency tree, it won't be aware that
>> disabling Gingerbase cascades
>> into disabling Ginger, so a full refresh is required because
>> Gingers390x was disabled too.
>>
>> Note that all these scenarios can be solved by forcing a UI refresh
>> every time but, in this case,
>> the only difference between the 'on the fly' reload versus a reboot
>> reload is that on the second
>> you'll need to login again.
>>
>>
>> To sum up, I am not against investigating the cherrypy on the fly
>> reload at this moment, but
>> I advise against pushing it together with this work, without UI
>> changes. The 'reboot reload'
>> we proposed in the RFC does not share these problems because when
>> the browser loses
>> connectivity it goes back to the login screen.
>>
>
> Yeap! Maybe I was not clear in my first comment. What you need to do
> to get this patch accepted upstream is only the plugin dependencies
> stuff.
>
> IMO we should have an **investigation** on what is needed to get the
> plugin enabling/disabling on the fly to know the next steps.
> For example, the UI development needs to wait that decision to avoid
> waste of time and code.
>
> All the scenarios you pointed above are still valid for plugin
> enabling/disabling through configuration file.
> And the need to restart the Wok server is more intrusive as it may
> cause issues in the ongoing tasks the plugin is performing. For
> example, some leftovers may be on system due that and cause issues on
> future actions.
> IMO it is better to an user get a message "API is no longer
> available" then 'break' the system in some way due a web server
> restart when some tasks was ongoing.
> Think about clone/create/migrate virtual machines, create debug
> reports, create/restore backups - which a mess it may cause in the
> system on a server restart.
> The plugin management on the fly will not interrupt any on going
> tasks - they only will not be shown on UI anymore but the background
> task will continue to work.
> And only future requests will be blocked - avoiding leftover on system.
>
> I personally think the on the fly configuration makes better the user
> experience and avoid issues.
>
> Also to have the plugin enabling/disabling on the fly working
> properly we will need to have the websockets communication to inform
> all open browser sessions to reload the UI.
> So, as I said, we should only have in mind what will be the next
> steps on that area.
>
>>
>> Daniel
>>
>>
>>>
>>>> Any help is appreciated.
>>>>
>>>>
>>>>>
>>>>> Regards,
>>>>> Aline Manera
>>>>>
>>>>> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>>>>>> From: Daniel Henrique Barboza
<danielhb(a)linux.vnet.ibm.com>
>>>>>>
>>>>>> v2:
>>>>>> - added User Log capabilities on /config/plugins/enable|disable
>>>>>> actions
>>>>>> - added 'enable=' as a valid entry in the parsing of the
conf file
>>>>>>
>>>>>> 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 (10):
>>>>>> /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
>>>>>> /config/plugins API: user log and parser improvements
>>>>>>
>>>>>> docs/API/config.md | 31 ++++++++++++++++++++
>>>>>> docs/API/plugins.md | 13 ---------
>>>>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>>>>> src/wok/control/plugins.py | 29 -------------------
>>>>>> src/wok/i18n.py | 4 +++
>>>>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>>>>> src/wok/server.py | 7 ++++-
>>>>>> src/wok/utils.py | 65
>>>>>> ++++++++++++++++++++++++++++++++++++-----
>>>>>> tests/test_api.py | 47 +++++++++++++++++++++++++++++-
>>>>>> tests/test_utils.py | 72
>>>>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>>>>> ui/js/src/wok.api.js | 4 +--
>>>>>> ui/js/src/wok.logos.js | 11 ++++---
>>>>>> ui/js/src/wok.main.js | 12 +++++---
>>>>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>>>>> delete mode 100644 docs/API/plugins.md
>>>>>> delete mode 100644 src/wok/control/plugins.py
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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
>
6:14 a.m.
On 01/26/2017 10:10 AM, Daniel Henrique Barboza wrote:
On 01/26/2017 10:04 AM, Aline Manera wrote:
>
> Hi Daniel,
>
> I have just tested and
>
> del cherrypy.tree.apps[<plugin-uri>]
>
> Successfully disable a plugin.
>
> And
>
> cherrypy.tree.mount(<plugin-app>, <plugin-uri>, <plugin-config>)
>
> (Check server.py for more details about the parameters to
> cherrypy.tree.mount)
>
> Enables a plugin.
Interesting.
>
> So, it would be great if you could add that to your V3 patch as well.
> Then, when we have the async notification events using websockets
> working we can remove the need to reboot the web server.
>
> What do you think?
You're proposing that /config/plugins/*name*/enable|disable would not
just
edit the config file but also do the on the fly enable/disablement of
the plug-ins,
even if for now the UI will need to reboot the web server after doing so
because of the pending UI issues?
Yeap!
Then when we have support on async notification using websockets, we
just need to update the /config/reload API to send a request to all the
open browser sessions to reload its page (to reflect the plugin changes)
instead of restarting cherrypy
I think it's ok. I'll get it done in v3.
Thanks!
>
> On 01/26/2017 09:41 AM, Aline Manera wrote:
>>
>>
>> On 01/25/2017 07:17 PM, Daniel Henrique Barboza wrote:
>>>
>>>
>>> On 01/25/2017 05:39 PM, Aline Manera wrote:
>>>>
>>>>
>>>> On 01/25/2017 05:08 PM, Daniel Henrique Barboza wrote:
>>>>>
>>>>>
>>>>> On 01/25/2017 03:37 PM, Aline Manera wrote:
>>>>>> Hi Daniel,
>>>>>>
>>>>>> In overall, the patch set looks good but that will not be enough
>>>>>> to close the feature request.
>>>>>>
>>>>>> You still need to add support to plugin dependencies. Kimchi and
>>>>>> Ginger depends on Ginger Base and when enable/disabling one, the
>>>>>> other should be enable/disable as well.
>>>>>> We have already talked about a solution to do that and IMO it is
>>>>>> simple to be implemented. Any special reason to do not do that
>>>>>> right now?
>>>>>
>>>>> I am not sure if it's simple to be implemented but I'll look
into
>>>>> it and, if feasible,
>>>>> add it in v3.
>>>>>
>>>>>>
>>>>>> Also, you should investigate how to enable/disable a plugin on
>>>>>> cherrypy configuration on the fly, without the need to restart
>>>>>> Wok server.
>>>>>> Cherrypy handles the API configuration in the form of a dict,
>>>>>> maybe only removing the plugin entry is enough. Have you checked
>>>>>> it? Any other solution/suggestion?
>>>>>
>>>>> I haven't found a way for cherrypy to reload a configuration on
>>>>> the fly
>>>>> without issuing server reload. As far as I've investigated it is
>>>>> not possible to
>>>>> retrieve the loaded modules after issuing cherrypy.tree.mount().
>>>>>
>>>>
>>>> The right place to look at is: cherrypy.tree.apps. It will be a
>>>> dict of plugin URI and plugin Root instance.
>>>>
>>>> Something like:
>>>>
>>>> {/plugins/kimchi: <KimchiRoot instance>, /plugins/ginger:
>>>> <GingerRoot instance>, ...}
>>>>
>>>> I have not tested it before, but shouldn't we edit this to remove
>>>> the entry we want and add when needed?
>>>
>>> Not sure, I can give a try. But all this discussion raises a
>>> question. Back in the RFC, we agreed on a
>>> solution that consists on editing the plug-ins config file and then
>>> reloads WoK. We're now discussing a
>>> way to make cherrypy enable/disable them on the fly. Is this now a
>>> part of this same work?
>>> My initial understanding was that the /config/reload API +
>>> /config/plugins API closes the feature
>>> request.
>>>
>>> There's also UI challenges with this 'on the fly' reload. The
whole
>>> idea of doing 'on the fly' is
>>> to avoid a full UI reload + relogin when enabling/disabling
>>> plug-in. Say userA is creating a new
>>> VM snapshot in Kimchi. adminB disables gingerbase. Let's ignore the
>>> fact that it can take 3 seconds
>>> for the UI to know about it (the Notification interval). In this
>>> case, Kimchi was also disabled, so the
>>> current operation userA was about to do is now invalid. A full UI
>>> reload is now required to
>>> remove both plug-ins from the UI. This is the trivial scenario.
>>>
>>> The "bad" scenario is, instead of disabling Gingerbase, adminB
>>> disables Ginger. Should we disrupt
>>> userA work with a full refresh? Kimchi wasn't affected by it.
>>> Should we wait until he does
>>> a refresh window to remove Ginger?
>>>
>>> The worst scenario: userA is using Gingers390x, Gingerbase is
>>> disabled. Unless Gingers390x
>>> UI is aware of its full dependency tree, it won't be aware that
>>> disabling Gingerbase cascades
>>> into disabling Ginger, so a full refresh is required because
>>> Gingers390x was disabled too.
>>>
>>> Note that all these scenarios can be solved by forcing a UI refresh
>>> every time but, in this case,
>>> the only difference between the 'on the fly' reload versus a reboot
>>> reload is that on the second
>>> you'll need to login again.
>>>
>>>
>>> To sum up, I am not against investigating the cherrypy on the fly
>>> reload at this moment, but
>>> I advise against pushing it together with this work, without UI
>>> changes. The 'reboot reload'
>>> we proposed in the RFC does not share these problems because when
>>> the browser loses
>>> connectivity it goes back to the login screen.
>>>
>>
>> Yeap! Maybe I was not clear in my first comment. What you need to do
>> to get this patch accepted upstream is only the plugin dependencies
>> stuff.
>>
>> IMO we should have an **investigation** on what is needed to get the
>> plugin enabling/disabling on the fly to know the next steps.
>> For example, the UI development needs to wait that decision to avoid
>> waste of time and code.
>>
>> All the scenarios you pointed above are still valid for plugin
>> enabling/disabling through configuration file.
>> And the need to restart the Wok server is more intrusive as it may
>> cause issues in the ongoing tasks the plugin is performing. For
>> example, some leftovers may be on system due that and cause issues
>> on future actions.
>> IMO it is better to an user get a message "API is no longer
>> available" then 'break' the system in some way due a web server
>> restart when some tasks was ongoing.
>> Think about clone/create/migrate virtual machines, create debug
>> reports, create/restore backups - which a mess it may cause in the
>> system on a server restart.
>> The plugin management on the fly will not interrupt any on going
>> tasks - they only will not be shown on UI anymore but the background
>> task will continue to work.
>> And only future requests will be blocked - avoiding leftover on system.
>>
>> I personally think the on the fly configuration makes better the
>> user experience and avoid issues.
>>
>> Also to have the plugin enabling/disabling on the fly working
>> properly we will need to have the websockets communication to inform
>> all open browser sessions to reload the UI.
>> So, as I said, we should only have in mind what will be the next
>> steps on that area.
>>
>>>
>>> Daniel
>>>
>>>
>>>>
>>>>> Any help is appreciated.
>>>>>
>>>>>
>>>>>>
>>>>>> Regards,
>>>>>> Aline Manera
>>>>>>
>>>>>> On 01/20/2017 03:32 PM, dhbarboza82(a)gmail.com wrote:
>>>>>>> From: Daniel Henrique Barboza
<danielhb(a)linux.vnet.ibm.com>
>>>>>>>
>>>>>>> v2:
>>>>>>> - added User Log capabilities on
/config/plugins/enable|disable
>>>>>>> actions
>>>>>>> - added 'enable=' as a valid entry in the parsing of
the conf file
>>>>>>>
>>>>>>> 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 (10):
>>>>>>> /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
>>>>>>> /config/plugins API: user log and parser improvements
>>>>>>>
>>>>>>> docs/API/config.md | 31 ++++++++++++++++++++
>>>>>>> docs/API/plugins.md | 13 ---------
>>>>>>> src/wok/control/config.py | 33 +++++++++++++++++++--
>>>>>>> src/wok/control/plugins.py | 29 -------------------
>>>>>>> src/wok/i18n.py | 4 +++
>>>>>>> src/wok/model/plugins.py | 32 +++++++++++++++------
>>>>>>> src/wok/server.py | 7 ++++-
>>>>>>> src/wok/utils.py | 65
>>>>>>> ++++++++++++++++++++++++++++++++++++-----
>>>>>>> tests/test_api.py | 47
+++++++++++++++++++++++++++++-
>>>>>>> tests/test_utils.py | 72
>>>>>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>>>>>> ui/js/src/wok.api.js | 4 +--
>>>>>>> ui/js/src/wok.logos.js | 11 ++++---
>>>>>>> ui/js/src/wok.main.js | 12 +++++---
>>>>>>> 13 files changed, 288 insertions(+), 72 deletions(-)
>>>>>>> delete mode 100644 docs/API/plugins.md
>>>>>>> delete mode 100644 src/wok/control/plugins.py
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> 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
>>
>
2633
days inactive
2647
days old
29 comments
4 participants
participants (4)
-
Aline Manera -
Daniel Henrique Barboza -
dhbarboza82＠gmail.com -
Lucio Correia