On 2013-08-22 00:22, Morgan Fainberg wrote:
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.
I'm always a fan of pushing error discovery earlier (class
instantiation) rather than later (method call). So a big +1 from me.
-Ben
_______________________________________________
OpenStack-dev mailing list
[email protected]
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
