Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add URL support to DataAccessor #245

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

whyjz
Copy link

@whyjz whyjz commented Mar 3, 2021

As discussed in #243, I edited the rule for finding the VRT -- if the file name is actually a URL, the Data Accessor will read the corresponding VRT file (that points to the remote, original file) on the local machine. It should be compatible with almost all the ICSE use cases. Let me know if you have any questions about this change.

Thanks!

Copy link
Member

@rtburns-jpl rtburns-jpl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piyushrpt do you think this is a reasonable assumption to make? Can you imagine a situation where someone would want the extraFilename to be remote as well?

self.extraFilename = self.filename + '.' + self._extra_reader
#if the filename is a URL, the extraFilename should indicate the file from the local machine
#instead of from the remote server.
if self.filename.startswith('http'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if self.filename.startswith('http'):
if self.filename.startswith('http://') or self.filename.startswith('https://'):

It may be possible someone's filename starts with http, but I wouldn't expect a local filename to contain //

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not create a .vrt and .xml and have source in vrt to point to remote? That way you have a record of what resource was actually used? That way you are not restricted to something thats just http / https. You can throw in anything vsis3 / vsiaz/ vsicurl etc. This approach still seems to need a .vrt. Just using basename of .vrt should be enough when it points to other things?

Copy link
Author

@whyjz whyjz Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piyushrpt I did create a .vrt that points to a remote geotiff. In my use case, the XML is not needed. Here's an example of what would happen if running this snippet using the original code version:

import isce
from isceobj.Image.Image import Image
obj = Image()
obj.setFilename("https://download.osgeo.org/geotiff/samples/GeogToWGS84GeoKey/GeogToWGS84GeoKey5.tif")
obj.setWidth(101)
obj.setDataType('SHORT')
obj.setCaster('read', 'FLOAT')
obj.createImage()
# ... (continue to some Ampcor code)
GDAL open (R): https://download.osgeo.org/geotiff/samples/GeogToWGS84GeoKey/GeogToWGS84GeoKey5.tif.vrt
ERROR 1: HTTP error code : 404
Error. Cannot open the file https://download.osgeo.org/geotiff/samples/GeogToWGS84GeoKey/GeogToWGS84GeoKey5.tif.vrt in read mode.
Error in file /home/whyj/Software/isce2-2.4.2-build/isce/components/iscesys/ImageApi/InterleavedAccessor/src/GDALAccessor.cpp at line 77 Exiting

You can see that the ISCE's default behavior is to read a VRT that is at the same location as the original file, and it does not matter whether you have created a VRT on your local machine or not. If we replace the geotiff file name with the local vrt, ISCE will try to gdal open .vrt.vrt, which does not exist.

Explicitly identify URL file names

Co-authored-by: Ryan Burns <rtburns@jpl.nasa.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants