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 function for querying collections from CMR #290

Merged
merged 3 commits into from
Aug 4, 2023
Merged

add function for querying collections from CMR #290

merged 3 commits into from
Aug 4, 2023

Conversation

tsutterley
Copy link
Contributor

add function for retrieving the maximum available version for a dataset

>>> import sliderule.earthdata
>>> sliderule.earthdata.__cmr_max_version('NSIDC_ECS','ATL03')
'006'

add function for retrieving the maximum available version for a dataset
Comment on lines +195 to +198
# ssl context
ctx = ssl.create_default_context()
ctx.check_hostname = False
ctx.verify_mode = ssl.CERT_NONE
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity what does this do is it necessary?

Copy link
Contributor Author

@tsutterley tsutterley Aug 3, 2023

Choose a reason for hiding this comment

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

sometimes urllib throws ssl errors if a context is not provided

Copy link
Member

Choose a reason for hiding this comment

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

I believe this disables cert verification. So it protects against man-in-the-middle and things like that. There are a handful of reasons a valid cert could fail to be verified; and while I know technically we shouldn't do this, it appears in the code for CMR access elsewhere and I don't think it is overly risky as there is no sensitive information being passed in either direction.

Comment on lines +200 to +201
req = urllib.request.Request(cmr_query_url)
response = urllib.request.urlopen(req, context=ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

requests is a dependency
https://github.com/ICESat2-SlideRule/sliderule/blob/7e16d3e90efc932fc96219a2bba81daf040b44fa/clients/python/requirements.txt#L1

but is urllib easy enough to use instead throughout the client such that requests could be dropped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point. we might want to create an issue to unify how HTTP requests work

Copy link
Contributor

@scottyhq scottyhq left a comment

Choose a reason for hiding this comment

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

@tsutterley
Copy link
Contributor Author

thanks @scottyhq! I added some tests to the TestCMR class within test_earthdata 👍

@scottyhq
Copy link
Contributor

scottyhq commented Aug 3, 2023

Looks like some of the tests are failing related to arcticdem file names. But this error as well: E AttributeError: module 'sliderule.earthdata' has no attribute '_TestCMR__cmr_max_version' . might need some guidance from @jpswinski to get the green checkmark!

@tsutterley
Copy link
Contributor Author

Looks like pytest doesn't like using functions starting with __. Importing the functions without that prefix works with pytest on my local machine

@jpswinski jpswinski merged commit b1e4aab into main Aug 4, 2023
4 of 5 checks passed
@jpswinski jpswinski deleted the earthdata branch August 4, 2023 13:54
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