[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):

 To start GUI with the changes I had to add

 {{{
 #!python
 import grass.script.core as gcore
 gcore._RAISE_ON_COMMAND_ERROR = True
 }}}

 to `wxgui.py` which is expected but to make GUI start I had to add try-
 except to `core.find_file()`:

 {{{
 #!python
 def find_file(name, element='cell', mapset=None):
     if element == 'raster' or element == 'rast':
         verbose(_('Element type should be "cell" and not "%s"') % element)
         element = 'cell'
     try:
         s = read_command("g.findfile", flags='n', element=element,
 file=name,
                          mapset=mapset)
     except CalledCommandError:
         return {'name': ''}
     return parse_key_val(s)
 }}}

 The try-except makes sense only when `_RAISE_ON_COMMAND_ERROR = True`, so
 I would say that it should not be there. It's sure that function
 `find_file` should not cause exit or raise when the file was not found. It
 should return `False` or perhaps `None` in this case.

 It seems that the fact that you cannot rely on the interface of the
 functions hardens the implementation of new functions which are supposed
 to be general. This disqualifies the global switching from being usable.

 I think that this is different from `raise_on_error` which changes
 behavior of `core.fatal()`. GRASS modules/program calls in general behaves
 in in different ways considering error codes from error states in
 functions. They are generally less predictable, although GRASS modules in
 most of the cases behaves quite nicely and translating error code to
 exception raise makes sense.

 Considering these difficulties (g.findfile, g.message and number of
 usages) it seems to me that the functions (their default behavior) cannot
 be changed before the 7.0 release and also that global settings
 _RAISE_ON_COMMAND_ERROR and _RETURN_ON_COMMAND_ERROR should not be part of
 API and should not be used except for some testing purposes.

 Thus, I now think that the best solution is to create new functions (in
 different module but based on `start_command`) or to add a parameter to
 existing functions but something tells me that I will anyway create a
 wrapper function with the parameter set and creating new functions all
 over again was what was motivation for this ticket.

 Appendix: G7:g.findfile behavior with not existing map is to produce
 output with empty values and return code 1. `find_file` ignores return
 code and caller checks if some of the values is an empty string (or
 `None`).

 {{{
 > g.findfile -n element=cell file=aaabbbcc; echo $?
 name=
 mapset=
 fullname=
 file=
 1
 }}}

 Appendix: Usage of `raise_on_error`. The usage in GUI seems to be not well
 settled.

 {{{
 $ cd gui/wxpython
 $ grep --color=auto --exclude-dir={.svn,.git,.OBJ,locale,dist.*} -IrnE
 "raise_on"
 animation/dialogs.py:1646:    gcore.set_raise_on_error(True)
 animation/frame.py:46:gcore.set_raise_on_error(True)
 iclass/g.gui.iclass.py:120:    grass.set_raise_on_error(False)
 vdigit/g.gui.vdigit.py:84:    grass.set_raise_on_error(False)
 psmap/dialogs.py:63:# grass.set_raise_on_error(True)
 psmap/frame.py:1056:        grass.set_raise_on_error(False)
 dbmgr/g.gui.dbmgr.py:43:        grass.set_raise_on_error(False)
 }}}

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