[GRASS-dev] Re: [GRASS GIS] #7: Location wizard: should predefine DB connection for new location

[GRASS GIS]

#7: Location wizard: should predefine DB connection for new location
----------------------+-----------------------------------------------------
  Reporter:  neteler  |       Owner:  hamish   
      Type:  defect   |      Status:  assigned 
  Priority:  major    |   Milestone:  6.4.0    
 Component:  default  |     Version:  svn-trunk
Resolution:           |    Keywords:           
----------------------+-----------------------------------------------------
Comment (by hamish):

 Michael:
 > If we want this to be easily changeable in the future, it should be
 > dealt with in a central place
 [...]
 > so that changing it once will switch from a dbf to sqlite default.

 yes, now set by line 9 of include/dbmi.h:
 http://trac.osgeo.org/grass/browser/grass/trunk/include/dbmi.h?rev=30545


 > This has come up periodically over the past year, as lack of a VAR
 > file seems to cause different scattered modules to fail, and is
 > usually puzzling to users as to what is going wrong.

 Name something that still fails and I'll fix it. I don't think there are
 any, but maybe there still is one- I fully accept the need for an audit.

 The main worry I had was for 'v.in.ogr location=' where it will attempt to
 populate a DB table before ever opening the mapset (and so the init.sh
 'db.connect -c' quote-unquote "solution" hasn't had the chance to run).
 But I've just tested that and it correctly creates the VAR file on the fly
 using db_set_default_connection().


 There really shouldn't be a problem with C modules as the vector and db
 libs already had code to automatically create the VAR file as needed
 through the Vect_default_field_info() fn. (further investigation required!
 probably with tools/sql.sh* working backwards from
 db_set_default_connection()).

 {{{
 # a poor test, but ok as a first pass...
 $ cd vector/

 # find C modules which create a DB link:
 $ ADDDB=`grep -rIl Vect_map_add_dblink * | grep -v '/.svn/'`

 # the following will be safe & auto generate VAR as needed:
 $ SAFE=`grep -rIl Vect_default_field_info * | grep -v '/.svn/'`

 # find "A" not in "B" (or B not in A); stuff to investigate:
 $ echo $ADDDB $SAFE | tr ' ' '\n' | sort | uniq -u

 lidar/v.lidar.edgedetection/main.c
 v.db.connect/main.c
 v.transform/trans_digit.c  # ok: default_field() w/o add_dblink()
 }}}

 Ideally the check/create VAR lib fn should be called from the core "create
 new DB" lib fn, if that is not already the case. Thus the db test belongs
 in Vect_map_add_dblink(), Vect_add_dblink(), or Vect_check_dblink(), or
 Vect_write_dblinks() ? By the low-levelness of the last 3 I'd guess put it
 in Vect_map_add_dblink(), *if* Vect_map_add_dblink() is ever called
 without calling Vect_map_add_dblink() first. (and if there is a case like
 that maybe it is a bug?)


 [*] can we rename tools/sql.sh with a more descriptive name?
 sql_lib_tree.sh or something? Is it trivial/hard to adapt that to run for
 SQLite instead of PostgreSQL?



 It WAS a problem from some scripts which created DB tables on their own
 (db.execute, v.db.add*) but AFAIK those are few and have now all been
 fixed for some time. I've now updated those to call 'db.connect -c'.


 > It sounds like this can be fixed 2 ways: change g.proj to create a
 > VAR file when it creates a location

 this is the cart leading the horse and means the VAR creation code must be
 scattered in a bunch of places. Better to funnel the task into one place
 which everything which creates a new table must go past, ie as close as
 possible to when it is actually needed. For the C modules this is
 currently via Vect_default_field_info(), which is for:
  \brief Get default information about link to database for new dblink


 > (this is how locations are created in the location wizard)

 they needn't be and probably shouldn't be. (unless 'v.in.ogr -c locat=',
 but then that's already covered by v.in.ogr)

 > or change modules to gracefully deal with the lack of a VAR file.

 this is the approach which has been taken and AFAIK is fully in place.

 > The ongoing discussion about which of these two ways to fix it has
 > left this unfixed for months.

 No, AFAIK it has been fixed for months already, but hacks were still left
 here and there in the code. Now I've cleaned those up. (r30545, r30546,
 r30547)


 > I'm agnostic about which way is best, except that I don't think that
 > we should hack it into the location wizard wxPython code.

 It shouldn't need to be in any mapset/location creation code. It should be
 dealt with: in a single place, at the time of DB creation, if needed.


 Thus I think it will be safe to again re-disable the 'db.connect -c' call
 in init.sh in SVN/trunk. (due to lingering doubts in 6.3.0 Init.sh is now
 set to create VAR+dbf/ if not there)


 I hope this answers Paul's post of Feb 19 too,


 Hamish

-- 
Ticket URL: <http://trac.osgeo.org/grass/ticket/7#comment:13>
GRASS GIS <http://grass.osgeo.org>
GRASS Geographic Information System (GRASS GIS) - http://grass.osgeo.org/