On 16.10.2012 16:27, Petr Viktorin wrote:
On 10/16/2012 04:02 PM, Jan Cholasta wrote:

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.


+    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?


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?

I see that the wording could be better, attaching new patch:
s/Process failed/Process execution failed/
s/Process successful/Process finished/


Jan Cholasta

Freeipa-devel mailing list

Reply via email to