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 :)
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.
I see that the wording could be better, attaching new patch:
s/Process failed/Process execution failed/
s/Process successful/Process finished/
--
Petr³
From 465d0d6f4001c068aebb5f4fe0653654134a410e Mon Sep 17 00:00:00 2001
From: Petr Viktorin <pvikt...@redhat.com>
Date: Tue, 25 Sep 2012 09:29:49 -0400
Subject: [PATCH] ipautil.run: Log the command line before running the command
When the user interrupts a long-running command, this ensures that
the command is logged. Also, when watching log files (or the -d
output), it's apparent what's being done.
https://fedorahosted.org/freeipa/ticket/3174
---
ipapython/ipautil.py | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 0b519c2957f63770f9a28d7abe9083f724a9cf40..e6cb7772316793c97cf5b0a4f9b5bb43ce0695d0 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -292,30 +292,35 @@ def run(args, stdin=None, raiseonerr=True,
p_out = subprocess.PIPE
p_err = subprocess.PIPE
+ arg_string = nolog_replace(' '.join(args), nolog)
+ root_logger.debug('Starting external process')
+ root_logger.debug('args=%s' % arg_string)
+
try:
p = subprocess.Popen(args, stdin=p_in, stdout=p_out, stderr=p_err,
close_fds=True, env=env, cwd=cwd)
stdout,stderr = p.communicate(stdin)
stdout,stderr = str(stdout), str(stderr) # Make pylint happy
except KeyboardInterrupt:
+ root_logger.debug('Process interrupted')
p.wait()
raise
+ except:
+ root_logger.debug('Process execution failed')
+ raise
+
+ root_logger.debug('Process finished')
# The command and its output may include passwords that we don't want
# to log. Replace those.
- args = ' '.join(args)
if capture_output:
stdout = nolog_replace(stdout, nolog)
stderr = nolog_replace(stderr, nolog)
- args = nolog_replace(args, nolog)
-
- root_logger.debug('args=%s' % args)
- if capture_output:
root_logger.debug('stdout=%s' % stdout)
root_logger.debug('stderr=%s' % stderr)
if p.returncode != 0 and raiseonerr:
- raise CalledProcessError(p.returncode, args)
+ raise CalledProcessError(p.returncode, arg_string)
return (stdout, stderr, p.returncode)
--
1.7.11.7
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
