[Freeipa-devel] [python-pytest-multihost]-Ticket-6 run_command produces exit code 1 on windows

[Petr Viktorin]

On 04/21/2016 07:25 AM, Niranjan wrote:
> Petr Viktorin wrote:
>> On 04/20/2016 06:11 AM, Niranjan wrote:
>>> Petr Viktorin wrote:
>>>> On 04/18/2016 12:39 PM, Niranjan wrote:
>>>>> Niranjan wrote:
>>>>>> Niranjan wrote:
>>>>>>> Petr Viktorin wrote:
>>>>>>>> On 04/06/2016 10:55 AM, Niranjan wrote:
>>>>>>>>> Greetings,
>>>>>>>>>
>>>>>>>>> For https://fedorahosted.org/python-pytest-multihost/ticket/6 , i have proposed
>>>>>>>>> a patch, I think this patch is more of a workaround , than a solution. I would
>>>>>>>>> like to get more inputs on how to use pytest-multihost to execute commands 
>>>>>>>>> on Windows. The method i am using requires cygwin with openssh/bash to be
>>>>>>>>> installed. 
>>>>>>>>
>>>>>>>> Wow. I'm surprised the only problem on Windows is the "set -e".
>>>>>>>>
>>>>>>>> Regarding the patch:
>>>>>>>>
>>>>>>>> - Why is get_winhost_class needed? I don't see it called anywhere.
>>>>>>>> - Similarly, why is host_type needed? Your patch only sets it.
>>>>>>>>
>>>>>>>> If run_command of WinHost and Unix hosts are this similar, I would put
>>>>>>>> the 'set -e\n' for Unix (and an empty string for Windows) in a class
>>>>>>>> attribute named e.g. "command_prelude", use it in run_command, and move
>>>>>>>> run_command from Host to BaseHost.
>>>>>>>> Would that work, or am I missing something?
>>>>>>> How do we detect the host is windows/unix then , we still need host_type 
>>>>>>> to know what the type of host is (unix/windows)?
>>>>>>>>
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Petr Viktorin
>>>>>> Please review the attached patch.
>>>>
>>>> Sorry for the delay.
>>>>
>>>> The information about whether the host is Unix or Windows is available:
>>>> the class is either Host or WinHost, so I don't think an extra host_type
>>>> is necessary.
>>>> I'd be open to adding host_type as *configuration* (and corresponding
>>>> attribute) if it also selected the actual Host class. Currently to use
>>>> WinHost you need to write custom code.
>>> I would like to have host_type available. We would like to add more
>>> functions in classes that override Host/WinHost class , which 
>>> can be then called depending upon the host_type. 
>>
>> Ah, I see; you're not using the WinHost subclass right now.
>>
>> I added a host_type; it is used to select the Host class.
>> You can define a "host_classes" dict in your Domain class to list Host
>> classes, and the particular Host class will be selected according to the
>> host_type.
>>
>> The usage would be like this:
>>
>> ---------
>>
>> class QeHost(QeBaseHost):
>>    """Linux host class"""
>>     @classmethod
>>     def gethostname(self):
>>         """ Return system hostname """
>>         cmd = self.run_command(['hostname'], raiseonerr=False)
>>         return cmd.stdout_text.strip()
>>    ...
>>
>> class QeWinHost(QeBaseHost, pytest_multihost.host.WinHost):
>>    """Windows host class"""
>>
>>    @classmethod
>>    def gethostname(self):
>>        """ Return system hostname """
>>        cmd = self.run_command(['hostname', '-A'], set_env=False,
>> raiseonerr=False)
>>        return cmd.stdout_text.strip()
>>    ...
>>
>>
>> class QeDomain:
>>    ...
>>    host_classes = {'default': QeHost, 'windows': QeWinHost}
>>
>>    # remove your existing "get_host_class" method when host_classes is
>> defined
>>    ...
>>
>> ---------
>>
>> And in the config, define host_type to 'windows'.
>>
>>
>> Would it work for you like this?
> Most of the things worked [...]
> 
> Specifically i see that you have "transport_class = transport.SSHTransport" added to Host Class
> but not in WinHost class. So initially it failed transport class not available,  but after i added below lines
> it worked:
> 
> class QeBaseHost(pytest_multihost.host.BaseHost):
>     """ QeBaseHost subclass of multhost plugin BaseHost class """
>     transport_class = transport.SSHTransport
> 
> Apart from the above minor thing, Everything else works. Again the above 3 lines solves the minor issue, 
> just wanted to know if the above lines are to be added and is by design. 

Right, since SSHTransport works for Windows hosts as well, I've added it
to BaseHost. With this version of patch it shouldn't be necessary to add
it in your code.

If this works for you, I'll commit it and release.


-- 
Petr Viktorin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-host_type-make-it-possible-to-use-Windows-hosts.patch
Type: text/x-patch
Size: 8135 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160421/85807aa5/attachment.bin>