ecmwf / earthkit-data

A format-agnostic Python interface for geospatial data
Apache License 2.0
47 stars 9 forks source link

cdsapi authentication issues #301

Closed malmans2 closed 4 months ago

malmans2 commented 4 months ago

What happened?

I'm encountering various issues:

  1. earthkit is not consistent with cdsapi beacuse it does not use the environment variables "CDSAPI_{RC,KEY,URL}". Looks like users are expected to either store the credentials in ~/.cdsapirc or use the prompt.
  2. When ~/.cdsapirc is missing, the prompt functionality fails using ipython (see snippet below)
  3. If I write a dummy ~/.cdsapirc and I set the CDSAPI env vars, I'm able to skip the prompt and cdsapi uses the env vars. I'm achieving what I need, but it looks like a bug because ~/.cdsapirc is mandatory but not used.

In summary, I think earthkit-data should either define its own authentication rules or use cdsapi rules, whereas right now there's a mix of the two. Allowing only ~/.cdsapirc would be a problem for our project because many users are working on a shared VM, so they need to be able to store their credentials in different files.

Side note: the cds source requires the package markdown because it's imported at top-level. However, as far as I can tell that library is only needed for the prompt messages, so it might be easier to import it within the relevant method.

What are the steps to reproduce the bug?

import pathlib
import earthkit.data

assert not (pathlib.Path.home() / ".cdsapirc").exists()
ds = earthkit.data.from_source(
    "cds",
    "reanalysis-era5-single-levels",
    variable=["2t", "msl"],
    product_type="reanalysis",
    area=[50, -10, 40, 10],  # N,W,S,E
    grid=[2, 2],
    date="2012-05-10",
    time="12:00",
)  # TypeError: can only concatenate str (not "tuple") to str

Version

0.5.1

Platform (OS and architecture)

Darwin MacBook-Pro-3.local 22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:21:56 PDT 2023; root:xnu-8796.141.3~6/RELEASE_X86_64 x86_64

Relevant log output

<IPython.core.display.HTML object>
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Cell In[1], line 5
      2 import earthkit.data
      4 assert not (pathlib.Path.home() / ".cdsapirc").exists()
----> 5 ds = earthkit.data.from_source(
      6     "cds",
      7     "reanalysis-era5-single-levels",
      8     variable=["2t", "msl"],
      9     product_type="reanalysis",
     10     area=[50, -10, 40, 10],  # N,W,S,E
     11     grid=[2, 2],
     12     date="2012-05-10",
     13     time="12:00",
     14 )  # TypeError: can only concatenate str (not "tuple") to str

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/__init__.py:149, in from_source(name, lazily, *args, **kwargs)
    146     return from_source_lazily(name, *args, **kwargs)
    148 prev = None
--> 149 src = get_source(name, *args, **kwargs)
    150 while src is not prev:
    151     prev = src

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/__init__.py:130, in SourceMaker.__call__(self, name, *args, **kwargs)
    123 klass = find_plugin(os.path.dirname(__file__), name, loader)
    125 # if os.environ.get("FIEDLIST_TESTING_ENABLE_MOCKUP_SOURCE", False):
    126 #     from earthkit.data.mockup import SourceMockup
    127
    128 #     klass = SourceMockup
--> 130 source = klass(*args, **kwargs)
    132 if getattr(source, "name", None) is None:
    133     source.name = name

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/core/__init__.py:22, in MetaBase.__call__(cls, *args, **kwargs)
     20 obj = cls.__new__(cls, *args, **kwargs)
     21 args, kwargs = cls.patch(obj, *args, **kwargs)
---> 22 obj.__init__(*args, **kwargs)
     23 return obj

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/cds.py:111, in CdsRetriever.__init__(self, dataset, *args, **kwargs)
    108 assert all(isinstance(request, dict) for request in args)
    109 self._args = args
--> 111 self.client()  # Trigger password prompt before thraeding
    113 nthreads = min(self.settings("number-of-download-threads"), len(self.requests))
    115 if nthreads < 2:

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/cds.py:95, in CdsRetriever.client(self)
     94 def client(self):
---> 95     return client()

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/cds.py:71, in client()
     69 def client():
     70     prompt = CDSAPIKeyPrompt()
---> 71     prompt.check()
     73     try:
     74         return cdsapi.Client()

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/prompt.py:131, in APIKeyPrompt.check(self, load)
    129 rcfile = os.path.expanduser(self.rcfile)
    130 if not os.path.exists(rcfile):
--> 131     self.ask_user_and_save()
    133 if load:
    134     with open(rcfile) as f:

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/prompt.py:146, in APIKeyPrompt.ask_user_and_save(self)
    145 def ask_user_and_save(self):
--> 146     input = self.ask_user()
    148     rcfile = os.path.expanduser(self.rcfile)
    149     with open(rcfile, "w") as f:

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/prompt.py:143, in APIKeyPrompt.ask_user(self)
    140 else:
    141     prompt = Text(self)
--> 143 return self.validate(prompt.ask_user())

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/prompt.py:68, in Prompt.ask_user(self)
     66 for p in self.owner.prompts:
     67     method = getpass if p.get("hidden", False) else input
---> 68     value = self.ask(p, method)
     69     value = value.strip() or p.get("default", "")
     71     validate = p.get("validate")

File ~/miniforge3/envs/eqc/lib/python3.11/site-packages/earthkit/data/sources/prompt.py:115, in Markdown.ask(self, p, method)
    113 message = f"Please enter a value for <span style='color: red;'>{p.get('title')}</span>"
    114 if "default" in p:
--> 115     message += (
    116         " or leave empty for the default value ",
    117         f"<span style='color: red;'>{p.get('default')}</span>",
    118     )
    119 message += ", then press *ENTER*"
    120 if "example" in p:

TypeError: can only concatenate str (not "tuple") to str

Accompanying data

No response

Organisation

B-Open / CADS-EQC

sandorkertesz commented 4 months ago

Thanks for reporting this issue. The cds source authentication will be extended to match cdsapi.

sandorkertesz commented 4 months ago

This issue has been fixed in develop. Please let me know if it works as expected .

malmans2 commented 4 months ago

Yes, it works as expected now. Thanks!

malmans2 commented 2 months ago

@sandorkertesz could you please let us know when you're planning to release this fix? It looks like there's been a few releases since this issue was closed. Thanks!

sandorkertesz commented 2 months ago

@malmans2 , 0.7.0 containing this fix is planned to be released today or tomorrow

sandorkertesz commented 2 months ago

@malmans2 0.7.0 with this feature has been released: https://earthkit-data.readthedocs.io/en/latest/release_notes/version_0.7_updates.html. Please note that some of the Python dependencies (e.g. cdsapi) became optional. See the details here: https://earthkit-data.readthedocs.io/en/latest/install.html

malmans2 commented 2 months ago

Great, thanks. We'll give it a go when it's released on conda-forge as well.