Open Stack

On Fri, Jun 19, 2015 at 5:31 PM, Mike Perez <thingee at gmail.com> wrote:

> On Fri, Jun 19, 2015 at 3:43 PM, John Griffith <john.griffith8 at gmail.com>
> wrote:
> > Anyway, maybe I'm missing a big advantage here, but without understanding
> > what this gains it makes the code structure a bit hard to maintain and
> > follow in a number of ways.  For example, in order to implement this in
> the
> > existing drivers they need to be changed to have an inheritance structure
> > like this: [5].  As opposed to just using the decorator which everything
> > would automatically propagate to any sub-class and enforce implementation
> > with no change or maintenance to existing drivers; but it still enforces
> > implementation of the required methods.
> >
> > I'd love if folks could study some of this a bit and let me know what it
> is
> > that I'm missing here.  Is there some value that I'm unaware of that the
> > muti-inheritance paradigm in the drivers provides us?
>
> I remember from the Kilo midcycle meetup [1] that you were part of, we
> discussed this accomplishing a few things:
>

​Sure, I was there, I even proposed the use of abc as a great idea.  I
didn't catch the part about the optional classes and what that would look
like however.​


>
> * The BaseVD represents the functionality we require from all drivers.
>
​Yep
​


> * The additional ABC classes represent features that are not required yet.
>   * These are represented by classes because some features require a
> bundle of methods for it to be fulfilled. Consistency group is an
> example. [2]
>

​Sure, I suppose that's fine for things like CG and Replication.  Although
I would think that if somebody implemented something optional like CG's
that they should be able to figure out they need all of the "cg" methods,
it's kinda like that warning on ladders to not stand on the very top rung.
By the way, maybe we should discuss the state of "optional API methods" at
the mid-cycle.

​


>   * Any driver that wishes to mark their driver as supporting a
> non-required feature inherits this feature and fulfills the required
> methods.
>

​Yeah... ok​, I guess.

* After communication is done on said feature being required, there
> would be time for driver maintainers to begin supporting it.
>   * Eventually that feature would disappear from it's own class and be
> put in the BaseVD. Anybody not supporting it would have a broken
> driver, a broken CI, and eventually removed from the release.
>

​Sure, I guess my question is what's the real value in this intermediate
step.  The bulk of these are things that I'd argue shouldn't be optional
anyway (snapshots, transfers, manage, extend, retype and even migrate).
Snapshots in particular I find surprising to be considered as "optional".
​


> * Some people had thoughts of having a way generate a matrix with this
> information, which I dislike the idea of.
>

​Yeah, back to the dreaded "Matrix", I can't imagine anything worse for
Cinder than a matrix of supported API calls on a driver per driver basis.
​


>
> Why have the middle step of having the non-required features be
> represented in some abc class? I guess it's a form of reference and
> failing sooner for not fulfilling the methods with the correct
> signature. This doesn't check if those methods return the right
> information though. So maybe it's not worth it.
>

​I'm not sure that works anyway, I mean... I can still implement whatever
methods in my driver I want, if I don't inherit from the VD class it
doesn't matter, it's not checked anywhere otherwise.
​


>
> Marc provided a patch that would show RBD switching to this model.
> This revealed my main objection to the change which was the ugliness
> of having to inherit from all these classes, however, no one else
> really cared.
>

​Hmm... I wish I would've caught this at the time, because I most CERTAINLY
would have agreed with you 100% as you can probably tell by my thoughts on
the subject in reviews, IRC and this ML post.
​


>
> [1] - https://etherpad.openstack.org/p/cinder-meetup-winter-2015
> [2] -
> https://github.com/openstack/cinder/blob/master/cinder/volume/driver.py#L902
>
> --
> Mike Perez
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150619/1776d228/attachment.html>