On 14 March 2017 at 05:25, Renat Akhmerov <renat.akhme...@gmail.com> wrote:
> So again, I’m for simplicity but that kind of simplicity that also allows > flexibility in the future. > > There’s one principle that I usually follow in programming that says: > > “*Space around code (absence of code) has more potential than the code > itself.*” > > That means that it’s better to get rid of any stuff that’s not currently > needed and add things > as requirements change. However, that doesn’t always work well in > framework development > because the cost of initial inflexibility may become too high in future, > that comes from the > need to stay backwards compatible. What I’m trying to say is that IMO it’s > ok just to keep > it as simple as just a base class with method run() for now and think how > we can add more > things in the future, if we need to, using mixin approach. So seems like > it’s going to be: > > class Action(object): > > def run(ctx): > … > > > class Mixin1(object): > > def method11(): > … > > def method12(): > … > > > class Mixin2(object): > > def method21(): > … > > def method22(): > … > > > Then my concrete action could use a combination of Action and any of the > mixin: > > class MyAction(Action, Mixin1): > … > > > class MyAction(Action, Mixin2): > … > > or just > > > class MyAction(Action): > … > > Is this flexible enough or does it have any potential issues? > Sure, that is fine and it works but I think this is almost exactly what rbrady has proposed earlier in this thread. I think the outstanding questions are; - should the context be a mixin or should run() always accept context? - should async be a mixin or should we continue with the is_sync() method and overwriting that in the sublcass? - should the openstack actions in mistral-extra be mixins? IMO, base class is still needed to define the contract that all actions > should follow. So that > a runner knew what’s possible to do with actions. > I agree but I don't think anyone is suggesting we don't have a base class. > > Renat Akhmerov > @Nokia > > On 13 Mar 2017, at 16:49, lương hữu tuấn <tuantulu...@gmail.com> wrote: > > > > On Mon, Mar 13, 2017 at 9:34 AM, Thomas Herve <the...@redhat.com> wrote: > >> On Fri, Mar 10, 2017 at 9:52 PM, Ryan Brady <rbr...@redhat.com> wrote: >> > >> > One of the pain points for me as an action developer is the OpenStack >> > actions[1]. Since they all use the same method name to retrieve the >> > underlying client, you cannot simply inherit from more than one so you >> are >> > forced to rewrite the client access methods. We saw this in creating >> > actions for TripleO[2]. In the base action in TripleO, we have actions >> that >> > make calls to more than one OpenStack client and so we end up >> re-writing and >> > maintaining code. IMO the idea of using multiple inheritance there >> would be >> > helpful. It may not require the mixin approach here, but rather a >> design >> > change in the generator to ensure the method names don't match. >> >> Is there any reason why those methods aren't functions? AFAICT they >> don't use the instance, they could live top level in the action module >> and be accessible by all actions. If you can avoid multiple >> inheritance (or inheritance!) you'll simplify the design. You could >> also do client = NovaAction().get_client() in your own action (if >> get_client was a public method). >> >> -- >> Thomas >> >> If you want to do that, you need to change the whole structure of base > action and the whole way of creating an action > as you have described and IMHO, i myself do not like this idea: > > 1. Mistral is working well (at the standpoint of creating action) and > changing it is not a short term work. > 2. Using base class to create base action is actually a good idea in order > to control and make easy to action developers. > The base class will define the whole mechanism to execute an action, > developers do not need to take care of it, just only > providing OpenStack clients (the _create_client() method). > 3. From the #2 point of view, the alternative to NovaAction().get_client() > does not make sense since the problem here is subclass mechanism, > not the way to call get_client(). > > @Renat: I myself not against to multiple inheritance too, the only thing > is if we want to make it multiple inheritance, we should think about it > more thoroughly for the hierarchy of inheritance, what each inheritance > layer does, etc. These work will make the multiple inheritance easy to > understand and for action developers as well easy to develop. So, IMHO, i > vote for make it simple, easy to understand first (if you continue with > mistral-lib) and then do the next thing later. > > Br, > > Tuan/Nokia > >> ____________________________________________________________ >> ______________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscrib >> e <http://openstack-dev-requ...@lists.openstack.org/?subject:unsubscribe> >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
