From 39431a689dab4b6c4ee07e0032399542c073bdcb Mon Sep 17 00:00:00 2001
From: veenstrajelmer <60435591+veenstrajelmer@users.noreply.github.com>
Date: Mon, 28 Oct 2024 20:24:03 +0100
Subject: [PATCH] Fix cds credentials (#1035)

* fixed cds_credentials in case of prompt, including testcase

* updated whatsnew

* resolved fixture marks warning

* updated minimal copernicusmarine version
---
 dfm_tools/download.py  |  6 ++++--
 docs/whats-new.md      |  3 +++
 pyproject.toml         |  6 +++---
 tests/conftest.py      |  1 -
 tests/test_download.py | 16 ++++++++++++++++
 5 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/dfm_tools/download.py b/dfm_tools/download.py
index 78294c1f..dc89fa03 100644
--- a/dfm_tools/download.py
+++ b/dfm_tools/download.py
@@ -96,11 +96,12 @@ 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
+    cds_url_default = "https://cds.climate.copernicus.eu/api"
     
+    # TODO: put this in a PR at https://github.com/ecmwf/cdsapi
     if "CDSAPI_KEY" in os.environ.keys():
         # in case of envvars, also set CDSAPI_URL envvar so it does not have to be supplied
-        cds_url = os.environ.get("CDSAPI_URL", "https://cds.climate.copernicus.eu/api")
+        cds_url = os.environ.get("CDSAPI_URL", cds_url_default)
         os.environ["CDSAPI_URL"] = cds_url
     
     try:
@@ -115,6 +116,7 @@ def cds_credentials():
                   "your API-key from https://cds.climate.copernicus.eu/profile "
                   "(first register, login and accept the terms). "
                   "More info in https://forum.ecmwf.int/t/3743.")
+            cds_url = cds_url_default
             cds_apikey = getpass.getpass("\nEnter your ECMWF API-key (string with dashes): ")
             cds_set_credentials(cds_url, cds_apikey)
         else:
diff --git a/docs/whats-new.md b/docs/whats-new.md
index 0c61bd8c..3dc31d26 100644
--- a/docs/whats-new.md
+++ b/docs/whats-new.md
@@ -2,6 +2,9 @@
 
 ## UNRELEASED
 
+### Fix
+- fixed CDS logging in for new users in `cds_credentials()` in [#1035](https://github.com/Deltares/dfm_tools/pull/1035)
+
 
 ## 0.30.0 (2024-10-20)
 
diff --git a/pyproject.toml b/pyproject.toml
index d138af07..11ced5d8 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -44,9 +44,9 @@ dependencies = [
 	"pydap>=3.4.0",
 	#erddapy>=2.0.0 supports pandas>=2.0.0
 	"erddapy>=2.0.0",
-	#copernicusmarine>=1.3.3 is the latest version and the developers recommend to always use the latest version until 2.0.0 is released: https://github.com/Deltares/dfm_tools/issues/936
-	"copernicusmarine>=1.3.3",
-	"copernicusmarine<2.0.0", # TODO: remove when dfm_tools is adjusted to not-yet-released 2.0 version: https://github.com/Deltares/dfm_tools/issues/933
+	#copernicusmarine>=1.3.4 is the latest version and the developers recommend to always use the latest version until 2.0.0 is released: https://github.com/Deltares/dfm_tools/issues/936
+	# TODO: remove when dfm_tools is adjusted to not-yet-released 2.0 version: https://github.com/Deltares/dfm_tools/issues/933
+	"copernicusmarine>=1.3.4,<2.0.0",
 	#rws-ddlpy>=0.6.0 `ddlpy.measurements_amount()` returns all amounts
 	"rws-ddlpy>=0.6.0",
 	#pooch>=1.1.0 has attribute retrieve
diff --git a/tests/conftest.py b/tests/conftest.py
index e1854308..b79101ca 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -11,7 +11,6 @@
 import pandas as pd
 
 
-@pytest.mark.requiressecrets
 @pytest.fixture
 def file_nc_era5_pattern(tmp_path):
     date_min = '2010-01-31'
diff --git a/tests/test_download.py b/tests/test_download.py
index 240e7c25..71b3bccf 100644
--- a/tests/test_download.py
+++ b/tests/test_download.py
@@ -20,6 +20,7 @@
 import xarray as xr
 import glob
 import numpy as np
+from unittest.mock import patch
 
 
 def get_cds_url_key():
@@ -46,6 +47,21 @@ def test_cds_credentials():
     cds_credentials()
 
 
+@patch("getpass.getpass")
+def test_cds_credentials_prompt(getpass):
+    # backup credentials and remove credentials envvars and file
+    cds_url, cds_apikey = get_cds_url_key()
+    with pytest.raises(ValueError):
+        cds_remove_credentials_raise()
+    
+    # provide apikey to cds_credentials() prompt
+    getpass.return_value = cds_apikey
+    cds_credentials()
+
+    # restore credentials file/envvars
+    set_cds_credentials_ifnot_none(cds_url, cds_apikey)
+
+
 @pytest.mark.requiressecrets
 @pytest.mark.unittest
 def test_cds_credentials_onlykey_envvars():