Hi, I did not know about this change before but after reading the whole story, IMHO i myself love the way of keeping it as simple at this moment as you guys i think agreed on. For MixinAction or PolicyMixin, etc. we can develop them later when we have concrete case studies.
Br, Tuan/Nokia On Fri, Mar 10, 2017 at 11:16 AM, Renat Akhmerov <[email protected]> wrote: > > On 10 Mar 2017, at 15:09, Dougal Matthews <[email protected]> wrote: > > On 10 March 2017 at 04:22, Renat Akhmerov <[email protected]> > wrote: > >> Hi, >> >> I probably like the base class approach better too. >> >> However, I’m trying to understand if we need this variety of classes. >> >> - Do we need a separate class for asynchronous actions? IMO, since >> is_sync() is just an instance method that can potentially return both True >> and False based on the instance state shouldn’t be introduced by a special >> class. Otherwise it’s confusing that a classes declared as AsyncAction can >> actually be synchronous (if its is_sync() returns True). So maybe we >> should >> just leave this method in the base class. >> - I”m also wondering if we should just always pass “context” into >> run() method. Those action implementations that don’t need it could just >> ignore it. Not sure though. >> >> This is a good point. I had originally thought it would be backwards > incompatible to make this change - however, users will need to update their > actions to inherit from mistral-lib so they will need to opt in. Then in > mistral we can do something like... > > if isinstance(action, mistral_lib.Action): > action.run(ctx) > else: > # deprecation warning about action now inheriting from mistral_lib and > taking a context etc. > action.run() > >> > Yes, right. > > As far as mixin approach, I’d say I’d be ok with having mixing for >> context-based actions. Although, like Dougal said, it may be a little >> harder to read, this approach gives a huge flexibility for long term. >> Imagine if we want to have a class of actions that some different kind of >> information. Just making it up… For example, some of my actions need to be >> aware of some policies (Congress-like) or information about metrics of the >> current operating system (this is probably a bad example because it’s easy >> to use standard Python modules but I’m just trying to illustrate the idea). >> In this case we could have PolicyMixin and OperatingSystemMixin that would >> set required info into the instance state or provide with handle interfaces >> for more advanced uses. >> > > I like the idea of mixins if we can see a future with many small > components that can be included in an action class. However, like you I > didn't manage to think of any real examples. > > It should be possible to migrate to a mixin approach later if we have the > need. > > > Well, I didn’t manage to find real use cases probably because I don’t > develop lots of actions :) Although the example with policies seems almost > real to me. This is something that was raised several times during design > sessions in the past. Anyway, I agree with you that seems like we can add > mixins later if we want to. I don’t see any reasons now why not. > > > Renat Akhmerov > @Nokia > > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > >
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
