updated cds_credentials including tests (#925)

* updated cds_credentials including tests

* updated whatsnew

* support for envvar onlykey (no url)

* added testcases for rcfile

* added commented code for renaming

* define default cds_url only once

* included rename of valid_time coordinate for ERA5 datasets downloaded with new cds-beta, update whatsnew

* added valid_time support in test_download_era5

dfm_tools/download.py

@@ -91,77 +91,91 @@ def download_ERA5(varkey,
         c.retrieve(name='reanalysis-era5-single-levels', request=request_dict, target=file_out)
 
 
-def cds_get_file():
-    file_cds_credentials = os.environ.get("CDSAPI_RC", os.path.expanduser("~/.cdsapirc"))
-    return file_cds_credentials
-
-
 def cds_credentials():
     """
     get cdsapikey from environment variables or file or query via getpass if necessary
     """
-    #TODO: put this in a PR at https://github.com/ecmwf/cdsapi (https://github.com/ecmwf/cdsapi/blob/master/cdsapi/api.py#L303)
-    cds_url = os.environ.get("CDSAPI_URL", "https://cds.climate.copernicus.eu/api/v2")
-    # set default/provided CDSAPI_URL back to environ for platforms like EDITO 
-    # that depend on environ (only CDSAPI_KEY has to be set in that case)
-    os.environ["CDSAPI_URL"] = cds_url
-    cds_uid_apikey = os.environ.get("CDSAPI_KEY")
-
-    # read credentials from file if it exists. This has higher precedence over env vars
-    file_cds_credentials = cds_get_file()
-    if os.path.isfile(file_cds_credentials):
-        config = cdsapi.api.read_config(file_cds_credentials)
-        cds_url = config["url"]
-        cds_uid_apikey = config["key"]
+    # TODO: put this in a PR at https://github.com/ecmwf/cdsapi
 
+    cds_url = os.environ.get("CDSAPI_URL", "https://cds-beta.climate.copernicus.eu/api")
     try:
-        # checks whether CDS apikey is in environment variable or ~/.cdsapirc file and if it is in correct format
-        c = cdsapi.Client()
-        # checks whether credentials (uid and apikey) are correct
-        c.retrieve(name="dummy", request={})
+        # set default for CDSAPI_URL envvar so it does not have to be supplied. This also ignores the URL in ~/.cdsapirc
+        os.environ["CDSAPI_URL"] = cds_url
+        # gets url/key from env vars or ~/.cdsapirc file
+        cds_url, cds_apikey, _ = cdsapi.api.get_url_key_verify(url=cds_url, key=None, verify=None)
     except Exception as e:
         if "Missing/incomplete configuration file" in str(e):
-            # to catch "Exception: Missing/incomplete configuration file"
-            # query uid and apikey if not present
-            print("Downloading CDS/ERA5 data requires a CDS API key, copy your UID and API-key from https://cds.climate.copernicus.eu/user (first register, login and accept the terms). ")
-            cds_uid = getpass.getpass("\nEnter your CDS UID (six digits): ")
-            cds_apikey = getpass.getpass("\nEnter your CDS API-key (string with dashes): ")
-            cds_uid_apikey = f"{cds_uid}:{cds_apikey}"
-            os.environ["CDSAPI_URL"] = cds_url
-            os.environ["CDSAPI_KEY"] = cds_uid_apikey
-            with open(file_cds_credentials,'w') as fc:
-                fc.write(f'url: {cds_url}\n')
-                fc.write(f'key: {cds_uid_apikey}')
-            cds_credentials()
-        elif "not the correct format" in str(e):
-            # to catch "AssertionError: The cdsapi key provided is not the correct format, please ensure it conforms to: <UID>:<APIKEY>."
-            cds_remove_credentials()
-            raise Exception(f"{e}. The CDS apikey environment variables were deleted. Try again.")
-        elif "Authorization Required" in str(e):
-            cds_remove_credentials()
-            raise Exception("Authorization failed. The CDS apikey environment variables were deleted. Try again.")
-        elif "Resource dummy not found" in str(e):
+            # query apikey if not present in file or envvars
+            print("Downloading CDS/ERA5 data requires a ECMWF API-key, copy your API-key from https://cds-beta.climate.copernicus.eu/profile (first register, login and accept the terms). More info in https://forum.ecmwf.int/t/3743). ")
+            cds_apikey = getpass.getpass("\nEnter your ECMWF API-key (string with dashes): ")
+            cds_set_credentials(cds_url, cds_apikey)
+        else:
+            raise e
+
+    # remove cdsapirc file or env vars if the url/apikey are according to old format
+    if cds_url=="https://cds.climate.copernicus.eu/api/v2":
+        # to avoid "HTTPError: 401 Client Error: Unauthorized for url"
+        cds_remove_credentials_raise(reason='Old CDS URL found')
+    if ":" in cds_apikey:
+        # to avoid "Exception: Not Found" and "HTTPError: 404 Client Error: Not Found for url: https://cds-beta.climate.copernicus.eu/api/resources/dummy"
+        cds_remove_credentials_raise(reason='Old CDS API-key found (with :)')
+
+    # check if the authentication works
+    # TODO: requested "check authentication" method in https://github.com/ecmwf/cdsapi/issues/111
+    try:
+        # checks whether CDS apikey is in environment variable or ~/.cdsapirc file
+        c = cdsapi.Client()
+        # checks whether authentication is succesful (correct combination of url and apikey)
+        c.retrieve(name='dummy', request={})
+    except RuntimeError as e:
+        if "dataset dummy not found" in str(e):
             # catching incorrect name, but authentication was successful
-            print('found CDS credentials and authorization successful')
+            print('found ECMWF API-key and authorization successful')
+        elif "Authentication failed" in str(e):
+            cds_remove_credentials_raise(reason='Authentication failed')
         else:
             raise e
 
 
-def cds_remove_credentials():
+def cds_get_file():
+    file_cds_credentials = os.environ.get("CDSAPI_RC", os.path.expanduser("~/.cdsapirc"))
+    return file_cds_credentials
+
+
+def cds_set_credentials_rcfile(cds_url, cds_apikey):
+    file_cds_credentials = cds_get_file()
+    with open(file_cds_credentials,'w') as fc:
+        fc.write(f'url: {cds_url}\n')
+        fc.write(f'key: {cds_apikey}')
+
+
+def cds_set_credentials(cds_url, cds_apikey):
+    # set env vars
+    os.environ["CDSAPI_URL"] = cds_url
+    os.environ["CDSAPI_KEY"] = cds_apikey
+
+    # set ~/.cdsapirc file
+    cds_set_credentials_rcfile(cds_url, cds_apikey)
+
+
+def cds_remove_credentials_raise(reason=''):
     """
     remove CDS url and uid:apikey environment variables and ~/.cdsapirc file
     environment variables defined in https://github.com/ecmwf/cdsapi/blob/main/cdsapi/api.py
     """
 
     keys_toremove = ["CDSAPI_URL",
-                     "CDSAPI_KEY"]
+                     "CDSAPI_KEY",
+                     "CDSAPI_RC"]
     for key in keys_toremove:
         if key in os.environ.keys():
             os.environ.pop(key)
-    
+
     file_cds_credentials = cds_get_file()
     if os.path.isfile(file_cds_credentials):
         os.remove(file_cds_credentials)
+
+    raise ValueError(f"{reason}. The CDS/ECMWF apikey environment variables and rcfile were deleted. Please try again.")
 
 
 def download_CMEMS(varkey,

dfm_tools/xarray_helpers.py

@@ -103,6 +103,12 @@ def preprocess_ERA5(ds):
         # TODO: this drops int encoding which leads to unzipped float32 netcdf files: https://github.com/Deltares/dfm_tools/issues/781
         ds = ds.mean(dim='expver')
 
+    # datasets retrieved with new cds-beta have valid_time instead of time dimn/varn
+    # https://forum.ecmwf.int/t/new-time-format-in-era5-netcdf-files/3796/5?u=jelmer_veenstra
+    # TODO: can be removed after https://github.com/Unidata/netcdf4-python/issues/1357 or https://forum.ecmwf.int/t/3796 is fixed
+    if 'valid_time' in ds.coords:
+        ds = ds.rename({'valid_time':'time'})
+
     # prevent incorrect scaling/offset when merging files
     prevent_dtype_int(ds)
 

docs/whats-new.md

@@ -5,6 +5,7 @@
 ### Feat
 - making `dfmt.open_dataset_extra()` more modular by partly moving it to separate private functions in [#913](https://github.com/Deltares/dfm_tools/pull/913)
 - added station_id variable to dataset returned by `dfmt.interp_uds_to_plipoints()` in [#914](https://github.com/Deltares/dfm_tools/pull/914)
+- update private functions under `dfmt.download_ERA5()` to CDS-Beta (requires ECMWF apikey instead) in [#925](https://github.com/Deltares/dfm_tools/pull/925)
 
 ### Fix
 - also apply convert_360to180 to longitude variable in `dfmt.open_dataset_curvilinear()` in [#913](https://github.com/Deltares/dfm_tools/pull/913)

tests/test_download.py

@@ -8,21 +8,155 @@
 import os
 import pytest
 import pandas as pd
-from dfm_tools.download import cds_credentials, copernicusmarine_credentials
+import cdsapi
+from dfm_tools.download import (cds_credentials,
+                                cds_set_credentials,
+                                cds_set_credentials_rcfile,
+                                cds_remove_credentials_raise,
+                                copernicusmarine_credentials,
+                                )
 import dfm_tools as dfmt
 import xarray as xr
 import glob
 
 
+def get_cds_url_key():
+    try:
+        cds_url, cds_apikey, _ = cdsapi.api.get_url_key_verify(url=None, key=None, verify=None)
+    except Exception as e:
+        if "Missing/incomplete configuration file" in str(e):
+            cds_url = None
+            cds_apikey = None
+        else:
+            raise e
+
+    return cds_url, cds_apikey
+
+
+def set_cds_credentials_ifnot_none(cds_url, cds_apikey):
+    if None not in [cds_url, cds_apikey]:
+        cds_set_credentials(cds_url, cds_apikey)
+
+
 @pytest.mark.requiressecrets
 @pytest.mark.unittest
 def test_cds_credentials():
-    # check if the credentials are present on this system
-    val = cds_credentials()
-    assert not val
+    cds_credentials()
+
+
+@pytest.mark.requiressecrets
+@pytest.mark.unittest
+def test_cds_credentials_onlykey_envvars():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    # remove credentials envvars and file
+    with pytest.raises(ValueError):
+        cds_remove_credentials_raise()
 
-    # some platforms depend on environ for url/apikey, check if the default cdsapi_url is set after running cds_credentials()
-    assert "CDSAPI_URL" in os.environ.keys()
+    assert "CDSAPI_URL" not in os.environ.keys()
+    os.environ["CDSAPI_KEY"] = cds_apikey
+
+    cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+
+
+@pytest.mark.unittest
+def test_cds_credentials_newurl_incorrectkey_rcfile():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    # remove credentials envvars and file
+    with pytest.raises(ValueError):
+        cds_remove_credentials_raise()
+
+    cds_url_temp = "https://cds-beta.climate.copernicus.eu/api"
+    cds_apikey_temp = "INCORRECT-APIKEY"
+    cds_set_credentials_rcfile(cds_url_temp, cds_apikey_temp)
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Authentication failed" in str(e.value)
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)    
+
+
+@pytest.mark.unittest
+def test_cds_credentials_newurl_incorrectkey_envvars():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    os.environ["CDSAPI_URL"] = "https://cds-beta.climate.copernicus.eu/api"
+    os.environ["CDSAPI_KEY"] = "INCORRECT-APIKEY"
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Authentication failed" in str(e.value)
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)
+
+
+@pytest.mark.unittest
+def test_cds_credentials_oldurl_incorrectkey_rcfile():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    # remove credentials envvars and file
+    with pytest.raises(ValueError):
+        cds_remove_credentials_raise()
+
+    cds_url_temp = "https://cds.climate.copernicus.eu/api/v2"
+    cds_apikey_temp = "INCORRECT-APIKEY"
+    cds_set_credentials_rcfile(cds_url_temp, cds_apikey_temp)
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Authentication failed" in str(e.value) # should actually be "Old CDS URL found", but the url from the file is ignored, which is acceptable
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)
+
+
+@pytest.mark.unittest
+def test_cds_credentials_oldurl_incorrectkey_envvars():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    os.environ["CDSAPI_URL"] = "https://cds.climate.copernicus.eu/api/v2"
+    os.environ["CDSAPI_KEY"] = "INCORRECT-APIKEY"
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Old CDS URL found" in str(e.value)
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)
+
+
+@pytest.mark.unittest
+def test_cds_credentials_newurl_oldkey_rcfile():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    # remove credentials envvars and file
+    with pytest.raises(ValueError):
+        cds_remove_credentials_raise()
+
+    cds_url_temp = "https://cds-beta.climate.copernicus.eu/api"
+    cds_apikey_temp = "olduid:old-api-key"
+    cds_set_credentials_rcfile(cds_url_temp, cds_apikey_temp)
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Old CDS API-key found (with :)" in str(e.value)
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)
+
+
+@pytest.mark.unittest
+def test_cds_credentials_newurl_oldkey_envvars():
+    cds_url, cds_apikey = get_cds_url_key()
+
+    os.environ["CDSAPI_URL"] = "https://cds-beta.climate.copernicus.eu/api"
+    os.environ["CDSAPI_KEY"] = "olduid:old-api-key"
+
+    with pytest.raises(ValueError) as e:
+        cds_credentials()
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+    assert "Old CDS API-key found (with :)" in str(e.value)
+    assert "The CDS/ECMWF apikey environment variables and rcfile were deleted" in str(e.value)
 
 
 @pytest.mark.requiressecrets
@@ -40,9 +174,16 @@ def test_download_era5(file_nc_era5_pattern):
     assert len(file_list) == 2
 
     ds = xr.open_mfdataset(file_nc_era5_pattern)
-    assert ds.sizes["time"] == 1416
-    assert ds.time.to_pandas().iloc[0] == pd.Timestamp('2010-01-01')
-    assert ds.time.to_pandas().iloc[-1] == pd.Timestamp('2010-02-28 23:00')
+    timedim = 'time'
+    # datasets retrieved with new cds-beta have valid_time instead of time dimn/varn
+    # https://forum.ecmwf.int/t/new-time-format-in-era5-netcdf-files/3796/5?u=jelmer_veenstra
+    # TODO: can be removed after https://github.com/Unidata/netcdf4-python/issues/1357 or https://forum.ecmwf.int/t/3796 is fixed
+    if 'valid_time' in ds.dims: #TODO: can be removed if 
+        timedim = 'valid_time'
+
+    assert ds.sizes[timedim] == 1416
+    assert ds[timedim].to_pandas().iloc[0] == pd.Timestamp('2010-01-01')
+    assert ds[timedim].to_pandas().iloc[-1] == pd.Timestamp('2010-02-28 23:00')
 
 
 @pytest.mark.requiressecrets