<div dir="ltr">Oh, Thanks Dougal, i am now clear since it is your TripleO use case. <br><br>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><br>Br,<br><br>Tuan</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 14, 2017 at 11:38 AM, Dougal Matthews <span dir="ltr"><<a href="mailto:dougal@redhat.com" target="_blank">dougal@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On 14 March 2017 at 10:21, lương hữu tuấn <span dir="ltr"><<a href="mailto:tuantuluong@gmail.com" target="_blank">tuantuluong@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-3521424200941579662gmail-h5">On Tue, Mar 14, 2017 at 10:28 AM, Dougal Matthews <span dir="ltr"><<a href="mailto:dougal@redhat.com" target="_blank">dougal@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="m_-3521424200941579662gmail-m_4158665181622021946h5">On 13 March 2017 at 09:49, lương hữu tuấn <span dir="ltr"><<a href="mailto:tuantuluong@gmail.com" target="_blank">tuantuluong@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span>On Mon, Mar 13, 2017 at 9:34 AM, Thomas Herve <span dir="ltr"><<a href="mailto:therve@redhat.com" target="_blank">therve@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span>On Fri, Mar 10, 2017 at 9:52 PM, Ryan Brady <<a href="mailto:rbrady@redhat.com" target="_blank">rbrady@redhat.com</a>> wrote:<br>
><br>
> One of the pain points for me as an action developer is the OpenStack<br>
> actions[1]. Since they all use the same method name to retrieve the<br>
> underlying client, you cannot simply inherit from more than one so you are<br>
> forced to rewrite the client access methods. We saw this in creating<br>
> actions for TripleO[2]. In the base action in TripleO, we have actions that<br>
> make calls to more than one OpenStack client and so we end up re-writing and<br>
> maintaining code. IMO the idea of using multiple inheritance there would be<br>
> helpful. It may not require the mixin approach here, but rather a design<br>
> change in the generator to ensure the method names don't match.<br>
<br>
</span>Is there any reason why those methods aren't functions? AFAICT they<br>
don't use the instance, they could live top level in the action module<br>
and be accessible by all actions. If you can avoid multiple<br>
inheritance (or inheritance!) you'll simplify the design. You could<br>
also do client = NovaAction().get_client() in your own action (if<br>
get_client was a public method).<br>
<span class="m_-3521424200941579662gmail-m_4158665181622021946m_908899259678575068m_-6409316695677436032HOEnZb"><font color="#888888"><br>
--<br>
Thomas<br>
</font></span><div class="m_-3521424200941579662gmail-m_4158665181622021946m_908899259678575068m_-6409316695677436032HOEnZb"><div class="m_-3521424200941579662gmail-m_4158665181622021946m_908899259678575068m_-6409316695677436032h5"><br></div></div></blockquote></span><div>If you want to do that, you need to change the whole structure of base action and the whole way of creating an action</div><div>as you have described and IMHO, i myself do not like this idea:<br><br>1. Mistral is working well (at the standpoint of creating action) and changing it is not a short term work.</div><div>2. Using base class to create base action is actually a good idea in order to control and make easy to action developers. </div><div>The base class will define the whole mechanism to execute an action, developers do not need to take care of it, just only</div><div>providing OpenStack clients (the _create_client() method).</div><div>3. From the #2 point of view, the alternative to NovaAction().get_client() does not make sense since the problem here is subclass mechanism,</div><div>not the way to call get_client().<br></div></div></div></div></blockquote></div></div></div></div></div></blockquote><div><br></div></div></div><div><span style="background-color:rgb(255,255,255)"><font color="#ff0000">Hi,<br><br>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.</font></span><br><br></div><span class="m_-3521424200941579662gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_-3521424200941579662gmail-m_4158665181622021946h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div></div></div></div></blockquote><div><br></div></div></div><div>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.<br><br></div></div></div></div></blockquote></span><div><font color="#ff0000">Sorry Dougal but i do not get your point. Why the get_client could not be used through instance since it has context?</font></div></div></div></div></blockquote><div><br></div></div></div><div>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:<br><br></div><div>class MyNovaAction(NovaAction):<br><br></div><div> def run(self):<br><br></div><div> client = self._create_client()<br></div><div> # ... do something with the client and return<br><br></div><div>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).<br><br></div><div>class MyNovaAndGlanceActioin(<wbr>NovaAction, GlanceAction):<br><br></div><div> def run(self):<br><br></div><div> nova = self._create_client()<br></div><div> glance = self._create_client() <- doesn't work because they both access the same method on NovaAction.<br><br><br>If the method was called "create_nova_client" and "create_glance_client" then you could inherit from both without any conflict.<br><br></div><div>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.<br><br> nova = NovaAction.client(context)</div><div><br></div><div>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.<br></div><div><br></div><div><br>[1]: <a href="https://github.com/openstack/mistral/blob/master/mistral/actions/openstack/actions.py#L75" target="_blank">https://github.com/openstack/<wbr>mistral/blob/master/mistral/<wbr>actions/openstack/actions.py#<wbr>L75</a><br>[2]: <a href="https://github.com/openstack/mistral/blob/master/mistral/actions/openstack/actions.py#L109" target="_blank">https://github.com/openstack/<wbr>mistral/blob/master/mistral/<wbr>actions/openstack/actions.py#<wbr>L109</a><br></div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br> </div><span class="m_-3521424200941579662gmail-"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div>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.<br></div><span><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>@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></div><div><br></div><div>Br,</div><div><br></div><div>Tuan/Nokia</div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="m_-3521424200941579662gmail-m_4158665181622021946m_908899259678575068m_-6409316695677436032HOEnZb"><div class="m_-3521424200941579662gmail-m_4158665181622021946m_908899259678575068m_-6409316695677436032h5">
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div></div></blockquote></span></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
<br></blockquote></span></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
<br></blockquote></span></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
<br></blockquote></span></div><br></div></div>
<br>______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.<wbr>openstack.org?subject:<wbr>unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/<wbr>cgi-bin/mailman/listinfo/<wbr>openstack-dev</a><br>
<br></blockquote></div><br></div>