<div dir="ltr"><div class="gmail_default" style="font-family:monospace,monospace"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 19, 2015 at 5:31 PM, Mike Perez <span dir="ltr"><<a href="mailto:thingee@gmail.com" target="_blank">thingee@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Fri, Jun 19, 2015 at 3:43 PM, John Griffith <<a href="mailto:john.griffith8@gmail.com">john.griffith8@gmail.com</a>> wrote:<br>
> Anyway, maybe I'm missing a big advantage here, but without understanding<br>
> what this gains it makes the code structure a bit hard to maintain and<br>
> follow in a number of ways. For example, in order to implement this in the<br>
> existing drivers they need to be changed to have an inheritance structure<br>
> like this: [5]. As opposed to just using the decorator which everything<br>
> would automatically propagate to any sub-class and enforce implementation<br>
> with no change or maintenance to existing drivers; but it still enforces<br>
> implementation of the required methods.<br>
><br>
> I'd love if folks could study some of this a bit and let me know what it is<br>
> that I'm missing here. Is there some value that I'm unaware of that the<br>
> muti-inheritance paradigm in the drivers provides us?<br>
<br>
</span>I remember from the Kilo midcycle meetup [1] that you were part of, we<br>
discussed this accomplishing a few things:<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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.</div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
* The BaseVD represents the functionality we require from all drivers.<br></blockquote><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Yep</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* The additional ABC classes represent features that are not required yet.<br>
* These are represented by classes because some features require a<br>
bundle of methods for it to be fulfilled. Consistency group is an<br>
example. [2]<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"><br></div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* Any driver that wishes to mark their driver as supporting a<br>
non-required feature inherits this feature and fulfills the required<br>
methods.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">Yeah... ok, I guess.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"><br></div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* After communication is done on said feature being required, there<br>
would be time for driver maintainers to begin supporting it.<br>
* Eventually that feature would disappear from it's own class and be<br>
put in the BaseVD. Anybody not supporting it would have a broken<br>
driver, a broken CI, and eventually removed from the release.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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".</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* Some people had thoughts of having a way generate a matrix with this<br>
information, which I dislike the idea of.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Why have the middle step of having the non-required features be<br>
represented in some abc class? I guess it's a form of reference and<br>
failing sooner for not fulfilling the methods with the correct<br>
signature. This doesn't check if those methods return the right<br>
information though. So maybe it's not worth it.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Marc provided a patch that would show RBD switching to this model.<br>
This revealed my main objection to the change which was the ugliness<br>
of having to inherit from all these classes, however, no one else<br>
really cared.<br></blockquote><div><br></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline">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.</div></div><div><div class="gmail_default" style="font-family:monospace,monospace;display:inline"></div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
[1] - <a href="https://etherpad.openstack.org/p/cinder-meetup-winter-2015" rel="noreferrer" target="_blank">https://etherpad.openstack.org/p/cinder-meetup-winter-2015</a><br>
[2] - <a href="https://github.com/openstack/cinder/blob/master/cinder/volume/driver.py#L902" rel="noreferrer" target="_blank">https://github.com/openstack/cinder/blob/master/cinder/volume/driver.py#L902</a><br>
<br>
--<br>
Mike Perez<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</blockquote></div><br></div></div>