Open Stack

On 14 March 2017 at 10:21, lương hữu tuấn <tuantuluong at gmail.com> wrote:

>
>
> On Tue, Mar 14, 2017 at 10:28 AM, Dougal Matthews <dougal at redhat.com>
> wrote:
>
>>
>>
>> On 13 March 2017 at 09: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().
>>>
>>
> Hi,
>
> It is hard to me to understand what Thomas wants to say but i just
> understood based on what he wrote:). Sorry for my misunderstanding.
>
>
>> I might be wrong, but I think you read that Thomas wants to use functions
>> for actions, not classes. I don't think that is the case. I think he is
>> referring to the get_client method which is also what rbrady is referring
>> to. At the moment multiple inheritance wont work if you want to inherit
>> from NovaAction and KeyStone action because they both provide a
>> "_get_client" method. If they has a unique name "get_keystone_client" and
>> "get_nova_client" then the multiple inheritance wouldn't clash.
>>
>> Sorry Dougal but i do not get your point. Why the get_client could not be
> used through instance since it has context?
>

In Mistral we have various OpenStack action classes. For example
NovaAction[1] and GlanceAction[2] (and many others in that file). If I want
to write an action that uses either Nova or Glance I can inherit from them,
for example:

class MyNovaAction(NovaAction):

    def run(self):

        client = self._create_client()
        # ... do something with the client and return

However, if I wanted to use use two OpenStack clients, which I admit is a
special case and I think one that only TripleO uses (that we know of).

class MyNovaAndGlanceActioin(NovaAction, GlanceAction):

    def run(self):

        nova = self._create_client()
        glance = self._create_client() <- doesn't work because they both
access the same method on NovaAction.


If the method was called "create_nova_client" and "create_glance_client"
then you could inherit from both without any conflict.

However, based on the reply Thomas sent earlier, I think we should consider
something like this when the OpenStack actions are moved to mistral-extra.

    nova = NovaAction.client(context)

This is slight adaptation changes "_create_client" to "client" and makes it
a class method that accepts the context. I think this would provide a very
clear interface. I also can't think of any advantage of inheriting from
NovaAction, there is no state shared with it, so we only want it to create
the class for us.


[1]:
https://github.com/openstack/mistral/blob/master/mistral/actions/openstack/actions.py#L75
[2]:
https://github.com/openstack/mistral/blob/master/mistral/actions/openstack/actions.py#L109


>
>
>
>> Thomas - The difficulty with these methods is that they need to access
>> the context - the context is going to be added to the action class, and
>> thus while the get_client methods don't use the instance now, they will
>> soon - unless we change direction.
>>
>>
>>
>>> @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.op
>>>> enstack.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.op
>>> enstack.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:unsubscrib
>> e
>> 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/e11b36d5/attachment.html>