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

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.

>> >>>>> 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(a)linux.vnet.ibm.com&gt; >>>>>>> --- >>>>>>> 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(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