[Kimchi-devel] [PATCH v2 1/2] Break the 'sample' plugin's monolithic model into several smaller ones

Aline Manera alinefm at linux.vnet.ibm.com
Wed Jan 29 14:22:14 UTC 2014


Hi Mark,

Thanks for adding tests to it.
I will integrate those patches to the refactor model ones - so we avoid 
conflicts
and re-work while merging

On 01/23/2014 03:41 AM, Mark Wu wrote:
> From: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
>
> In kimchi the model is always a monolithic class. This is not good from
> the point of modulization, and it is not necessary to put everything
> into one big model. Unfortunately the problem will not get solved
> unless we conduct enormous refactor against the whole controller-model
> architecture.
>
> This patch tries to mitigate the problem by firstly splitting the model
> class into some smaller ones, than automatically mapping all sub-model
> instance methods to a RootModel instance. As a demonstration, this patch
> splits the 'sample' plugin's model.
>
> Signed-off-by: Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> ---
>   plugins/__init__.py     |  23 ++++++++++
>   plugins/sample/model.py | 109 ++++++++++++++++++++++++++++++------------------
>   src/kimchi/model.py     |  26 ++++++++++++
>   3 files changed, 117 insertions(+), 41 deletions(-)
>
> diff --git a/plugins/__init__.py b/plugins/__init__.py
> index e69de29..4103a65 100644
> --- a/plugins/__init__.py
> +++ b/plugins/__init__.py
> @@ -0,0 +1,23 @@
> +#
> +# Project Kimchi
> +#
> +# Copyright IBM, Corp. 2014
> +#
> +# Authors:
> +#  Zhou Zheng Sheng <zhshzhou at linux.vnet.ibm.com>
> +#
> +# This library is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU Lesser General Public
> +# License as published by the Free Software Foundation; either
> +# version 2.1 of the License, or (at your option) any later version.
> +#
> +# This library is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +# Lesser General Public License for more details.
> +#
> +# You should have received a copy of the GNU Lesser General Public
> +# License along with this library; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> +
> +
> diff --git a/plugins/sample/model.py b/plugins/sample/model.py
> index 9a2f22f..e606176 100644
> --- a/plugins/sample/model.py
> +++ b/plugins/sample/model.py
> @@ -21,79 +21,106 @@
>   # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>
>   from kimchi.exception import InvalidOperation, NotFoundError
> +from kimchi.model import RootModel
>
>
> -class Model(object):
> +class CirclesModel(object):
> +    def __init__(self):
> +        self._circles = {}
> +
> +    def create(self, params):
> +        name = params['name']
> +        if name in self._circles:
> +            raise InvalidOperation("Circle %s already exists" % name)
> +        self._circles[name] = Circle(params['radius'])
> +        return name
>
> +    def get_list(self):
> +        return sorted(self._circles)
> +
> +
> +class CircleModel(object):
> +    def __init__(self, parent_model):
> +        # Circel and Circles models are friends, it's OK to share _circles.
> +        self._circles = parent_model._circles
> +
> +    def lookup(self, name):
> +        try:
> +            circle = self._circles[name]
> +        except KeyError:
> +            raise NotFoundError("Circle %s not found" % name)
> +        return {'radius': circle.radius}
> +
> +    def update(self, name, params):
> +        if name not in self._circles:
> +            raise NotFoundError("Circle %s not found" % name)
> +        self._circles[name].radius = params['radius']
> +        return name
> +
> +    def delete(self, name):
> +        try:
> +            del self._circles[name]
> +        except KeyError:
> +            pass
> +
> +
> +class RectanglesModel(object):
>       def __init__(self):
> -        self.rectangles = {}
> -        self.circles = {}
> +        self._rectangles = {}
>
> -    def rectangles_create(self, params):
> +    def create(self, params):
>           name = params['name']
> -        if name in self.rectangles:
> +        if name in self._rectangles:
>               raise InvalidOperation("Rectangle %s already exists" % name)
> -        self.rectangles[name] = Rectangle(params['length'], params['width'])
> +        self._rectangles[name] = Rectangle(params['length'], params['width'])
>           return name
>
> -    def rectangles_get_list(self):
> -        return sorted(self.rectangles)
> +    def get_list(self):
> +        return sorted(self._rectangles)
> +
>
> -    def rectangle_lookup(self, name):
> +class RectangleModel(object):
> +    def __init__(self, parent_model):
> +        self._rectangles = parent_model._rectangles
> +
> +    def lookup(self, name):
>           try:
> -            rectangle = self.rectangles[name]
> +            rectangle = self._rectangles[name]
>           except KeyError:
>               raise NotFoundError("Rectangle %s not found" % name)
>           return {'length': rectangle.length, 'width': rectangle.width}
>
> -    def rectangle_update(self, name, params):
> -        if name not in self.rectangles:
> +    def update(self, name, params):
> +        if name not in self._rectangles:
>               raise NotFoundError("Rectangle %s not found" % name)
>           try:
> -            self.rectangles[name].length = params['length']
> +            self._rectangles[name].length = params['length']
>           except KeyError:
>               pass
>
>           try:
> -            self.rectangles[name].width = params['width']
> +            self._rectangles[name].width = params['width']
>           except KeyError:
>               pass
>           return name
>
> -    def rectangle_delete(self, name):
> +    def delete(self, name):
>           try:
> -            del self.rectangles[name]
> +            del self._rectangles[name]
>           except KeyError:
>               pass
>
> -    def circles_create(self, params):
> -        name = params['name']
> -        if name in self.circles:
> -            raise InvalidOperation("Circle %s already exists" % name)
> -        self.circles[name] = Circle(params['radius'])
> -        return name
>
> -    def circles_get_list(self):
> -        return sorted(self.circles)
> -
> -    def circle_lookup(self, name):
> -        try:
> -            circle = self.circles[name]
> -        except KeyError:
> -            raise NotFoundError("Circle %s not found" % name)
> -        return {'radius': circle.radius}
> +class Model(RootModel):
> +    def __init__(self):
> +        circles = CirclesModel()
> +        circle = CircleModel(circles)
>
> -    def circle_update(self, name, params):
> -        if name not in self.circles:
> -            raise NotFoundError("Circle %s not found" % name)
> -        self.circles[name].radius = params['radius']
> -        return name
> +        rectangles = RectanglesModel()
> +        rectangle = RectangleModel(rectangles)
>
> -    def circle_delete(self, name):
> -        try:
> -            del self.circles[name]
> -        except KeyError:
> -            pass
> +        return super(Model, self).__init__(
> +            [circle, circles, rectangle, rectangles])
>
>
>   class Rectangle(object):
> diff --git a/src/kimchi/model.py b/src/kimchi/model.py
> index ea528c6..47f2d22 100644
> --- a/src/kimchi/model.py
> +++ b/src/kimchi/model.py
> @@ -1853,3 +1853,29 @@ class LibvirtConnection(object):
>                   # However the values need to be considered wisely to not affect
>                   # hosts which are hosting a lot of virtual machines
>               return conn
> +
> +
> +class RootModel(object):
> +    '''
> +    This model squashes all sub-model's public callable methods to itself.
> +
> +    Model methods are not limited to get_list, create, lookup, update, delete.
> +    Controller can call generate_action_handler to generate new actions, which
> +    call the related model methods. So all public callable methods of a
> +    sub-model should be mapped to this model.
> +    '''
> +    def __init__(self, model_instances):
> +        for model_instance in model_instances:
> +            cls_name = model_instance.__class__.__name__
> +            if cls_name.endswith('Model'):
> +                method_prefix = cls_name[:-len('Model')].lower()
> +            else:
> +                method_prefix = cls_name.lower()
> +
> +            callables = [m for m in dir(model_instance)
> +                         if not m.startswith('_') and
> +                         callable(getattr(model_instance, m))]
> +
> +            for member_name in callables:
> +                m = getattr(model_instance, member_name, None)
> +                setattr(self, '%s_%s' % (method_prefix, member_name), m)




More information about the Kimchi-devel mailing list