[Freeipa-devel] [PATCH] 0091 ipautil.run: Log the command line before running the command

[Jan Cholasta]

On 16.10.2012 17:26, Petr Viktorin wrote:
> On 10/16/2012 04:53 PM, Jan Cholasta wrote:
>> On 16.10.2012 16:27, Petr Viktorin wrote:
>>> On 10/16/2012 04:02 PM, Jan Cholasta wrote:
>>>> Hi,
>>>>
>>>> On 15.10.2012 14:45, Petr Viktorin wrote:
>>>>> As I was debugging code that calls long-running or failing commands, I
>>>>> got tired of the invocation being logged after the command is done.
>>>>> This patch should improve the logging.
>>>>>
>>>>> https://fedorahosted.org/freeipa/ticket/3174
>>>>> ---
>>>>> Petr³
>>>>>
>>>>
>>>> +    except:
>>>> +        root_logger.debug('Process failed')
>>>> +        raise
>>>>
>>>> Why do you use blank except here? Can you print the reason why the
>>>> process has failed or the exit code here?
>>>>
>>>> Honza
>>>>
>>>
>>> I re-raise command afterwards, so the error is not really handled or
>>> ignored. It's one of the few situations where a bare except is
>>> justified :)
>>
>> OK, makes sense.
>>
>>> Since the error is re-raised, it should be handled by the caller.
>>> Printing additional info here should be redundant.
>>> The exception can happen before the Popen object is created, so the
>>> return code might not be available. Anyway bad return code is handled
>>> later, here we'd get errors like command not found, fork() failure etc.
>>
>> Right. Anyway, I think logging the return code might come handy in some
>> situations. Can you please add it?
>>
>
> You're right. Added.
>
>>>
>>> I see that the wording could be better, attaching new patch:
>>> s/Process failed/Process execution failed/
>>> s/Process successful/Process finished/
>>>
>>
>> Honza
>>
>
>

Thanks. ACK.

Honza

-- 
Jan Cholasta