[Pki-devel] [PATCH] TRAC Ticket #561 - Replace subprocess.call() with subprocess.check_call()

[Endi Sukma Dewata]

On 8/20/2013 7:05 PM, Matthew Harmsen wrote:
>> 1. Build failed due to pylint errors:
>>
>> E0602:2664,25:KRAConnector.execute_using_pki: Undefined variable 'https'
>> E0602:2938,25:SecurityDomain.get_installation_token: Undefined
>> variable 'https'
>>
>> It looks like the 'https' should have been quoted.
>>
>> 2. I tried running the following test code:
>>
>>   import subprocess
>>   command = ["/bin/echo", "test", ">", "/dev/null", "2>&1"]
>>   subprocess.check_call(command, stdout=None)
>>
>> The ">" redirection doesn't seem to work, it's actually printed to the
>> screen. To get it to work correctly I had to specify shell=True.
>> Alternatively, it's also possible to open /dev/null file and pass the
>> file handler:
>>
>>   with open(os.devnull, "w") as fnull:
>>     subprocess.check_call(command, stdout=fnull, stderr=fnull)
>>
>> There are some instances in the code where the ">" redirection is used
>> without shell=True.
>>
> I have attached a new patch which addresses (1) and (2) above; please
> review.
>
> -- Matt

Another thing, there are instances where the stderr is redirected to 
stdout using "2>&1", but the command already has 
stderr=subprocess.STDOUT, so the "2>&1" and shell=True can be removed 
from these commands. This happens in pkihelper.py:2713,2789,2904.

The only invocations that need shell=True are probably 
pkiparser.py:162,169 because they are processing shell variables.

Other than that everything works fine. ACK.

-- 
Endi S. Dewata