Open Stack

Oh, Thanks Dougal, i am now clear since it is your TripleO use case.

So yes, in this case, IMHO, we should keep the _get_client() as before but
change it to the classmethod. May be other methods too like
_create_client(), etc. We can think this change to be an alternative to the
solution of creating extra Minxin classes.

Br,

Tuan

On Tue, Mar 14, 2017 at 11:38 AM, Dougal Matthews <dougal at redhat.com> wrote:

>
>
> 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.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/3f459e50/attachment.html>