More robust filsystem check for file-based and non-file based input connections. #337

Closed
artemp opened this Issue Oct 11, 2011 · 6 comments

Comments

Projects
None yet
1 participant
Owner

artemp commented Oct 11, 2011

The Mapnik OGR plugin's filesystem check (line 71 of ogr_datasource.cpp) does not permit the use of OGR's OGDI driver (and others?). The problem stems from the OGDI driver's filename string format:

gltp://

which needs to have parsed to pass the existing filesystem check. There may be similar issues with other OGR/GDAL drivers.

Owner

artemp commented Oct 11, 2011

[springmeyer] I think the right approach is to just rename the 'file' keyword to datasource for the 'ogr' driver and accept any datasource string that ogr reads. I'll look into GDAL dataset connections we may be preventing next. (see also #126)

Owner

artemp commented Oct 11, 2011

[kunitoki] Yes i see the point. It is a mixed gdal/ogr problem since there are some formats based on network urls (think WMS for gdal too) that cannot be checked for existence like any other local file.

As Dane pointed out, i think that is the path to follow, we only need to be sure that the gdal/ogr driver is correctly reporting any problem to the mapnik datasource when it tries to open a non-existent file (for file based dataset).

What about renaming 'file' to 'dataset' ? (i think have a 'datasource' parameter inside a isn't exactly straightforward)

Owner

artemp commented Oct 11, 2011

[springmeyer] Replying to [comment:2 kunitoki]:

Yes i see the point. It is a mixed gdal/ogr problem since there are some formats based on network urls (think WMS for gdal too) that cannot be checked for existence like any other local file.

Yes, and the new TMS GDAL driver too...

As Dane pointed out, i think that is the path to follow, we only need to be sure that the gdal/ogr driver is correctly reporting any problem to the mapnik datasource when it tries to open a non-existent file (for file based dataset).

Exactly, we should be letting gdal/ogr spit back the right errors, and I've got a patch I'll post to #336 for all to review that starts doing that....

What about renaming 'file' to 'dataset' ? (i think have a 'datasource' parameter inside a isn't exactly straightforward)
Yes, I agree that 'datasource' could cause more confusion than its worth - so, I agree with using 'dataset' and made that same comment on # 126 - just a question of when we should make that change....

Owner

artemp commented Oct 11, 2011

[springmeyer] Okay, a note here on the '''gdal'' plugin.

Same problem for sure - that I shouldn't have added the filesystem check since GDAL is about as smart as it gets about reporting the right error.

So, if we pass the file param to GDALOpen/GDALDataset and then catch CPLGetLastErrorMsg() we can get some nice error messages:

{{{

Gdal(file='nodata_byte.tif')
<mapnik.Datasource object at 0x2383b0>
Gdal(file='nodata_byt.tif')
ERROR 4: `nodata_byt.tif' does not exist in the file system,
and is not recognised as a supported dataset name.

Traceback (most recent call last):
File "", line 1, in
File "/Library/Python/2.5/site-packages/mapnik/init.py", line 219, in Gdal
return CreateDatasource(keywords)
RuntimeError: `nodata_byt.tif' does not exist in the file system,
and is not recognised as a supported dataset name.
}}}

Or when a file is opened successfully but we don't have the driver installed:

{{{
ERROR 4: `data/uint32_3.hdf' not recognised as a supported file format.

`data/uint32_3.hdf' not recognised as a supported file format.

ERROR 1: The selected file is an ENVI header file, but to
open ENVI datasets, the data file should be selected
instead of the .hdr file. Please try again selecting
the data file corresponding to the header file:
data/utmsmall.hdr

The selected file is an ENVI header file, but to
open ENVI datasets, the data file should be selected
instead of the .hdr file. Please try again selecting
the data file corresponding to the header file:
data/utmsmall.hdr
}}}

Owner

artemp commented Oct 11, 2011

[springmeyer] applied fix in r1129

(leaving open until we figure out/fixup slightly more tricky ogr handling)

Owner

artemp commented Oct 11, 2011

[springmeyer] forgot to close this ticket out, as Kunitoki applied my patch for the ogr driver in r1130

Thanks again for the report Ryan. Re-open if you find any further problems.

artemp closed this Oct 11, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment