josephramsay / lds_replicate

Replication scripts for LDS (LINZ Data Service).
http://data.linz.govt.nz/
2 stars 1 forks source link

Clarity in user configuration #8

Open josephramsay opened 11 years ago

josephramsay commented 11 years ago

1. Need a meaningful error message when the layerconf object returns a nonetype has no attribute...

We use 2 (maybe 3) config files. One main one setting paths, keys etc. Most of the stuff in this can be overriden on the command line or in a duplicate user config file. The other file, a layer config file, gets stored either as text or in the destination database. It contains meta information about each layer including primary key field ("id" or blank depending on whether its topo/hydro or not) and date last modified (for incremental updates)

If it fails to initialise or cannot be found it will trigger the message above. I'll throw a custom exception to make this more meaningful .

Just to be clear are you going to thrown an "exception" or fail with an error message? Also what was error? [JP]

Fail. Without a valid layerconf you could potentially mess up your database

new error message: class LayerConfigurationException(Exception): pass if self.dst.layerconf is None: raise LayerConfigurationException("Cannot initialise Layer-Configuration file/table. fn="+str(fname)+',int='+str(dst.isConfInternal())) [JR]

2. There is a potential conflict with a user putting in paramaters for one output but requesting another.

Beause the parameters themselves are optional (otherwise read from the config file) we still need the destination description ie the "pg" parameter. The solution here might be to identify when a user inputs such a conflict.

3. Cannot put API key on the commandline...

Thinking about it this isnt a problem since the api key forms part of the "-s" LDS URL. Adding it as a seperate item and retaining the url parameter could lead to a conflict situation as above.

Great. Would also be good to tell the user if the API set is not set [JP]

Good point. That will apply to most settings (though the main conf should really have defaults set if the command line option is set these are ignored ie CL overrides everything and users responsiblity to get it right) but some basic checking would be wise. [JR]

josephramsay commented 11 years ago
  1. cont Added a try/catch to the main() method to hide stack-trace from the user.
josephramsay commented 11 years ago

2 . cont. With meaningful error messages on connection string validation it should be clear to the user when this type of error occurs i.e. that they've requested one type of output format but provided the connection string for another. Not all possible error conditions have been tested for: e.g We dont check the WFS string for optional parameters like GetFeature vs GetCapabilities nor do we check PG:... user='' and password='' since a trusted_connection negates this requirement.

NB. [Change] Newest GDAL/FileGDB fixes FileGDB bug where SetAttributeFilter required its own custom query format (sans single quotes). Caused error "Failed Searching (An expected Field was not found or could not be retrieved properly.)"

layer.SetAttributeFilter("id = v:x772")

becomes

layer.SetAttributeFilter("id = 'v:x772'")

aligning with common driver convention

josephramsay commented 11 years ago

Added internal/external as CL options that will override config file Added layer and incremental date overrides is user conn_str is set. (checks also set in case conn_str and matching parameter values are set simultaneously) Added read/parse conn_str to override options