Merge pull request #20 from virtualtam/cli/config/error-msg

cli: improve exception handling for erroneous configuration

Author: virtualtam

docs/changelog.rst

@@ -27,6 +27,7 @@ The format is based on `Keep a Changelog`_ and this project adheres to
   * rename ``--output`` to ``--format``
   * default to 'pprint' output format
   * improve endpoint-specific parser argument generation
+  * improve exception handling and logging
 
 
 `v0.1.0 <https://github.com/shaarli/python-shaarli-client/releases/tag/v0.1.0>`_ - 2017-03-12

shaarli_client/config.py

@@ -0,0 +1,54 @@
+"""Configuration file utilities"""
+import logging
+import os
+from configparser import ConfigParser
+from pathlib import Path
+
+
+class InvalidConfiguration(Exception):
+    """Raised when invalid/no configuration is found"""
+
+    def __init__(self, message):
+        """Custom exception message"""
+        super(InvalidConfiguration, self).__init__(
+            "Invalid configuration: %s" % message
+        )
+
+
+def get_credentials(args):
+    """Retrieve Shaarli authentication information"""
+    if args.url and args.secret:
+        # credentials passed as CLI arguments
+        logging.warning("Passing credentials as arguments is unsafe"
+                        " and should be used for debugging only")
+        return (args.url, args.secret)
+
+    config = ConfigParser()
+
+    if args.instance:
+        instance = 'shaarli:{}'.format(args.instance)
+    else:
+        instance = 'shaarli'
+
+    if args.config:
+        # user-specified configuration file
+        config_files = config.read([args.config])
+    else:
+        # load configuration from a list of possible locations
+        home = Path(os.path.expanduser('~'))
+        config_files = config.read([
+            str(home / '.config' / 'shaarli' / 'client.ini'),
+            str(home / '.shaarli_client.ini'),
+            'shaarli_client.ini'
+        ])
+
+    if not config_files:
+        raise InvalidConfiguration("No configuration file found")
+
+    logging.info("Reading configuration from: %s",
+                 ', '.join([str(cfg) for cfg in config_files]))
+
+    try:
+        return (config[instance]['url'], config[instance]['secret'])
+    except KeyError as exc:
+        raise InvalidConfiguration("Missing entry: %s" % exc)

shaarli_client/main.py

@@ -1,13 +1,11 @@
 """shaarli-client main CLI entrypoint"""
 import json
 import logging
-import os
 import sys
 from argparse import ArgumentParser
-from configparser import ConfigParser
-from pathlib import Path
 
 from .client import ShaarliV1Client
+from .config import InvalidConfiguration, get_credentials
 from .utils import generate_all_endpoints_parsers
 
 
@@ -53,7 +51,9 @@ def main():
     try:
         url, secret = get_credentials(args)
         response = ShaarliV1Client(url, secret).request(args)
-
+    except InvalidConfiguration as exc:
+        logging.error(exc)
+        sys.exit(1)
     except (KeyError, TypeError, ValueError) as exc:
         logging.error(exc)
         parser.print_help()
@@ -65,37 +65,3 @@ def main():
         print(json.dumps(response.json(), sort_keys=True, indent=4))
     elif args.format == 'text':
         print(response.text)
-
-
-def get_credentials(args):
-    """Retrieve Shaarli authentication information"""
-    if args.url and args.secret:
-        # credentials passed as CLI arguments
-        logging.warning("Passing credentials as arguments is unsafe"
-                        " and should be used for debugging only")
-        return (args.url, args.secret)
-
-    config = ConfigParser()
-
-    if args.instance:
-        instance = 'shaarli:{}'.format(args.instance)
-    else:
-        instance = 'shaarli'
-
-    if args.config:
-        # user-specified configuration file
-        logging.info("Reading configuration from: %s", args.config)
-        with open(args.config, 'r') as f_config:
-            config.read_file(f_config)
-    else:
-        # load configuration from a list of possible locations
-        home = Path(os.path.expanduser('~'))
-        config_files = config.read([
-            home / '.config' / 'shaarli' / 'client.ini',
-            home / '.shaarli_client.ini',
-            'shaarli_client.ini'
-        ])
-        logging.info("Reading configuration from: %s",
-                     ', '.join([str(cfg) for cfg in config_files]))
-
-    return (config[instance]['url'], config[instance]['secret'])

tests/test_config.py

@@ -0,0 +1,126 @@
+"""Tests for Shaarli configuration utilities"""
+# pylint: disable=invalid-name,redefined-outer-name
+from argparse import Namespace
+from configparser import ConfigParser
+
+import pytest
+
+from shaarli_client.config import InvalidConfiguration, get_credentials
+
+SHAARLI_URL = 'http://shaar.li'
+SHAARLI_SECRET = 's3kr37'
+
+
+@pytest.fixture(scope='session')
+def shaarli_config(tmpdir_factory):
+    """Generate a client configuration file"""
+    config = ConfigParser()
+    config['shaarli'] = {
+        'url': SHAARLI_URL,
+        'secret': SHAARLI_SECRET
+    }
+    config['shaarli:shaaplin'] = {
+        'url': SHAARLI_URL,
+        'secret': SHAARLI_SECRET
+    }
+    config['shaarli:nourl'] = {
+        'secret': SHAARLI_SECRET
+    }
+    config['shaarli:nosecret'] = {
+        'url': SHAARLI_URL,
+    }
+
+    config_path = tmpdir_factory.mktemp('config').join('shaarli_client.ini')
+
+    with config_path.open('w') as f_config:
+        config.write(f_config)
+
+    return config_path
+
+
+def test_get_credentials_from_cli():
+    """Get authentication information as CLI parameters"""
+    url, secret = get_credentials(
+        Namespace(url=SHAARLI_URL, secret=SHAARLI_SECRET)
+    )
+    assert url == SHAARLI_URL
+    assert secret == SHAARLI_SECRET
+
+
+@pytest.mark.parametrize('instance', [None, 'shaaplin'])
+def test_get_credentials_from_config(tmpdir, shaarli_config, instance):
+    """Read credentials from a standard location"""
+    with tmpdir.as_cwd():
+        shaarli_config.copy(tmpdir.join('shaarli_client.ini'))
+
+        url, secret = get_credentials(
+            Namespace(
+                config=None,
+                instance=instance,
+                url=None,
+                secret=None
+            )
+        )
+
+    assert url == SHAARLI_URL
+    assert secret == SHAARLI_SECRET
+
+
+@pytest.mark.parametrize('instance', [None, 'shaaplin'])
+def test_get_credentials_from_userconfig(shaarli_config, instance):
+    """Read credentials from a user-provided configuration file"""
+    url, secret = get_credentials(
+        Namespace(
+            config=str(shaarli_config),
+            instance=instance,
+            url=None,
+            secret=None
+        )
+    )
+    assert url == SHAARLI_URL
+    assert secret == SHAARLI_SECRET
+
+
+def test_get_credentials_no_config(monkeypatch):
+    """No configuration file found"""
+    monkeypatch.setattr(ConfigParser, 'read', lambda x, y: [])
+
+    with pytest.raises(InvalidConfiguration) as exc:
+        get_credentials(
+            Namespace(
+                config=None,
+                instance=None,
+                url=None,
+                secret=None
+            )
+        )
+    assert "No configuration file found" in str(exc.value)
+
+
+def test_get_credentials_missing_section(shaarli_config):
+    """The specified instance has no configuration section"""
+    with pytest.raises(InvalidConfiguration) as exc:
+        get_credentials(
+            Namespace(
+                config=str(shaarli_config),
+                instance='nonexistent',
+                url=None,
+                secret=None
+            )
+        )
+    assert "Missing entry: 'shaarli:nonexistent'" in str(exc.value)
+
+
+@pytest.mark.parametrize('attribute', ['url', 'secret'])
+def test_get_credentials_missing_attribute(shaarli_config, attribute):
+    """The specified instance has no configuration section"""
+    with pytest.raises(InvalidConfiguration) as exc:
+        get_credentials(
+            Namespace(
+                config=str(shaarli_config),
+                instance='no{}'.format(attribute),
+                url=None,
+                secret=None
+            )
+        )
+    assert "Missing entry: '{}'".format(attribute) in str(exc.value)