#2326: Command functions in grass.script.core miss a correct error reporting
--------------------------------+-------------------------------------------
Reporter: wenzeslaus | Owner: grass-dev@…
Type: enhancement | Status: new
Priority: major | Milestone: 7.1.0
Component: Python | Version: svn-trunk
Keywords: script, exceptions | Platform: All
Cpu: Unspecified |
--------------------------------+-------------------------------------------
Comment(by glynn):
Replying to [comment:6 wenzeslaus]:
> Here are my suggestions for changes in `grass.script.core`.
These changes are excessive. All that is required is to check the exit
code and generate an error if it is non-zero. If the child process returns
a zero exit code, the existing behaviour should not be affected.
In particular, stderr should not be captured. It isn't limited to errors
(e.g. messages and percentages are written to stderr), and such
information should normally be sent to the terminal as its generated.
Also, checking kwargs!["args"] isn't useful.
In most cases, the failure to check exit codes was inherited from the
original shell script. run_command() replaces "simple" command execution,
read_command() replaces backticks. pipe_command() and feed_command() are
used for a GRASS command at one end of a pipeline. write_command()
replaces "echo data | command".
My suggestion would be that the functions which wait for the process to
terminate (run_command, read_command, write_command) should call a
check_status() function, e.g.
{{{
def check_status(p, args, kwargs):
if p.wait() == 0:
return 0
raise CalledCommandError(p.returncode, args, kwargs)
}}}
run_command() and write_command() would replace
{{{
return p.wait()
}}}
with
{{{
return check_status(p)
}}}
This usage pattern would allow check_status() to be modified to provide
more options regarding error handling, e.g. raise an exception, call
fatal(), or just return the exit code, with the behaviour controlled by a
variable or a keyword argument.
Alternatively, we could just modify the Popen wrapper to provide a
modified .wait() method which does this automatically. This would probably
"do the right thing" for scripts which use start_command() etc and
subsequently call p.wait() themselves.
--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2326#comment:8>
GRASS GIS <http://grass.osgeo.org>
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
