Handle lowercase daac and provider inputs

[jrbourbeau]

I was just working with someone who got an error because they provider provider="pocloud" (lowercase) instead of provider="POCLOUD". All our internal machinery expects provider and daac to be all uppercase, however it's pretty natural (especially for new users) to use lowercase instead. We should just handle this case by normalizing these inputs to be all uppercase. earthaccess. get_s3_credentials already had logic for this, but other methods didn't. This PR adds similar logic to other top-level earthaccess APIs.

[github-actions]

👈 Launch a binder notebook on this branch for commit 3b48f40

I will automatically update this comment whenever this PR is modified

👈 Launch a binder notebook on this branch for commit f186096

👈 Launch a binder notebook on this branch for commit 46814f7

👈 Launch a binder notebook on this branch for commit 880f45e

[MattF-NSIDC]

I found confirmation, thanks to John Teague in Slack, that provider IDs cannot have lowercase letters.

https://wiki.earthdata.nasa.gov/display/CMR/CMR+Data+Partner+User+Guide#CMRDataPartnerUserGuide-CMRProviderIdentification

The identifier must have a short name that satisfies the following criteria: 10 characters or less; unique against all other providers; and capital letter as the first character followed by capital letters, numbers, or an underscore. The long name of the identifier can be much longer.

[MattF-NSIDC]

I've never heard of the daac field before, is that a CMR field?

EDIT: It's a field in a constant lookup internal to earthaccess:

earthaccess/earthaccess/daac.py

Line 6 in 7db2e59

DAACS = [

[jrbourbeau]

Sure, and our internal handling of provider and daac IDs should definitely be all uppercase, I'm just saying that users may not know that they need to be all uppercase. I'm just proposing here that we handle any needed lowercase --> uppercase conversion for users.

EDIT: So users don't need to think about upper vs. lowercase

[MattF-NSIDC]

I understood all that :) I'm totally on board with the proposal! I should have been more clear in my first post.

I was just validating the underlying assumption that provider IDs are 100% guaranteed to be uppercase, because while I'd never seen a lowercase character, I hadn't seen it guaranteed in writing before. Maybe we should link to the doc above in a docstring for _normalize_location() for posterity?

[MattF-NSIDC]

Total nit that shouldn't be taken seriously: does Optional[str] seem like a readability improvement at all?

I'm writing Typescript lately where you can do location?: string and that's pretty slick.

[jrbourbeau]

(Preface: I'm not very steeped in type annotations) To me Union seems more appropriate here because this is a required positional argument, not a keyword with a default of None. But I don't care strongly either way as long as linting checks pass.

[MattF-NSIDC]

Union seems more appropriate here because this is a required positional argument, not a keyword with a default of None

Do you mean you think Union is more clear for that reason? Functionally they are the same thing, the argument would still be required: https://docs.python.org/3/library/typing.html#typing.Optional

[jrbourbeau]

Do you mean you think Union is more clear for that reason?

Yes. Though I don't feel very strongly either way (or in general about type annotations). Happy to update if you'd prefer.

[MattF-NSIDC]

I see the unclearness of Optional in that regard, good point! I also don't feel strongly, so let's leave it? :)

[jrbourbeau]

I was just validating the underlying assumption that provider IDs are 100% guaranteed to be uppercase, because while I'd never seen a lowercase character, I hadn't seen it guaranteed in writing before

Ah, I see -- thank you for confirming!

Maybe we should link to the doc above in a docstring for _normalize_location() for posterity?

Sounds good to me. However, https://wiki.earthdata.nasa.gov/display/CMR/CMR+Data+Partner+User+Guide#CMRDataPartnerUserGuide-CMRProviderIdentification gives "Forbidden" for me

Do you happen to have a different link handy?

[MattF-NSIDC]

That link works for me after logging in through EarthData Login. Were you not even prompted to log in? Or maybe you're already logged in and don't have access. Let me see if I can find a place this is publicly documented.

[MattF-NSIDC]

Can you access this link?

https://wiki.earthdata.nasa.gov/pages/viewpage.action?spaceKey=ED&title=CMR+Data+Partner+User+Guide#CMRDataPartnerUserGuide-CMRProviderIdentification

or

https://wiki.earthdata.nasa.gov/display/ED/CMR+Data+Partner+User+Guide?src=contextnavpagetreemode

[jrbourbeau]

Both of those links give me

500 Internal Server Error
If you are the administrator of this website, then please read this web application's log file and/or the web server's log file to find out what went wrong.

I'll update the docstring to include some explanation. We can always add a link after the fact if needed

[MattF-NSIDC]

Jeez. Why are these docs behind a login wall at all? 🤯

Thanks for trying!

[MattF-NSIDC]

@jrbourbeau can you try those links again? I'm told there was maintenance this morning.

[jrbourbeau]

Now getting "Forbidden" like in #355 (comment) instead of the 500 error

[mfisher87]

Waiting for response from CMR folks about a public copy of this documentation. If we want to get this merged, we could write a docstring linking to this private documentation, so those of us with access can find the source of truth when we need to validate our assumptions.

Better to have a private reference than no reference, I think. Eventually we can replace it with a public link!

[jrbourbeau]

Better to have a private reference than no reference, I think. Eventually we can replace it with a public link!

I went ahead and added a link in the docstring

I'm planning to merge this in a few hours if no further comments. This seems like a low-risk, relatively small change that will make users lives a bit nicer. Happy to handle any follow-up work if there is any.