[PATCH 1/3] Plugins: Fix plugins framework in kimchi

If the plugin tab name string is not found in kimchi i18n file, the tab name is set as 'unknown'. This patch fixes this problem and also adds routines to add tab html file in cherrypy configuration automati cally. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/server.py | 38 ++++++++++++++++++++++++++++++++++++-- ui/js/src/kimchi.main.js | 2 +- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/kimchi/server.py b/src/kimchi/server.py index b820263..919e2e0 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -28,6 +28,10 @@ import logging.handlers import os import sslcert + +from xml.etree import ElementTree + + from kimchi import auth from kimchi import config from kimchi import model @@ -208,13 +212,43 @@ class Server(object): script_name = plugin_config['kimchi']['uri'] del plugin_config['kimchi'] + tabs_extra_file = config.get_plugin_tab_xml(plugin_name) plugin_config['/ui/config/tab-ext.xml'] = { 'tools.staticfile.on': True, - 'tools.staticfile.filename': - config.get_plugin_tab_xml(plugin_name), + 'tools.staticfile.filename': tabs_extra_file, 'tools.nocache.on': True} + + # Providing plugin tabs htmls + et = ElementTree.ElementTree(file=tabs_extra_file) + + tabs = et.findall('tab') + for tab in tabs: + html = tab.find('path').text + tabs_extra_file = os.path.join(config.get_prefix(), html) + if html.startswith('/'): + html = html.replace(script_name,'') + else: + html = html.replace(script_name[1:],'') + plugin_config[html] = { + 'tools.staticfile.on': True, + 'tools.staticfile.filename': tabs_extra_file, + 'tools.nocache.on': True} + except KeyError: continue + except IOError as e: + msg = "Failed to load plugin tabs file %s" %tabs_extra_file + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue + except ElementTree.ParseError as e: + msg = "XML parse error in file %s: %s" %( + tabs_extra_file, e.msg) + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue try: plugin_app = import_class(plugin_class)() diff --git a/ui/js/src/kimchi.main.js b/ui/js/src/kimchi.main.js index cc8afee..8b84a34 100644 --- a/ui/js/src/kimchi.main.js +++ b/ui/js/src/kimchi.main.js @@ -211,7 +211,7 @@ kimchi.getTabHtml = function(url) { $(xmlData).find('tab').each(function() { var $tab = $(this); var titleKey = $tab.find('title').text(); - var title = i18n[titleKey]; + var title = i18n[titleKey] || titleKey; var path = $tab.find('path').text(); tabsHtml += "<li><a class='item' href=" + path + ">" + title + "</a></li>"; }); -- 1.8.1.4

Currently, the api_schema for the plugins have not been checked. This patch fixes this problem getting the api_schema from the main plugin class. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..676fa08 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -89,7 +89,9 @@ def internal_redirect(url): def validate_params(params, instance, action): root = cherrypy.request.app.root - if hasattr(root, 'api_schema'): + if hasattr(instance, 'api_schema'): + api_schema = instance.api_schema + elif hasattr(root, 'api_schema'): api_schema = root.api_schema else: return -- 1.8.1.4

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> On 01/14/2014 02:17 PM, Rodrigo Trujillo wrote:
Currently, the api_schema for the plugins have not been checked. This patch fixes this problem getting the api_schema from the main plugin class.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..676fa08 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -89,7 +89,9 @@ def internal_redirect(url): def validate_params(params, instance, action): root = cherrypy.request.app.root
- if hasattr(root, 'api_schema'): + if hasattr(instance, 'api_schema'): + api_schema = instance.api_schema + elif hasattr(root, 'api_schema'): api_schema = root.api_schema else: return

Currently, the api_schema for the plugins have not been checked. This patch fixes this problem getting the api_schema from the main plugin class.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..676fa08 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -89,7 +89,9 @@ def internal_redirect(url): def validate_params(params, instance, action): root = cherrypy.request.app.root
- if hasattr(root, 'api_schema'): + if hasattr(instance, 'api_schema'): + api_schema = instance.api_schema + elif hasattr(root, 'api_schema'): api_schema = root.api_schema else: return I don't know how you get the conclusion that plugin's api schema validation doesn't When you make request to the plugin app, cherrypy.request.app.root is
On 01/15/2014 12:17 AM, Rodrigo Trujillo wrote: the instance of plugin class. In sample plugin, it's plugins.sample.Drawings. You could can add a print statement to verify it. And the test cases in test_plugin.py also can verify the api schema work.

On 01/16/2014 12:49 AM, Mark Wu wrote:
Currently, the api_schema for the plugins have not been checked. This patch fixes this problem getting the api_schema from the main plugin class.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/control/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/kimchi/control/utils.py b/src/kimchi/control/utils.py index c3c5f8e..676fa08 100644 --- a/src/kimchi/control/utils.py +++ b/src/kimchi/control/utils.py @@ -89,7 +89,9 @@ def internal_redirect(url): def validate_params(params, instance, action): root = cherrypy.request.app.root
- if hasattr(root, 'api_schema'): + if hasattr(instance, 'api_schema'): + api_schema = instance.api_schema + elif hasattr(root, 'api_schema'): api_schema = root.api_schema else: return I don't know how you get the conclusion that plugin's api schema validation doesn't When you make request to the plugin app, cherrypy.request.app.root is
On 01/15/2014 12:17 AM, Rodrigo Trujillo wrote: the instance of plugin class. In sample plugin, it's plugins.sample.Drawings. You could can add a print statement to verify it.
And the test cases in test_plugin.py also can verify the api schema work. You are right, made new tests and it works :) Just sent another patch (v2), without this modifications. Thanks Mark _______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel

The sample plugin needed some modifications to work properly with Kimchi tabs. This patch also disables it. Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- plugins/sample/sample.conf | 2 +- plugins/sample/ui/config/tab-ext.xml | 6 +++--- plugins/sample/ui/tab.html | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) create mode 100644 plugins/sample/ui/tab.html diff --git a/plugins/sample/sample.conf b/plugins/sample/sample.conf index c4e80f7..2f28f50 100644 --- a/plugins/sample/sample.conf +++ b/plugins/sample/sample.conf @@ -1,5 +1,5 @@ [kimchi] -enable = True +enable = False plugin_class = "Drawings" uri = "/plugins/sample" diff --git a/plugins/sample/ui/config/tab-ext.xml b/plugins/sample/ui/config/tab-ext.xml index 948fa07..b040058 100644 --- a/plugins/sample/ui/config/tab-ext.xml +++ b/plugins/sample/ui/config/tab-ext.xml @@ -1,7 +1,7 @@ <?xml version="1.0" encoding="utf-8"?> -<!--<tabs-ext> +<tabs-ext> <tab> <title>Test</title> - <filePath>plugins/sample/ui/tab.html</filePath> + <path>plugins/sample/ui/tab.html</path> </tab> -</tabs-ext>--> \ No newline at end of file +</tabs-ext> diff --git a/plugins/sample/ui/tab.html b/plugins/sample/ui/tab.html new file mode 100644 index 0000000..3cf7988 --- /dev/null +++ b/plugins/sample/ui/tab.html @@ -0,0 +1 @@ +Test tab for sample plugin. -- 1.8.1.4

Reviewed-by: Aline Manera <alinefm@linux.vnet.ibm.com> A comment below On 01/14/2014 02:17 PM, Rodrigo Trujillo wrote:
The sample plugin needed some modifications to work properly with Kimchi tabs. This patch also disables it.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- plugins/sample/sample.conf | 2 +- plugins/sample/ui/config/tab-ext.xml | 6 +++--- plugins/sample/ui/tab.html | 1 + 3 files changed, 5 insertions(+), 4 deletions(-) create mode 100644 plugins/sample/ui/tab.html
diff --git a/plugins/sample/sample.conf b/plugins/sample/sample.conf index c4e80f7..2f28f50 100644 --- a/plugins/sample/sample.conf +++ b/plugins/sample/sample.conf @@ -1,5 +1,5 @@ [kimchi] -enable = True +enable = False plugin_class = "Drawings" uri = "/plugins/sample"
This is the cherrypy config file for the plugin. So if in addition to tabs.xml file more files/dir should be exposed, you need to add the config here.
diff --git a/plugins/sample/ui/config/tab-ext.xml b/plugins/sample/ui/config/tab-ext.xml index 948fa07..b040058 100644 --- a/plugins/sample/ui/config/tab-ext.xml +++ b/plugins/sample/ui/config/tab-ext.xml @@ -1,7 +1,7 @@ <?xml version="1.0" encoding="utf-8"?> -<!--<tabs-ext> +<tabs-ext> <tab> <title>Test</title> - <filePath>plugins/sample/ui/tab.html</filePath> + <path>plugins/sample/ui/tab.html</path> </tab> -</tabs-ext>--> \ No newline at end of file +</tabs-ext> diff --git a/plugins/sample/ui/tab.html b/plugins/sample/ui/tab.html new file mode 100644 index 0000000..3cf7988 --- /dev/null +++ b/plugins/sample/ui/tab.html @@ -0,0 +1 @@ +Test tab for sample plugin.

On 01/14/2014 02:17 PM, Rodrigo Trujillo wrote:
If the plugin tab name string is not found in kimchi i18n file, the tab name is set as 'unknown'. This patch fixes this problem and also adds routines to add tab html file in cherrypy configuration automati cally.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/server.py | 38 ++++++++++++++++++++++++++++++++++++-- ui/js/src/kimchi.main.js | 2 +- 2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/server.py b/src/kimchi/server.py index b820263..919e2e0 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -28,6 +28,10 @@ import logging.handlers import os import sslcert
+ +from xml.etree import ElementTree + + from kimchi import auth from kimchi import config from kimchi import model @@ -208,13 +212,43 @@ class Server(object): script_name = plugin_config['kimchi']['uri'] del plugin_config['kimchi']
+ tabs_extra_file = config.get_plugin_tab_xml(plugin_name) plugin_config['/ui/config/tab-ext.xml'] = { 'tools.staticfile.on': True, - 'tools.staticfile.filename': - config.get_plugin_tab_xml(plugin_name), + 'tools.staticfile.filename': tabs_extra_file, 'tools.nocache.on': True} +
+ # Providing plugin tabs htmls + et = ElementTree.ElementTree(file=tabs_extra_file) + + tabs = et.findall('tab') + for tab in tabs: + html = tab.find('path').text + tabs_extra_file = os.path.join(config.get_prefix(), html) + if html.startswith('/'): + html = html.replace(script_name,'') + else: + html = html.replace(script_name[1:],'') + plugin_config[html] = { + 'tools.staticfile.on': True, + 'tools.staticfile.filename': tabs_extra_file, + 'tools.nocache.on': True} + except KeyError: continue + except IOError as e: + msg = "Failed to load plugin tabs file %s" %tabs_extra_file + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue + except ElementTree.ParseError as e: + msg = "XML parse error in file %s: %s" %( + tabs_extra_file, e.msg) + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue
From kimchi perspective, we only need to know the tabs.xml for each plugin - so UI can load the interface in one view All other cherrypy configuration should be handled by the plugin config file. So all the above code isn't needed.
try: plugin_app = import_class(plugin_class)() diff --git a/ui/js/src/kimchi.main.js b/ui/js/src/kimchi.main.js index cc8afee..8b84a34 100644 --- a/ui/js/src/kimchi.main.js +++ b/ui/js/src/kimchi.main.js @@ -211,7 +211,7 @@ kimchi.getTabHtml = function(url) { $(xmlData).find('tab').each(function() { var $tab = $(this); var titleKey = $tab.find('title').text(); - var title = i18n[titleKey]; + var title = i18n[titleKey] || titleKey; var path = $tab.find('path').text(); tabsHtml += "<li><a class='item' href=" + path + ">" + title + "</a></li>"; });

On 01/16/2014 12:31 AM, Aline Manera wrote:
On 01/14/2014 02:17 PM, Rodrigo Trujillo wrote:
If the plugin tab name string is not found in kimchi i18n file, the tab name is set as 'unknown'. This patch fixes this problem and also adds routines to add tab html file in cherrypy configuration automati cally.
Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo@linux.vnet.ibm.com> --- src/kimchi/server.py | 38 ++++++++++++++++++++++++++++++++++++-- ui/js/src/kimchi.main.js | 2 +- 2 files changed, 37 insertions(+), 3 deletions(-)
diff --git a/src/kimchi/server.py b/src/kimchi/server.py index b820263..919e2e0 100644 --- a/src/kimchi/server.py +++ b/src/kimchi/server.py @@ -28,6 +28,10 @@ import logging.handlers import os import sslcert
+ +from xml.etree import ElementTree + + from kimchi import auth from kimchi import config from kimchi import model @@ -208,13 +212,43 @@ class Server(object): script_name = plugin_config['kimchi']['uri'] del plugin_config['kimchi']
+ tabs_extra_file = config.get_plugin_tab_xml(plugin_name) plugin_config['/ui/config/tab-ext.xml'] = { 'tools.staticfile.on': True, - 'tools.staticfile.filename': - config.get_plugin_tab_xml(plugin_name), + 'tools.staticfile.filename': tabs_extra_file, 'tools.nocache.on': True} +
+ # Providing plugin tabs htmls + et = ElementTree.ElementTree(file=tabs_extra_file) + + tabs = et.findall('tab') + for tab in tabs: + html = tab.find('path').text + tabs_extra_file = os.path.join(config.get_prefix(), html) + if html.startswith('/'): + html = html.replace(script_name,'') + else: + html = html.replace(script_name[1:],'') + plugin_config[html] = { + 'tools.staticfile.on': True, + 'tools.staticfile.filename': tabs_extra_file, + 'tools.nocache.on': True} + except KeyError: continue + except IOError as e: + msg = "Failed to load plugin tabs file %s" %tabs_extra_file + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue + except ElementTree.ParseError as e: + msg = "XML parse error in file %s: %s" %( + tabs_extra_file, e.msg) + cherrypy.log.error_log.error(msg) + cherrypy.log.error_log.error('Ignoring plugin "%s"', + plugin_name) + continue
From kimchi perspective, we only need to know the tabs.xml for each plugin - so UI can load the interface in one view All other cherrypy configuration should be handled by the plugin config file. So all the above code isn't needed.
Yeap, agree. Just sent a new patch without this modifications Thanks.
try: plugin_app = import_class(plugin_class)() diff --git a/ui/js/src/kimchi.main.js b/ui/js/src/kimchi.main.js index cc8afee..8b84a34 100644 --- a/ui/js/src/kimchi.main.js +++ b/ui/js/src/kimchi.main.js @@ -211,7 +211,7 @@ kimchi.getTabHtml = function(url) { $(xmlData).find('tab').each(function() { var $tab = $(this); var titleKey = $tab.find('title').text(); - var title = i18n[titleKey]; + var title = i18n[titleKey] || titleKey; var path = $tab.find('path').text(); tabsHtml += "<li><a class='item' href=" + path + ">" + title + "</a></li>"; });
_______________________________________________ Kimchi-devel mailing list Kimchi-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
participants (3)
-
Aline Manera
-
Mark Wu
-
Rodrigo Trujillo