ecmwf / ecmwf-api-client

Python API to access ECMWF archive
Apache License 2.0
66 stars 20 forks source link

Make get_apikey_values aligned with docstring #15

Closed din14970 closed 2 years ago

din14970 commented 2 years ago

The get_apikey_values function shows in my opinion some unexpected behavior, where it will never raise an APIKeyFetchError. If it fails to load credentials from either env variables or from the rc file it will return some bogus credentials. In my opinion this is easier and to the point. If the key can not be returned from the env variables, it will try the rc file. If this is also not found an APIKeyFetchError will be raised. Otherwise the correct key is returned.

FussyDuck commented 2 years ago

CLA assistant check
All committers have signed the CLA.

cristian-codorean commented 2 years ago

@din14970, the bogus credentials you are referring to are valid credentials which are meant to be used for anonymous access to some ECMWF datasets. ECMWF has fairly recently opened anonymous access to some of its datasets, mainly results of research experiments.

Now, from what I can see, code has been updated to support anonymous access, but not the docstring, as you have kindly pointed out. So, what I will do is close this pull request, since we want to leave the anonymous access credentials in, and update the docstring separately to align it with the code.

Then review the logic of the code there, since it's currently not very clear. For example, the way it works now, if you had an invalid rc file, you'd be switched to anonymous access without any indication of how and why ... I think there should either be a hard failure if you had an invalid rc file, or otherwise some information in the logs generated about that and the fallback to anonymous access.