[GRASS-dev] [GRASS GIS] #2326: Command functions in grass.script.core miss a correct error reporting

[GRASS GIS]

#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 wenzeslaus):

 I've added new patch. Functions call one of the following error handing
 functions:

 {{{
 #!python
 def _called_command_error(returncode, args, kwargs):
     msg = _("Command %s %s returned non-zero exit status %s") % (args,
 kwargs, returncode)
     if _RAISE_ON_COMMAND_ERROR:
         raise CalledCommandError(msg, returncode, args, kwargs)
     fatal(msg)  # this exits or raises


 def _called_command_return(returncode, args, kwargs):
     if _RETURN_ON_COMMAND_ERROR:
         return returncode
     elif returncode:
         _called_command_error(returncode, args, kwargs)
 }}}

 Examples of functions:

 {{{
 #!python
 # we can return return code, raise or exit
 def run_command(*args, **kwargs):
     ps = start_command(*args, **kwargs)
     returncode = ps.wait()
     return _called_command_return(returncode, args, kwargs)
 }}}

 {{{
 #!python
 # we want to always return output here, so raise or exit
 def read_command(*args, **kwargs):
     process = pipe_command(*args, **kwargs)
     output = process.communicate()[0]
     returncode = process.poll()
     if returncode:
         _called_command_error(returncode, args, kwargs)
     return output
 }}}

  * I don't call `wait()` or `poll()` in the checking method, this is done
 in the functions in different (appropriate) ways.
  * There is no interface yet for the global variables which controls if
 the functions will raise, exit/fatal or return (if applicable).
  * I'm using `message(msg, flag='w')` instead of `message(msg, flag='w')`
 to avoid issue described in comment:10.
  * Besides global variables there may or probably should be also a
 parameter which will apply to individual functions. But I still have a
 feeling that I would like to use special function, no a function with
 parameter.
  * Functions `read_command` and `write_read_command` capture stdout, so I
 don't see a reason to not capture also stderr and print it (perhaps
 begging and end) in the error message, except for inconsistency with other
 functions. Again I still have a feeling that I want this also for other
 functions.
  * I was looking to [source:grass/trunk/gui/wxpython/core/gcmd.py#L553
 gui/wxpython/core/gcmd.py] and I'm wondering why there is so much code
 around `subprocess.Popen` call and why it is not needed in `script.core`.
  * I'm still thinking how to include this changes into trunk and 7_0. It
 is a change of API and it requires to go to almost 200 places in trunk and
 addons (see comment:5).

 Download the test, e.g. to `/lib/python/script/testsuite` directory, and
 run it (inside GRASS session):
 {{{
 python -m unittest discover lib/python/script/testsuite
 }}}

-- 
Ticket URL: <http://trac.osgeo.org/grass/ticket/2326#comment:11>
GRASS GIS <http://grass.osgeo.org>