Open Stack

I've been doing some pondering on how Keystone handles the various
pluggable systems with it's Manager / Driver architecture.

Currently we implement the base driver class as follows:

There is a driver object that has a number of reference functions defined
on it, each of these functions typically raises NotImplemented() and has a
docstring describing the expected behavior.  A simple example of this would
be the Token Driver base class.  A complex example would be the Identity
Driver base class.

Example:

class AbstractBaseClassTest(object):

    def abstract_method1(args, kwargs):

        raise NotImplemented()

    def abstract_method2(args, kwargs):
        ...


This type of system is not inherently bad, nor flawed, but there are some
cases when I've run into situations that raise a NotImplemented() error
unexpectedly (usually in custom code I've had to write around a driver or
replace a driver with and "internal to my company" implementation).


For those not familiar with ABCMeta, abstract base classes, and the abc
module:

In a model that uses an abstract metaclass, the base class methods that
need to be overridden are decorated with the "abstractmethod" decorator
from the "abc" module.  This decorator when coupled with the ABCMeta
metaclass, requires all abstract methods to be overridden by the class that
inherits from the base class before the class can be instantiated.
 Similarly there is an "abstractproperty" decorator for @property methods.

The exception raised looks like:

TypeError: Can't instantiate abstract class TestClass with abstract methods
AbsTest1, AbsTest2


The benefits are two fold.  First, this means that a new driver could not
be implemented without overriding the expected methods (no raising
"NotImplemented()" unintentionally) and it guarantees that even seldom-used
expected functions would be defined on the driver implementations.  Second
benefit is that these abstract methods can be called via the standard
super() call, therefore if there is some base functionality that should be
used (or legitimately you want to raise NotImplemented()), it is possible
to have that defined on the parent class.

Example abstract base class (with using six module instead of directly
setting __metaclass__):

class AbstractBaseClassTest(six.with_metaclass(abc.ABCMeta)):
    @abc.abstractmethod
    def abstract_method1(int_arg):
        # we can do something here instead of raising
        # NotImplemented()
        return (int_arg + 1)


On to the downsides, the compatibility between python2.7 and python3k
(using the six library) is not the most pleasing thing to look at.  It
requires defining the object that inherits from a method call
six.with_metaclass.  I also have not looked at the performance implications
of this, however, at first blush, it doesn't look like should be
significant.  There are possibly other pitfalls that haven't come to mind
as of yet.

In short, the drivers should probably be actual abstract classes, since
that is what they effectively are.  I've seen this functionality used in
both Neutron and Ironic.  I could see it providing some benefits from it in
Keystone.  I wanted to get the general feeling from the Mailing List on
using abstracted base classes in lieu of our current implementation prior
to proposing a Blueprint, etc.  I don't see this as a Havana implementation
but possibly something to consider for Icehouse.

I don't know if the benefits really would bring us a huge win in Keystone,
but I think it would make understanding what should be implemented when
subclassing for driver development (and similar pluggable systems) a bit
more clear.

Cheers!
--
Morgan Fainberg
Sr. Software Architect | Metacloud, Inc
Email: m at metacloud.com
IRC: morganfainberg
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20130821/e3b28986/attachment.html>