Open Stack

On 10 March 2017 at 04:22, Renat Akhmerov <renat.akhmerov at 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 <rbrady at 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
> rbrady at 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-request at 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-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/20170310/64e7d162/attachment.html>