[Kimchi-devel] [PATCH] [Wok 2/7] Improve plugin lookup by request

[Lucio Correia]

On 01-09-2016 12:46, Lucio Correia wrote:
> On 01-09-2016 10:25, Aline Manera wrote:
>>
>>
>> On 08/31/2016 06:06 PM, Lucio Correia wrote:
>>> Currently get_plugin_from_request() assumes plugin's
>>> root URI is always /plugins/<plugin_name>, which is
>>> in fact configurable by each plugin.
>>>
>>> This patch fixes it by mapping plugins' root URI
>>> on load time, and replacing get_plugin_from_request()
>>> by a more generic get_plugin_from_uri() to be used
>>> also in AsyncTask.
>>>
>>> Signed-off-by: Lucio Correia <luciojhc at linux.vnet.ibm.com>
>>> ---
>>>   src/wok/control/base.py |  4 ++--
>>>   src/wok/utils.py        | 14 +++++++++-----
>>>   2 files changed, 11 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/wok/control/base.py b/src/wok/control/base.py
>>> index f4443c8..213e6b5 100644
>>> --- a/src/wok/control/base.py
>>> +++ b/src/wok/control/base.py
>>> @@ -33,7 +33,7 @@ from wok.control.utils import validate_params
>>>   from wok.exception import InvalidOperation, UnauthorizedError,
>>> WokException
>>>   from wok.reqlogger import RequestRecord
>>>   from wok.stringutils import encode_value, utf8_dict
>>> -from wok.utils import get_plugin_from_request, wok_log
>>> +from wok.utils import get_plugin_from_uri, wok_log
>>>
>>>
>>>   # Default request log messages
>>> @@ -47,7 +47,7 @@ def log_request(code, params, exception, method,
>>> status, user=None):
>>>       log_id = RequestRecord(
>>>           {'code': code, 'params': params},
>>>           exception,
>>> -        app=get_plugin_from_request(),
>>
>>> +        app=get_plugin_from_uri(cherrypy.request.script_name),
>>
>> Maybe you can use "cherrypy.request.app.root.domain" directly or... (see
>> below)
>>

It seems to work. But it does not work in AsyncTask (probably out of 
request scope). See below, let's discuss this in patch 4/7.


======================================================================
ERROR: test_async_tasks (test_tasks.AsyncTaskTests)
----------------------------------------------------------------------
Traceback (most recent call last):
   File "test_tasks.py", line 61, in test_async_tasks
     taskid = AsyncTask('', self._quick_op, 'Hello').id
   File "/home/dev/wok/src/wok/asynctask.py", line 61, in __init__
     self.plugin = cherrypy.request.app.root.domain
AttributeError: 'NoneType' object has no attribute 'root'


>>>           req=method,
>>>           status=status,
>>>           user=user or cherrypy.session.get(USER_NAME, 'N/A'),
>>> diff --git a/src/wok/utils.py b/src/wok/utils.py
>>> index 7599e85..b3d252c 100644
>>> --- a/src/wok/utils.py
>>> +++ b/src/wok/utils.py
>>> @@ -44,6 +44,7 @@ from wok.exception import InvalidParameter,
>>> TimeoutExpired
>>>   from wok.stringutils import decode_value
>>>
>>>
>>> +plugin_root = {}
>>>   wok_log = cherrypy.log.error_log
>>>
>>>
>>> @@ -82,6 +83,10 @@ def get_enabled_plugins():
>>>               plugin_config = _load_plugin_conf(name)
>>>               try:
>>>                   if plugin_config['wok']['enable']:
>>
>>> +                    # update plugin lookup dict
>>> +                    uri = plugin_config['wok']['uri']
>>> +                    plugin_root[name] = uri
>>> +
>>
>> It is better to have the uri as the key. So you can easily grab the
>> application name based on uri.
>>
>> plugin_root[uri] = name
>
> That was the original idea, but I did this way because the usage of
> asynctask (it needed starts with to deal with target_uri).
>
> Since you gave me an alternative for that usage, I will test it and can
> change this afterwards.

In fact I need name -> uri because usage of reqlogger.py (patch 3/7). I 
need uri in the format /plugins/kimchi there. That's why I use 
startswith() here to lookup the correct plugin based on target_uri. 
Let's discuss this in patch 4/7.

>
>>
>>>                       yield (name, plugin_config)
>>>               except (TypeError, KeyError):
>>>                   continue
>>> @@ -106,14 +111,13 @@ def get_all_tabs():
>>>       return tabs
>>>
>>>
>>> -def get_plugin_from_request():
>>> +def get_plugin_from_uri(uri):
>>>       """
>>>       Returns name of plugin being requested. If no plugin, returns
>>> 'wok'.
>>>       """
>>
>>> -    script_name = cherrypy.request.script_name
>>> -    split = script_name.split('/')
>>> -    if len(split) > 2 and split[1] == 'plugins':
>>> -        return split[2]
>>> +    for plugin, root in plugin_root.iteritems():
>>> +        if uri.startswith(root):
>>> +            return plugin
>>>
>>>       return 'wok'
>>>
>>
>> Replase all the above block to:
>>
>> return plugin_root.get(uri, 'wok')
>>
>
> OK if the above works.
>
>


-- 
Lucio Correia
Software Engineer
IBM LTC Brazil