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

[Rob Crittenden]

Jan Cholasta wrote:
> 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
>

pushed to ipa-3-0 and master

rob