[Kimchi-devel] [PATCH] Controller: Improve Kimchi Specific Exception Reporting

[Sheldon]

On 03/19/2014 03:42 PM, Mark Wu wrote:
> On 03/18/2014 03:16 PM, zhshzhou at linux.vnet.ibm.com wrote:
>> From: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>>
>> The resource and collection class catches exception in the index method
>> and translates to HTTP error code with error message. While this is good
>> and friendly to the user, the exception handling code is somewhat
>> ad-hoc.
>>
>> The first problem is that it does not catch and translate all the Kimchi
>> exception. When new code or feature is added to Kimchi, it's possible
>> that the new code raises an exception that is not expected before. In
>> this case it fails to translate the error message, so the cherrypy
>> framework translates it to HTTP code 500 with a vague message saying 
>> "The
>> server encountered an internal error". This is not friendly to the user.
>> In this patch it "except KimchiException" at the last of the except
>> sequence, and translates it into HTTP error code 500 with specific error
>> message, so that the user knows what's going on.
> Yes, it's a necessary improvement for user experience.
>>
>> The second problem is that the index method handles GET, POST, PUT and
>> DELETE requests, and the error translating code for each type of request
>> is duplicated. This is not necessary. So this patch merges the error
>> handling code.
> I agree with that we need do some cleanup on the code you mentioned.
> But I prefer to split it into another patch since I hope we have a 
> further refactoring.
> I think we can add a http error code for kimchi exception, and then we 
> can construct HTTPError
> from kimchi exception by one method defined in the exception class

we can overridden the HTTPError.

We have send a patch to overridden the HTTPError before.
http://lists.ovirt.org/pipermail/kimchi-devel/2014-January/002004.html

seems later others think the kimchi exception can derived from HTTPError.
>>
>> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>> ---
>> src/kimchi/control/base.py | 76 
>> +++++++++++++++++++---------------------------
>> 1 file changed, 32 insertions(+), 44 deletions(-)
>>
>> diff --git a/src/kimchi/control/base.py b/src/kimchi/control/base.py
>> index d2d041e..320239b 100644
>> --- a/src/kimchi/control/base.py
>> +++ b/src/kimchi/control/base.py
>> @@ -26,6 +26,7 @@ from kimchi.control.utils import get_class_name, 
>> internal_redirect, model_fn
>> from kimchi.control.utils import parse_request, validate_method
>> from kimchi.control.utils import validate_params
>> from kimchi.exception import InvalidOperation, InvalidParameter
>> +from kimchi.exception import KimchiException
>> from kimchi.exception import MissingParameter, NotFoundError, 
>> OperationFailed
>>
>>
>> @@ -87,6 +88,8 @@ class Resource(object):
>> raise cherrypy.HTTPError(500, e.message)
>> except NotFoundError, e:
>> raise cherrypy.HTTPError(404, e.message)
>> + except KimchiException, e:
>> + raise cherrypy.HTTPError(500, e.message)
>>
>> wrapper.__name__ = action_name
>> wrapper.exposed = True
>> @@ -116,31 +119,20 @@ class Resource(object):
>> @cherrypy.expose
>> def index(self):
>> method = validate_method(('GET', 'DELETE', 'PUT'))
>> - if method == 'GET':
>> - try:
>> - return self.get()
>> - except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> - except InvalidOperation, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except OperationFailed, e:
>> - raise cherrypy.HTTPError(406, e.message)
>> - elif method == 'DELETE':
>> - try:
>> - return self.delete()
>> - except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> - except InvalidParameter, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - elif method == 'PUT':
>> - try:
>> - return self.update()
>> - except InvalidParameter, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except InvalidOperation, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> + try:
>> + return {'GET': self.get,
>> + 'DELETE': self.delete,
>> + 'PUT': self.update}[method]()
>> + except InvalidOperation, e:
>> + raise cherrypy.HTTPError(400, e.message)
>> + except InvalidParameter, e:
>> + raise cherrypy.HTTPError(400, e.message)
>> + except NotFoundError, e:
>> + raise cherrypy.HTTPError(404, e.message)
>> + except OperationFailed, e:
>> + raise cherrypy.HTTPError(500, e.message)
>> + except KimchiException, e:
>> + raise cherrypy.HTTPError(500, e.message)
>>
>> def update(self):
>> try:
>> @@ -269,29 +261,25 @@ class Collection(object):
>> @cherrypy.expose
>> def index(self, *args, **kwargs):
>> method = validate_method(('GET', 'POST'))
>> - if method == 'GET':
>> - try:
>> + try:
>> + if method == 'GET':
>> filter_params = cherrypy.request.params
>> validate_params(filter_params, self, 'get_list')
>> return self.get(filter_params)
>> - except InvalidOperation, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> -
>> - elif method == 'POST':
>> - try:
>> + elif method == 'POST':
>> return self.create(parse_request(), *args)
>> - except MissingParameter, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except InvalidParameter, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except OperationFailed, e:
>> - raise cherrypy.HTTPError(500, e.message)
>> - except InvalidOperation, e:
>> - raise cherrypy.HTTPError(400, e.message)
>> - except NotFoundError, e:
>> - raise cherrypy.HTTPError(404, e.message)
>> + except InvalidOperation, e:
>> + raise cherrypy.HTTPError(400, e.message)
>> + except InvalidParameter, e:
>> + raise cherrypy.HTTPError(400, e.message)
>> + except MissingParameter, e:
>> + raise cherrypy.HTTPError(400, e.message)
>> + except NotFoundError, e:
>> + raise cherrypy.HTTPError(404, e.message)
>> + except OperationFailed, e:
>> + raise cherrypy.HTTPError(500, e.message)
>> + except KimchiException, e:
>> + raise cherrypy.HTTPError(500, e.message)
>>
>>
>> class AsyncCollection(Collection):
>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>
>
>


-- 
Thanks and best regards!

Sheldon Feng(冯少合)<shaohef at linux.vnet.ibm.com>
IBM Linux Technology Center