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

Updates to handle virtual files #143

Merged
merged 2 commits into from
Apr 23, 2020
Merged

Conversation

sssangha
Copy link
Collaborator

Added updates as proposed in issue #138

Specifically, updates to appropriately load virtual products and also test whether or not the gdal install is properly configured to handle these virtual product files.

Additionally, files no longer opened with the gdal driver to parse product layer keys. These layer names are now hardcoded and parsed under the "mappingVersion" subfunction, similar to how radar metdata keys were handled in the previous commit #141

if any("https://" in i for i in self.files):
gdal.SetConfigOption('GDAL_HTTP_COOKIEFILE','cookies.txt')
#check if reader is being captured as netcdf
[s for s in self.files if 'https://' in s][0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

L75 might have been used for debugging?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup, will have to remove. Will do now.

Comment on lines +122 to +124
if 'https://' in fname:
version=str(gdal.Open(fname).GetMetadata_Dict()['version'])
else:
Copy link
Collaborator

@dbekaert dbekaert Apr 22, 2020

Choose a reason for hiding this comment

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

Reading of 'NC_GLOBAL#version' should not be different if in S3.
L122-124 likely to be removed.
To be tested on opensarlabs as it needs the netcdf to be recognized!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup, will have to confirm on opensarlabs, once the technical issue on my end gets resolved. From my end locally though, I could successfully access the version from the URLs this way.

Comment on lines 72 to 77
if any("https://" in i for i in self.files):
gdal.SetConfigOption('GDAL_HTTP_COOKIEFILE','cookies.txt')
#check if reader is being captured as netcdf
[s for s in self.files if 'https://' in s][0]
fmt=gdal.Open([s for s in self.files if 'https://' in s][0]).GetDriver().GetDescription()
if fmt != 'netCDF': raise Exception ('Your GDAL install is not configured to handle virtual files')
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should also add here a test for a single file which does not have https.
e.g. verify for a local file that it also gets captured with the netcdf reader.
If not netcdf error out and say version >2.3 is needed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I will simplify and add in the proposed check for locally downloaded products.

Comment on lines 72 to 77
if any("https://" in i for i in self.files):
gdal.SetConfigOption('GDAL_HTTP_COOKIEFILE','cookies.txt')
#check if reader is being captured as netcdf
[s for s in self.files if 'https://' in s][0]
fmt=gdal.Open([s for s in self.files if 'https://' in s][0]).GetDriver().GetDescription()
if fmt != 'netCDF': raise Exception ('Your GDAL install is not configured to handle virtual files')
Copy link
Collaborator

Choose a reason for hiding this comment

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

L77 the error for S3 being h5, could be more specific indicating: Linux kernel >=4.3 and libnetcdf >=4.5

@dbekaert
Copy link
Collaborator

@sssangha added a review with comments.

Sanity check to confirm local files are captured with the netcdf error. Exception statement also updated to be more transparent.
@sssangha
Copy link
Collaborator Author

@dbekaert I have made some of the suggested changes. Did not change the way the version is read yet though, as I cannot login to opensarlab yet to confirm.

Copy link
Collaborator

@dbekaert dbekaert left a comment

Choose a reason for hiding this comment

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

looks good, Will merge it in and do a commit on top of this to make it work.
Will post on the issue ticket with more information,

@dbekaert dbekaert merged commit af8d430 into aria-tools:dev Apr 23, 2020
@sssangha sssangha deleted the sss_urlupdate branch April 24, 2020 22:33
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.

2 participants