[openstack-dev] [mistral] Mistral Custom Actions API Design

Dougal Matthews dougal at redhat.com
Tue Mar 14 09:34:57 UTC 2017


On 14 March 2017 at 05:25, Renat Akhmerov <renat.akhmerov at 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 <tuantuluong at gmail.com> wrote:
>
>
>
> On Mon, Mar 13, 2017 at 9:34 AM, Thomas Herve <therve at redhat.com> wrote:
>
>> On Fri, Mar 10, 2017 at 9:52 PM, Ryan Brady <rbrady at 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-request at lists.openstack.org?subject:unsubscrib
>> e <http://OpenStack-dev-request@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
>
>
>
> __________________________________________________________________________
> 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/20170314/ed022a56/attachment.html>


More information about the OpenStack-dev mailing list