[Kimchi-devel] [PATCH] Disable static path checking by cherrypy

[Aline Manera]

On 23/01/2015 20:34, Christy Perez wrote:
>
> On 01/23/2015 12:04 PM, Christy Perez wrote:
>>
>> On 01/23/2015 10:07 AM, Aline Manera wrote:
>>> On 23/01/2015 12:34, Christy Perez wrote:
>>>> On 01/23/2015 06:50 AM, Aline Manera wrote:
>>>>> On 22/01/2015 21:51, Christy Perez wrote:
>>>>>> On 01/22/2015 10:41 AM, Aline Manera wrote:
>>>>>>> On 21/01/2015 17:51, Christy Perez wrote:
>>>>>>>> CherryPy has a checker module that runs certain tests for resources.
>>>>>>>> One such test is for static paths, in which is tests all resources
>>>>>>>> using a non-existent html file. Since Kimchi is not a static web app,
>>>>>>>> this check can be safely disregarded.
>>>>>>> Kimchi servers some static files and directories in its configuration.
>>>>>>> So I am not sure it is a good solution.
>>>>>>>
>>>>>>>> The motivation behind this change is the following scenario:
>>>>>>>>
>>>>>>>> The need to treat a certain flavor of a resource differently takes
>>>>>>>> advantage of the base model passed to the control's init function.
>>>>>>>> When calling the base model function to retreive the desired
>>>>>>>> property,
>>>>>>>> an exception is thrown because the underlying resource doesn't exist.
>>>>>>>>
>>>>>>>> This brings to light that any use of the base model from the init
>>>>>>>> of any control object will result in the same checker issue.
>>>>>>>>
>>>>>>>> To easily test this issue:
>>>>>>>> - Add the following line to the __init__ function in control/vms.py:
>>>>>>>>         model.vm_lookup(self.ident.decode('utf-8'))
>>>>>>>> - Run the rest tests: $ sudo ./run_tests.sh test_rest.RestTests
>>>>>>> To solve it we could add a new parameter "destroyable" to
>>>>>>> generate_action_handler()
>>>>>> What would qualify something as destroyable?
>>>>> A "destroyable" function will be that one which deletes a resource if it
>>>>> is non-persistent.
>>>>>
>>>>> Like, vm.poweroff(), vm.shutoff(), network.deactivate(), etc
>>>> So, this is completely decoupled from the non-persistent issue. I'd like
>>>> to find a way to fix this for any object that might hit it. It doesn't
>>>> happen because a resource isn't persistent, either.
>>> I am not sure I understood your point.
>>>
>>> According to "destroyable" value and persistent data we will raise or
>>> not the exception.
>>>
>>> data = self.lookup()
>>>
>>> if not destroyable and 'persistent' in data.keys() and data['persistent']:
>>>     raise NotFoundError()
>>>
>> I'm trying to say that this will happen for anything that tries to
>> access the underlying model. If you put the same line into the templates
>> control, you'll get the same error. So if anyone ever wanted to do any
>> special handling for templates, they'll have to figure out a way around
>> it (just like you're proposing for the vms). So this patch was me trying
>> to figure out a way to deal with the fact that the cherrypy checker is
>> going to ask for nonexistent objects in general. My hope was to make
>> life easier for us in the future (for everything).
>>
>> Better?
>>
> But FWIW, I've done a lot more looking at the cherrypy code, and I'm
> going to abandon this approach. The correct thing to do is to make sure
> that the control's init never raises an exception, and what cherrypy is
> doing is valid. So, I'll go back to reworking my previous patchset.
>
> But it's good to keep in mind that this will have to be taken into
> account for any other object type, should we attempt to use the
> basemodel functions in the its control's init.
>

Ok. Thanks for the details.

>>>
>>>>>>     If you mean a
>>>>>> non-persistent object ... that's more narrow of a scope than the
>>>>>> problem
>>>>>> I was trying to solve with this.
>>>>>>
>>>>>>> _generate_action_handler_base() already calls lookup() so we can check
>>>>>>> the return data
>>>>>>>
>>>>>>> data = self.lookup()
>>>>>>>
>>>>>>> If not destroyable and 'persistent' in data.keys() and
>>>>>>> data['persistent']:
>>>>>>>        raise NotFoundError()
>>>>>>>
>>>>>>>> See: cherrypy._cpchecker
>>>>>>>>
>>>>>>>> Signed-off-by: Christy Perez <christy at linux.vnet.ibm.com>
>>>>>>>> ---
>>>>>>>>      src/kimchi/config.py.in | 6 +++---
>>>>>>>>      1 file changed, 3 insertions(+), 3 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/src/kimchi/config.py.in b/src/kimchi/config.py.in
>>>>>>>> index 83a5dd0..86ca862 100644
>>>>>>>> --- a/src/kimchi/config.py.in
>>>>>>>> +++ b/src/kimchi/config.py.in
>>>>>>>> @@ -23,10 +23,9 @@ import os
>>>>>>>>      import platform
>>>>>>>>      import threading
>>>>>>>>
>>>>>>>> -
>>>>>>>> +from cherrypy import checker
>>>>>>>>      from ConfigParser import SafeConfigParser
>>>>>>>>
>>>>>>>> -
>>>>>>>>      from kimchi.xmlutils.utils import xpath_get_text
>>>>>>>>
>>>>>>>>      __version__ = "@kimchiversion@"
>>>>>>>> @@ -40,7 +39,6 @@ kimchiLock = threading.Lock()
>>>>>>>>      # Storage pool constant for read-only pool types
>>>>>>>>      READONLY_POOL_TYPE = ['iscsi', 'scsi', 'mpath']
>>>>>>>>
>>>>>>>> -
>>>>>>>>      def get_object_store():
>>>>>>>>          return os.path.join(paths.state_dir, 'objectstore')
>>>>>>>>
>>>>>>>> @@ -182,6 +180,8 @@ class UIConfig(dict):
>>>>>>>>      class KimchiConfig(dict):
>>>>>>>>          # session time out is 10 minutes
>>>>>>>>          SESSIONSTIMEOUT = 10
>>>>>>>> +    global checker
>>>>>>>> +    checker.check_static_paths = False
>>>>>>>>
>>>>>>>>          kimchi_config = {
>>>>>>>>              '/': {
>> _______________________________________________
>> Kimchi-devel mailing list
>> Kimchi-devel at ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>