[Kimchi-devel] [PATCH 01/16] Move generate_action_handler() function to Resource() class

[Aline Manera]

On 12/24/2013 01:28 AM, Zhou Zheng Sheng wrote:
> on 2013/12/24 02:41, Aline Manera wrote:
>> From: Aline Manera <alinefm at br.ibm.com>
>>
>> generate_action_handler() function is only used by Resources instances so move
>> it to Resource()
>>
>> Signed-off-by: Aline Manera <alinefm at br.ibm.com>
>> ---
>>   src/kimchi/controller.py |   75 +++++++++++++++++++++++-----------------------
>>   1 file changed, 37 insertions(+), 38 deletions(-)
>>
>> diff --git a/src/kimchi/controller.py b/src/kimchi/controller.py
>> index 2940278..ba8b53d 100644
>> --- a/src/kimchi/controller.py
>> +++ b/src/kimchi/controller.py
>> @@ -103,33 +103,6 @@ def validate_params(params, instance, action):
>>               e.message for e in validator.iter_errors(request)))
>>
>>
>> -def generate_action_handler(instance, action_name, action_args=None):
>> -    def wrapper(*args, **kwargs):
>> -        validate_method(('POST'))
>> -        try:
>> -            model_args = list(instance.model_args)
>> -            if action_args is not None:
>> -                model_args.extend(parse_request()[key] for key in action_args)
>> -            fn = getattr(instance.model, model_fn(instance, action_name))
>> -            fn(*model_args)
>> -            raise internal_redirect(instance.uri_fmt %
>> -                                    tuple(instance.model_args))
>> -        except MissingParameter, param:
>> -            raise cherrypy.HTTPError(400, "Missing parameter: '%s'" % param)
>> -        except InvalidParameter, param:
>> -            raise cherrypy.HTTPError(400, "Invalid parameter: '%s'" % param)
>> -        except InvalidOperation, msg:
>> -            raise cherrypy.HTTPError(400, "Invalid operation: '%s'" % msg)
>> -        except OperationFailed, msg:
>> -            raise cherrypy.HTTPError(500, "Operation Failed: '%s'" % msg)
>> -        except NotFoundError, msg:
>> -            raise cherrypy.HTTPError(404, "Not found: '%s'" % msg)
>> -
>> -    wrapper.__name__ = action_name
>> -    wrapper.exposed = True
>> -    return wrapper
>> -
>> -
>>   class Resource(object):
>>       """
>>       A Resource represents a single entity in the API (such as a Virtual Machine)
>> @@ -153,6 +126,32 @@ class Resource(object):
>>           self.model_args = (ident,)
>>           self.update_params = []
>>
>> +    def generate_action_handler(self, instance, action_name, action_args=None):
> Hi, it seems the instance argument is redundant here. While we call
> "obj.method(a, b, c)", the "self" is passed to "def method(self, a, b,
> c)" as the first argument implicitly. Here "instance" and "self" serves
> the same purposes and refer to the same thing. So I think, "instance" in
> the function argument list can be removed, and change all "instance" in
> the function body to "self". This method should be called like following.
>    self.start = self.generate_action_handler('start')
> This is how object works in Python.

ACK.

I've just moved it inside the class and don't pay attention in the 
function itself.
I will changed it on V2.

Thanks

>> +        def wrapper(*args, **kwargs):
>> +            validate_method(('POST'))
>> +            try:
>> +                model_args = list(instance.model_args)
>> +                if action_args is not None:
>> +                    model_args.extend(parse_request()[key] for key in action_args)
>> +                fn = getattr(instance.model, model_fn(instance, action_name))
>> +                fn(*model_args)
>> +                raise internal_redirect(instance.uri_fmt %
>> +                                        tuple(instance.model_args))
>> +            except MissingParameter, param:
>> +                raise cherrypy.HTTPError(400, "Missing parameter: '%s'" % param)
>> +            except InvalidParameter, param:
>> +                raise cherrypy.HTTPError(400, "Invalid parameter: '%s'" % param)
>> +            except InvalidOperation, msg:
>> +                raise cherrypy.HTTPError(400, "Invalid operation: '%s'" % msg)
>> +            except OperationFailed, msg:
>> +                raise cherrypy.HTTPError(500, "Operation Failed: '%s'" % msg)
>> +            except NotFoundError, msg:
>> +                raise cherrypy.HTTPError(404, "Not found: '%s'" % msg)
>> +
>> +        wrapper.__name__ = action_name
>> +        wrapper.exposed = True
>> +        return wrapper
>> +
>>       def lookup(self):
>>           try:
>>               lookup = getattr(self.model, model_fn(self, 'lookup'))
>> @@ -369,9 +368,9 @@ class VM(Resource):
>>           self.update_params = ["name"]
>>           self.screenshot = VMScreenShot(model, ident)
>>           self.uri_fmt = '/vms/%s'
>> -        self.start = generate_action_handler(self, 'start')
>> -        self.stop = generate_action_handler(self, 'stop')
>> -        self.connect = generate_action_handler(self, 'connect')
>> +        self.start = self.generate_action_handler(self, 'start')
>> +        self.stop = self.generate_action_handler(self, 'stop')
>> +        self.connect = self.generate_action_handler(self, 'connect')
>>
>>       @property
>>       def data(self):
>> @@ -453,8 +452,8 @@ class Network(Resource):
>>       def __init__(self, model, ident):
>>           super(Network, self).__init__(model, ident)
>>           self.uri_fmt = "/networks/%s"
>> -        self.activate = generate_action_handler(self, 'activate')
>> -        self.deactivate = generate_action_handler(self, 'deactivate')
>> +        self.activate = self.generate_action_handler(self, 'activate')
>> +        self.deactivate = self.generate_action_handler(self, 'deactivate')
>>
>>       @property
>>       def data(self):
>> @@ -475,8 +474,8 @@ class StorageVolume(Resource):
>>           self.info = {}
>>           self.model_args = [self.pool, self.ident]
>>           self.uri_fmt = '/storagepools/%s/storagevolumes/%s'
>> -        self.resize = generate_action_handler(self, 'resize', ['size'])
>> -        self.wipe = generate_action_handler(self, 'wipe')
>> +        self.resize = self.generate_action_handler(self, 'resize', ['size'])
>> +        self.wipe = self.generate_action_handler(self, 'wipe')
>>
>>       @property
>>       def data(self):
>> @@ -522,8 +521,8 @@ class StoragePool(Resource):
>>           super(StoragePool, self).__init__(model, ident)
>>           self.update_params = ["autostart"]
>>           self.uri_fmt = "/storagepools/%s"
>> -        self.activate = generate_action_handler(self, 'activate')
>> -        self.deactivate = generate_action_handler(self, 'deactivate')
>> +        self.activate = self.generate_action_handler(self, 'activate')
>> +        self.deactivate = self.generate_action_handler(self, 'deactivate')
>>
>>       @property
>>       def data(self):
>> @@ -689,8 +688,8 @@ class Host(Resource):
>>           self.stats = HostStats(self.model)
>>           self.stats.exposed = True
>>           self.uri_fmt = '/host/%s'
>> -        self.reboot = generate_action_handler(self, 'reboot')
>> -        self.shutdown = generate_action_handler(self, 'shutdown')
>> +        self.reboot = self.generate_action_handler(self, 'reboot')
>> +        self.shutdown = self.generate_action_handler(self, 'shutdown')
>>           self.partitions = Partitions(self.model)
>>           self.partitions.exposed = True
>>
>