-
Notifications
You must be signed in to change notification settings - Fork 86
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
Small store fixups #308
Small store fixups #308
Conversation
@@ -533,7 +533,7 @@ def _get_granules( | |||
provider = granules[0]["meta"]["provider-id"] | |||
endpoint = self._own_s3_credentials(granules[0]["umm"]["RelatedUrls"]) | |||
cloud_hosted = granules[0].cloud_hosted | |||
access = "direc" if (cloud_hosted and self.running_in_aws) else "external" | |||
access = "direct" if (cloud_hosted and self.running_in_aws) else "external" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking out loud: I think it'd be really useful to add type safety here. We could type access
as Enum or as a union of literal string types.
I got sidetracked last week and didn't have a chance to come back to this and my other problem that's described in #307 until now. I thought I might come across other items that may end up in this PR, but since I'm just getting re-started and I'm not certain of that, I want to make sure these small fixes get merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally this looks good to me. @betolink @MattF-NSIDC any additional thoughts?
🚀 |
Couple of minor fixups that I noticed while looking at
earthaccess.store
:access
variable assignment (direc
->direct
)threads
arg to_open_urls_https
in_open_urls
, replacing hard-coded value of 8.