nsidc / earthaccess

Python Library for NASA Earthdata APIs
https://earthaccess.readthedocs.io/
MIT License
414 stars 81 forks source link

Expose `endpoint` argument in `get_s3fs_session` top-level function #602

Open chuckwondo opened 4 months ago

chuckwondo commented 4 months ago

Per https://github.com/nsidc/earthaccess/discussions/482, there is a desire to be able to directly invoke get_s3fs_session from the top of earthaccess, like so:

earthaccess.get_s3fs_session(endpoint="https://nisar.asf.earthdatacloud.nasa.gov/s3credentials")

Currently, it is possible to do the following, but accessing __store__ is perhaps not desirable:

earthaccess.__store__.get_s3fs_session(endpoint="https://nisar.asf.earthdatacloud.nasa.gov/s3credentials")
jhkennedy commented 4 months ago

yes, this is a good idea. In the same vein, I also find this method useful and think it would be good to expose directly:

>>> earthaccess.__store__.auth.get_s3_credentials(endpoint='https://nisar.asf.earthdatacloud.nasa.gov/s3credentials')
{'accessKeyId': 'XXX',
 'secretAccessKey': 'XXX',
 'sessionToken': 'XXX',
 'expiration': '2024-06-12 20:48:21+00:00'}

and might be a nice addition to the TEA s3 credentials READMEs: https://nisar.asf.earthdatacloud.nasa.gov/s3credentialsREADME

Notably, this does not work:

>>> arthaccess.auth.Auth.get_s3_credentials(endpoint='nisar.asf.earthdatacloud.nasa.gov/s3credentials')
TypeError: Auth.get_s3_credentials() missing 1 required positional argument: 'self'

even though I think it could be provided as just a function or provided as a static method.

JessicaS11 commented 3 months ago

For anyone interested in working on this issue, there is currently an earthaccess.get_s3fs_session() function exposed at the top level (api.py). My understanding is this issue aims specifically to include exposure of the endpoint kwarg there.

chuckwondo commented 3 months ago

For anyone interested in working on this issue, there is currently an earthaccess.get_s3fs_session() function exposed at the top level (api.py). My understanding is this issue aims specifically to include exposure of the endpoint kwarg there.

Yes, sorry if my original description was confusing.

mfisher87 commented 3 months ago

How's this read to y'all?