[PATCH 1/2] Fixup kickstart script logging

Vratislav Podzimek vpodzime at redhat.com
Thu Feb 14 09:25:09 UTC 2013


On Wed, 2013-02-13 at 14:21 -0800, Brian C. Lane wrote:
> From: "Brian C. Lane" <bcl at redhat.com>
> 
> Changes to the execWith* methods removed the ability to handle a string
> as stdin/out/err. Change the callers to first open where they want
> output to be sent to and pass that instead.
> Only stdout is needed because _run_program combines stdout and stderr,
> pass stderr through but ignore it.
> 
> Also clean up a couple places that were passing strings that don't need
> to.
> ---
>  anaconda                |  3 +--
>  pyanaconda/iutil.py     | 61 ++++++++++++++++++++++++++++++-------------------
>  pyanaconda/kickstart.py | 35 +++++++++++++---------------
>  pyanaconda/network.py   |  3 +--
>  4 files changed, 55 insertions(+), 47 deletions(-)
> 
> diff --git a/anaconda b/anaconda
> index 324d31e..06773ca 100755
> --- a/anaconda
> +++ b/anaconda
> @@ -85,8 +85,7 @@ def startMetacityWM():
>          try:
>              args = ['--display', ':1',
>                      '--sm-disable']
> -            iutil.execWithRedirect('metacity', args,
> -                                   stdout='/dev/null', stderr='/dev/null')
> +            iutil.execWithRedirect('metacity', args)
>          except BaseException as e:
>              # catch all possible exceptions
>              log.error("Problems running the window manager: %s" % str(e))
> diff --git a/pyanaconda/iutil.py b/pyanaconda/iutil.py
> index dbc7c3b..2d2b08c 100644
> --- a/pyanaconda/iutil.py
> +++ b/pyanaconda/iutil.py
> @@ -49,7 +49,15 @@ def augmentEnv():
>                 })
>      return env
>  
> -def _run_program(argv, root='/', stdin=None, env_prune=None):
> +def _run_program(argv, root='/', stdin=None, stdout=None, env_prune=None):
> +    """ Run an external program, log the output and return it to the caller
> +        @param argv The command to run and argument
> +        @param root The directory to chroot to before running command.
> +        @param stdin The file descriptor to read stdin from.
> +        @param stdout Optional file descriptor to write stdout and stderr to.
These should be both 'file object' instead of 'file descriptor'. The
same goes for the other doc strings as well.

Otherwise these patches both look good to me. My only concern is about
using proc.communicate() instead of the 'tee' class. It is a story much
older than my history in the team but I'm quite sure there was a reason
for writing and using that class. Do we support e.g. interactive
scripts? Or don't we want output of the program logged even before the
program exits so that it can be easily seen, that it e.g. expects input?
Are we going to use _run_program for something with really long output?
Because in all those cases proc.communicate() is not a good idea.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list