Repository: eWaterCycle/era5cli

Issue:
Store API key using the cli. Avoid overlap with Atmosphere Data Store key
Ask the user in the command line to fill in the URL and API Key, so that the .cdsapirc will be automatically generated.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/eWaterCycle/era5cli/pull/138?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eWaterCycle) Report
> Merging [#138](https://codecov.io/gh/eWaterCycle/era5cli/pull/138?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eWaterCycle) (fb595f8) into [main](https://codecov.io/gh/eWaterCycle/era5cli/commit/8fca8337a659e613d03cf6aeba2890c292b361e3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=eWaterCycle) (8fca833) will **decrease** coverage by `10.08%`.
> The diff coverage is `34.24%`.

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##             main     #138       +/-   ##
===========================================
- Coverage   99.24%   89.17%   -10.08%     
===========================================
  Files           7        8        +1     
  Lines         398      471       +73     
===========================================
+ Hits          395      420       +25     
- Misses          3       51       +48     
```

</details>
BSchilperoort: I had mistakenly commited the changes to add config --show to the other branch (split requests by month). I'll merge this and continue there 😅 

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 1601d09..3c45320 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,11 @@
 Release Notes
 *************
 
+Unreleased
+~~~~~~~~~~
+* Change CDS keys from .cdsapirc file to .config/eracli.txt file. This will avoid conflict with e.g. ADS.
+* Add validator for era5cli.txt keys. This should provide better feedback to users and reduce user error.
+
 1.3.2 (2021-12-13)
 ~~~~~~~~~~~~~~~~~~
 * Elaborate the range of years that can be queried `#123 <https://github.com/eWaterCycle/era5cli/pull/123>`_
diff --git a/docs/source/instructions.rst b/docs/source/instructions.rst
index 58e355d..4a1fb3d 100644
--- a/docs/source/instructions.rst
+++ b/docs/source/instructions.rst
@@ -11,45 +11,19 @@ Register at Copernicus Climate Data Service
 
 -  Copy your user ID and API key.
 
-Create a key ascii file
-~~~~~~~~~~~~~~~~~~~~~~~
-
-Linux
-#####
-In Linux create a new file called .cdsapirc in the home directory of your user and add the following two lines:
-
-::
-
-   url: https://cds.climate.copernicus.eu/api/v2
-
-   key: UID:KEY
-
-Replace UID with your user ID and KEY with your API key
-
-Windows
-#######
-In Windows create a new file called .cdsapirc (e.g. with Notepad) where in your windows environment, %USERPROFILE% is usually located at C:\Users\Username folder). And add the following two lines
+Configure era5cli to use your ID and key
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+To configure era5cli to use your ID and API key, do:
 ::
 
-   url: https://cds.climate.copernicus.eu/api/v2
-
-   key: UID:KEY
-
-Replace UID with your user ID and KEY with your API key
+   era5cli config --uid ID_NUMBER --key "KEY"
 
-MacOS
-#####
-In MacOS create a new file called .cdsapirc in the home directory of your user and add the following two lines:
-
-
-::
 
-   url: https://cds.climate.copernicus.eu/api/v2
+Where ID_NUMBER is your user ID (e.g. ``123456``) and "KEY" is your API key, inside double quotes (e.g. ``"4s215sgs-2dfa-6h34-62h2-1615ad163414"``).
 
-   key: UID:KEY
+After running this command your ID and key are validated and stored inside your home folder, under ``.config/era5cli.txt``. You can also edit this file manually.
 
-Replace UID with your user ID and KEY with your API key
 
 Info on available variables
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~
diff --git a/era5cli/cli.py b/era5cli/cli.py
index f004eaf..f1a0ed7 100644
--- a/era5cli/cli.py
+++ b/era5cli/cli.py
@@ -8,6 +8,7 @@ from datetime import datetime
 import era5cli.fetch as efetch
 import era5cli.info as einfo
 import era5cli.inputref as ref
+from era5cli import key_management
 
 
 def _level_parse(level):
@@ -403,6 +404,65 @@ def _build_parser():
         ),
     )
 
+    config = subparsers.add_parser(
+        "config",
+        description="",
+        prog=textwrap.dedent(
+            """
+            Configure the CDS login info for era5cli.
+
+            This will create a config file in your home directory, in folder named
+            ".config". The CDS URL, your UID and the CDS keys will be stored here.
+
+            To find your key and UID, go to https://cds.climate.copernicus.eu/ and login
+            with your email and password. Then go to your user profile (top right).
+
+            Use `era5cli config --help` for more information.
+            """
+        ),
+        help=textwrap.dedent(
+            """
+            Configure the CDS login info for era5cli.
+
+            """
+        ),
+        formatter_class=argparse.RawTextHelpFormatter,
+    )
+
+    config.add_argument(
+        "--uid",
+        type=str,
+        required=True,
+        help=textwrap.dedent(
+            """
+            Your CDS User ID, e.g.: 123456
+            """
+        ),
+    )
+
+    config.add_argument(
+        "--key",
+        type=str,
+        required=True,
+        help=textwrap.dedent(
+            """
+            Your CDS key, e.g.: "4s215sgs-2dfa-6h34-62h2-1615ad163414"
+            """
+        ),
+    )
+
+    config.add_argument(
+        "--url",
+        type=str,
+        required=False,
+        default=key_management.DEFAULT_CDS_URL,
+        help=textwrap.dedent(
+            f"""
+            (optional) URL to the CDS, by default: {key_management.DEFAULT_CDS_URL}
+            """
+        ),
+    )
+
     return parser
 
 
@@ -481,6 +541,13 @@ def _execute(args):
     if args.command == "info":
         return _run_info(args)
 
+    if args.command == "config":
+        return key_management.run_config(
+            url=args.url,
+            uid=args.uid,
+            key=args.key,
+        )
+
     # the fetching subroutines
     years = _construct_year_list(args)
     synoptic, statistics, days, hours = _set_period_args(args)
diff --git a/era5cli/fetch.py b/era5cli/fetch.py
index ee8544c..0353d44 100644
--- a/era5cli/fetch.py
+++ b/era5cli/fetch.py
@@ -6,6 +6,7 @@ import cdsapi
 from pathos.threading import ThreadPool as Pool
 import era5cli.inputref as ref
 import era5cli.utils
+from era5cli import key_management
 
 
 class Fetch:
@@ -105,6 +106,8 @@ class Fetch:
         land=False,
     ):
         """Initialization of Fetch class."""
+        self._get_login()  # Get login info from config file.
+
         self.months = era5cli.utils._zpad_months(months)
         """list(str): List of zero-padded strings of months
         (e.g. ['01', '02',..., '12'])."""
@@ -158,6 +161,9 @@ class Fetch:
         """bool: Whether to download from the ERA5-Land
         dataset."""
 
+    def _get_login(self):
+        self.url, self.key = key_management.load_era5cli_config()
+
     def fetch(self, dryrun=False):
         """Split calls and fetch results.
 
@@ -447,7 +453,7 @@ class Fetch:
                 "please do not kill this process in the meantime.",
                 os.linesep,
             )
-            connection = cdsapi.Client()
+            connection = cdsapi.Client(url=self.url, key=self.key)
             print("".join(queueing_message))  # print queueing message
             connection.retrieve(name, request, outputfile)
             era5cli.utils.append_history(name, request, outputfile)
diff --git a/era5cli/key_management.py b/era5cli/key_management.py
new file mode 100644
index 0000000..c97279e
--- /dev/null
+++ b/era5cli/key_management.py
@@ -0,0 +1,173 @@
+import os
+from pathlib import Path
+from typing import Tuple
+import cdsapi
+from requests.exceptions import ConnectionError  # pylint: disable=redefined-builtin
+
+
+ERA5CLI_CONFIG_PATH = Path.home() / ".config" / "era5cli.txt"
+CDSAPI_CONFIG_PATH = Path.home() / ".cdsapirc"
+DEFAULT_CDS_URL = "https://cds.climate.copernicus.eu/api/v2"
+
+AUTH_ERR_MSG = "401 Authorization Required"
+NO_DATA_ERR_MSG = "There is no data matching your request"
+
+
+class InvalidRequestError(Exception):
+    "Raised when an invalid request error is given by the cdsapi."
+
+
+class InvalidLoginError(Exception):
+    "Raised when an invalid login is provided to the cds server."
+
+
+def attempt_cds_login(url: str, fullkey: str) -> True:
+    """Attempt to connect to the CDS, to validate the URL and UID + key.
+
+    Args:
+        url: URL to the CDS API.
+        fullkey: Combination of your UID and key, separated with a colon.
+
+    Raises:
+        ConnectionError: If no connection to the CDS could be made.
+        InvalidLoginError: If an invalid authetication was provided to the CDS.
+        InvalidRequestError: If the test request failed, likely due to changes in the
+            CDS API's variable naming.
+
+    Returns:
+        True if a connection was made succesfully.
+    """
+    connection = cdsapi.Client(
+        url=url,
+        key=fullkey,
+        verify=True,
+        quiet=True,  # Supress output to the console from the test retrieve.
+    )
+
+    try:
+        # Check the URL
+        connection.status()  # pragma: no cover
+
+        # Checks if the authentication works, without downloading data
+        connection.retrieve(  # pragma: no cover
+            "reanalysis-era5-single-levels",
+            {
+                "variable": "2t",
+                "product_type": "reanalysis",
+                "date": "2012-12-01",
+                "time": "14:00",
+                "format": "netcdf",
+            },
+        )
+        return True
+
+    except ConnectionError as err:
+        raise ConnectionError(
+            f"{os.linesep}Failed to connect to CDS. Please check your internet "
+            "connection and/or the"
+            f" URL in the era5cli configuration: {ERA5CLI_CONFIG_PATH.resolve()}"
+            f"{os.linesep}Or redefine your configuration with 'era5cli config'"
+        ) from err
+
+    except Exception as err:
+        if AUTH_ERR_MSG in str(err):
+            raise InvalidLoginError(
+                f"{os.linesep}Authorization with the CDS served failed. Likely due to"
+                " an incorrect key or UID."
+                f"{os.linesep}Please check your era5cli configuration file: "
+                f"{ERA5CLI_CONFIG_PATH.resolve()}{os.linesep}"
+                "Or redefine your configuration with 'era5cli config'"
+            ) from err
+        if NO_DATA_ERR_MSG in str(err):
+            raise InvalidRequestError(
+                f"{os.linesep}Something changed in the CDS API. Please raise an issue "
+                "on https://www.github.com/eWaterCycle/era5cli"
+            ) from err
+        raise err  # pragma: no cover
+
+
+def run_config(
+    url: str,
+    uid: str,
+    key: str,
+) -> True:
+    """Check the user-input configuration. Entry point for the CLI."""
+    try:
+        attempt_cds_login(url, fullkey=f"{uid}:{key}")
+        write_era5cli_config(url, uid, key)
+        print(
+            f"Keys succesfully validated and stored in {ERA5CLI_CONFIG_PATH.resolve()}"
+        )
+        return True
+    except InvalidLoginError:
+        print(
+            "Error: the UID and key are rejected by the CDS. "
+            "Please check and try again."
+        )
+    return False
+
+
+def check_era5cli_config() -> None:
+    """Validate if the era5cli config exists, and can connect to the CDS.
+
+    If no era5cli config file exists, but a CDS api file exists in the default location,
+    This routine will attempt to use those keys, and ask the user if they want to use
+    these.
+    """
+    if ERA5CLI_CONFIG_PATH.exists():
+        url, fullkey = load_era5cli_config()
+        attempt_cds_login(url, fullkey)
+    else:
+        print("era5cli configuration file not found. Looking for CDSAPI key.")
+        if not valid_cdsapi_config():
+            raise InvalidLoginError(
+                "No valid CDS login found. Please configure your CDS login using: "
+                "'era5cli config'"
+            )
+
+
+def valid_cdsapi_config() -> bool:
+    """Validate the default cdsapirc file. Promts the user for using these for era5cli.
+
+    Returns:
+        True if a valid key has been found & written to file. Otherwise False.
+    """
+    if CDSAPI_CONFIG_PATH.exists():
+        url, fullkey = load_cdsapi_config()
+        try:
+            if attempt_cds_login(url, fullkey):
+                userinput = input(
+                    "Valid CDS keys found in the .cdsapirc file. Do you want to use "
+                    "these for era5cli? [Y/n]"
+                )
+                if userinput in ["Y", "y", "Yes", "yes"]:
+                    write_era5cli_config(
+                        url, uid=fullkey.split(":")[0], key=fullkey.split(":")[1]
+                    )
+                    return True
+        except (ConnectionError, InvalidLoginError, InvalidRequestError):
+            return False
+    return False
+
+
+def load_era5cli_config() -> Tuple[str, str]:
+    with open(ERA5CLI_CONFIG_PATH, encoding="utf8") as f:
+        url = f.readline().replace("url:", "").strip()
+        uid = f.readline().replace("uid:", "").strip()
+        key = f.readline().replace("key:", "").strip()
+    return url, f"{uid}:{key}"
+
+
+def write_era5cli_config(url: str, uid: str, key: str):
+    ERA5CLI_CONFIG_PATH.parent.mkdir(exist_ok=True)
+    with open(ERA5CLI_CONFIG_PATH, mode="w", encoding="utf-8") as f:
+        f.write(f"url: {url}\n")
+        f.write(f"uid: {uid}\n")
+        f.write(f"key: {key}\n")
+
+
+def load_cdsapi_config() -> Tuple[str, str]:
+    with open(CDSAPI_CONFIG_PATH, encoding="utf-8") as f:
+        url = f.readline().replace("url:", "").strip()
+        fullkey = f.readline().replace("key:", "").strip()
+    return url, fullkey


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index dcbece7..36bba12 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -4,6 +4,7 @@ import unittest.mock as mock
 import pytest
 import era5cli.cli as cli
 import era5cli.inputref as ref
+from era5cli import key_management
 
 
 def test_parse_args():
@@ -310,3 +311,39 @@ def test_main_info(info):
     argv = ["info", "total_precipitation"]
     args = cli._parse_args(argv)
     cli._execute(args)
+
+
+config_args = [
+    "config",
+    "--uid",
+    "123456",
+    "--key",
+    "abc-def",
+]
+
+
+def test_config_parse():
+    args = cli._parse_args(config_args)
+    assert args.uid == "123456"
+    assert args.key == "abc-def"
+
+
+@mock.patch("era5cli.key_management.attempt_cds_login", return_value=True)
+@mock.patch("era5cli.key_management.write_era5cli_config")
+def test_config_write(mock_a, mock_b):
+    """Assuming the CDS login is valid, see if the write function is called"""
+    args = cli._parse_args(config_args)
+    cli._execute(args)
+
+
+@mock.patch(
+    "era5cli.key_management.attempt_cds_login",
+    side_effect=key_management.InvalidLoginError,
+)
+@mock.patch("era5cli.key_management.write_era5cli_config")
+def test_config_invalid(mock_a, mock_b, capfd):
+    """Assume the CDS login is invalid, see if the right error is printed to console."""
+    args = cli._parse_args(config_args)
+    cli._execute(args)
+    out, _ = capfd.readouterr()
+    assert "Error: the UID and key are rejected" in out
diff --git a/tests/test_config.py b/tests/test_config.py
new file mode 100644
index 0000000..1b2a859
--- /dev/null
+++ b/tests/test_config.py
@@ -0,0 +1,136 @@
+from unittest.mock import patch
+import pytest
+import requests.exceptions as rex
+from era5cli import key_management
+
+
+@pytest.fixture(scope="function")
+def empty_path_era5(tmp_path_factory):
+    return tmp_path_factory.mktemp(".config") / "era5cli.txt"
+
+
+@pytest.fixture(scope="function")
+def valid_path_era5(tmp_path_factory):
+    fn = tmp_path_factory.mktemp(".config") / "era5cli.txt"
+    with open(fn, mode="w", encoding="utf-8") as f:
+        f.write("url: b\nuid: 123\nkey: abc-def\n")
+    return fn
+
+
+@pytest.fixture(scope="function")
+def empty_path_cds(tmp_path_factory):
+    return tmp_path_factory.mktemp(".config") / "cdsapirc.txt"
+
+
+@pytest.fixture(scope="function")
+def valid_path_cds(tmp_path_factory):
+    fn = tmp_path_factory.mktemp(".config") / "cdsapirc.txt"
+    with open(fn, mode="w", encoding="utf-8") as f:
+        f.write("url: a\nkey: 123:abc-def")
+    return fn
+
+
+class TestEra5CliConfig:
+    def test_load_era5cli_config(self, valid_path_era5):
+        with patch("era5cli.key_management.ERA5CLI_CONFIG_PATH", valid_path_era5):
+            assert key_management.load_era5cli_config() == ("b", "123:abc-def")
+
+    def test_check_era5cli_config(self, valid_path_era5):
+        mp1 = patch("era5cli.key_management.ERA5CLI_CONFIG_PATH", valid_path_era5)
+        mp2 = patch("era5cli.key_management.attempt_cds_login", return_value=True)
+        with mp1, mp2:
+            key_management.check_era5cli_config()
+
+
+class TestConfigCdsrc:
+    """Test the cases where a .cdsapirc file exists.
+
+    This leads to three options:
+        - File exists, the keys are valid, and user does not want to copy the keys.
+        - File exists, the keys are valid, and the users wants to copy the keys.
+        - File exists, and the keys are not valid.
+    """
+
+    def test_cdsrcfile_user_says_no(self, empty_path_era5, valid_path_cds):
+        """.cdsapirc exists. User says no. Should raise InvalidLoginError"""
+        mp1 = patch("builtins.input", return_value="N")
+        mp2 = patch("era5cli.key_management.attempt_cds_login", return_value=True)
+        mp3 = patch("era5cli.key_management.ERA5CLI_CONFIG_PATH", empty_path_era5)
+        mp4 = patch("era5cli.key_management.CDSAPI_CONFIG_PATH", valid_path_cds)
+        with mp1, mp2, mp3, mp4:
+            with pytest.raises(
+                key_management.InvalidLoginError, match="No valid CDS login found"
+            ):
+                key_management.check_era5cli_config()
+
+    def test_cdsrcfile_user_says_yes(self, empty_path_era5, valid_path_cds):
+        """.cdsapirc exists. User says yes. url+key is copied to the era5cli config."""
+        mp1 = patch("builtins.input", return_value="Y")
+        mp2 = patch("era5cli.key_management.attempt_cds_login", return_value=True)
+        mp3 = patch("era5cli.key_management.ERA5CLI_CONFIG_PATH", empty_path_era5)
+        mp4 = patch("era5cli.key_management.CDSAPI_CONFIG_PATH", valid_path_cds)
+        with mp1, mp2, mp3, mp4:
+            key_management.check_era5cli_config()
+            with open(empty_path_era5, "r", encoding="utf-8") as f:
+                assert f.readlines() == ["url: a\n", "uid: 123\n", "key: abc-def\n"]
+
+    def test_cdsrcfile_invalid_keys(self, empty_path_era5, valid_path_cds):
+        """.cdsapirc exists. url+key is validated, and is bad."""
+        mp1 = patch(
+            "era5cli.key_management.attempt_cds_login",
+            side_effect=key_management.InvalidLoginError,
+        )
+        mp2 = patch("era5cli.key_management.ERA5CLI_CONFIG_PATH", empty_path_era5)
+        mp3 = patch("era5cli.key_management.CDSAPI_CONFIG_PATH", valid_path_cds)
+        with mp1, mp2, mp3:
+            with pytest.raises(
+                key_management.InvalidLoginError, match="No valid CDS login found"
+            ):
+                key_management.check_era5cli_config()
+
+
+class TestAttemptCdsLogin:
+    """Test the keymanagement.attempt_cds_login function.
+
+    attempt_cds_login is monkeypatched elsewhere, these tests ensure it works as
+    expected.
+    """
+
+    def test_status_fail(self):
+        with patch("cdsapi.Client.status", side_effect=rex.ConnectionError):
+            with pytest.raises(rex.ConnectionError, match="Failed to connect to CDS"):
+                key_management.attempt_cds_login(url="test", fullkey="abc:def")
+
+    def test_connection_fail(self):
+        mp1 = patch("cdsapi.Client.status")
+        mp2 = patch(
+            "cdsapi.Client.retrieve",
+            side_effect=Exception("401 Authorization Required"),
+        )
+        with mp1, mp2:
+            with pytest.raises(
+                key_management.InvalidLoginError,
+                match="Authorization with the CDS served failed",
+            ):
+                key_management.attempt_cds_login(url="test", fullkey="abc:def")
+
+    def test_retrieve_fail(self):
+        mp1 = patch("cdsapi.Client.status")
+        mp2 = patch(
+            "cdsapi.Client.retrieve",
+            side_effect=Exception("There is no data matching your request"),
+        )
+        with mp1, mp2:
+            with pytest.raises(
+                key_management.InvalidRequestError,
+                match="Something changed in the CDS API",
+            ):
+                key_management.attempt_cds_login(url="test", fullkey="abc:def")
+
+    def test_all_pass(self):
+        mp1 = patch("cdsapi.Client.status")
+        mp2 = patch("cdsapi.Client.retrieve")
+        with mp1, mp2:
+            assert (
+                key_management.attempt_cds_login(url="test", fullkey="abc:def") is True
+            )
diff --git a/tests/test_fetch.py b/tests/test_fetch.py
index 8813c80..1534e37 100644
--- a/tests/test_fetch.py
+++ b/tests/test_fetch.py
@@ -40,29 +40,36 @@ def initialize(
     prelimbe=False,
     land=False,
 ):
-    """Initializer of the class."""
-    return fetch.Fetch(
-        years=years,
-        months=months,
-        days=days,
-        hours=hours,
-        area=area,
-        variables=variables,
-        outputformat=outputformat,
-        outputprefix="era5",
-        period=period,
-        ensemble=ensemble,
-        statistics=statistics,
-        synoptic=synoptic,
-        pressurelevels=pressurelevels,
-        merge=merge,
-        threads=threads,
-        prelimbe=prelimbe,
-        land=land,
-    )
-
-
-def test_init():
+    with mock.patch(
+        "era5cli.fetch.key_management.load_era5cli_config",
+        return_value=("url", "key:uid"),
+    ):
+        """Initializer of the class."""
+        return fetch.Fetch(
+            years=years,
+            months=months,
+            days=days,
+            hours=hours,
+            area=area,
+            variables=variables,
+            outputformat=outputformat,
+            outputprefix="era5",
+            period=period,
+            ensemble=ensemble,
+            statistics=statistics,
+            synoptic=synoptic,
+            pressurelevels=pressurelevels,
+            merge=merge,
+            threads=threads,
+            prelimbe=prelimbe,
+            land=land,
+        )
+
+
+@mock.patch(
+    "era5cli.fetch.key_management.load_era5cli_config", return_value=("url", "key:uid")
+)
+def test_init(load_era5cli_config):
     """Test init function of Fetch class."""
     era5 = fetch.Fetch(
         years=[2008, 2009],
diff --git a/tests/test_integration.py b/tests/test_integration.py
index 9db0d69..0dfcbdb 100644
--- a/tests/test_integration.py
+++ b/tests/test_integration.py
@@ -2,6 +2,7 @@
 
 import logging
 from textwrap import dedent
+from unittest import mock
 import pytest
 from era5cli.cli import main
 
@@ -136,7 +137,11 @@ def test_main(call_result, capsys, caplog):
     if "--dryrun" not in call:
         pytest.fail("call must be a dryrun")
     with caplog.at_level(logging.INFO):
-        main(call)
+        with mock.patch(
+            "era5cli.fetch.key_management.load_era5cli_config",
+            return_value=("url", "key:uid"),
+        ):
+            main(call)
     captured = capsys.readouterr().out
     assert result == captured
     try: