On 10 March 2017 at 04:22, Renat Akhmerov <renat.akhme...@gmail.com> 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() So just having one class would really be the simplest approach. 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. What do you think? > > Renat Akhmerov > @Nokia > > On 9 Mar 2017, at 11:35, Ryan Brady <rbr...@redhat.com> wrote: > > At the PTG and previous discussions in IRC, I mentioned there were two > different design ideas I had for the developer experience for custom action > development in mistral-lib. The purpose and intent behind the patch[1] was > discussed in person at the PTG and that was helpful for me wrt to scope. I > feel it would be helpful to discuss and decide together the final piece of > this patch. I'd like to get any feedback on either of these two ideas as > they will shape how developers integrate with Mistral in the future, impact > our OpenStack integration efforts in mistral-extra. Nothing stops a > developer from adopting either style in their custom action libraries, but > most will likely want to remain consistent with style present in the > upstream code. > > I have created separate declaration and usage examples in hopes of > illustrating some of the similarities and differences. To me it seems the > base class example is more declarative/explicit, but the mixin example is > more extensible and dry. Both examples reflect on backwards compatibility > and possible changes to how mistral checks for sync/async actions and how > to pass the context (as needed by actions that integrate with OpenStack). > > > base classes declaration: https://gist.github.com/rbrady/ > ff86c484e8e6e53ba2dc3dfa17b01b09 > > base class usage: https://gist.github.com/rbrady/ > 716a02fb2bd38d822c6df8bd642d3ea6 > > mixins declaration: https://gist.github.com/rbrady/ > d30ae640b19df658a17cd93827125678 > > mixins usage: https://gist.github.com/rbrady/ > 248cb52d5c5f94854d8c76eee911ce8e > > > Thanks, > > Ryan > > -- > Ryan Brady > Cloud Engineering > rbr...@redhat.com > 919.890.8925 <(919)%20890-8925> > > > [1] https://review.openstack.org/#/c/411412/ > __________________________________________________________________________ > 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