From b586aa00489b3beef4ce7b2991105f89cc42a752 Mon Sep 17 00:00:00 2001
From: Sorin Sbarnea <ssbarnea@redhat.com>
Date: Sat, 19 Sep 2020 16:05:04 +0100
Subject: [PATCH] Improved config loading

---
 gri/__main__.py | 23 +++++++++++------------
 gri/gerrit.py   | 29 +++++++++++++++++++++--------
 gri/review.py   |  2 +-
 tox.ini         |  6 ++++--
 4 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/gri/__main__.py b/gri/__main__.py
index 94bfc22..d6b6364 100755
--- a/gri/__main__.py
+++ b/gri/__main__.py
@@ -32,7 +32,7 @@
 term = Console(theme=theme, highlighter=rich.highlighter.ReprHighlighter(), record=True)
 CFG_FILE = "~/.gertty.yaml"
 
-LOG = logging.getLogger(__name__)
+LOG = logging.getLogger(__package__)
 
 
 class Config(dict):
@@ -55,10 +55,10 @@ def load_config(config_file):
 class App:
     def __init__(self, ctx):
         self.ctx = ctx
-        self.cfg = Config(file=ctx.params['config'])
+        self.cfg = Config(file=ctx.params["config"])
         self.servers = []
-        self.user = ctx.params['user']
-        server = ctx.params['server']
+        self.user = ctx.params["user"]
+        server = ctx.params["server"]
         for srv in (
             self.cfg["servers"]
             if server is None
@@ -149,9 +149,8 @@ def get_command(self, ctx, cmd_name):
 )
 @click.option("--user", "-u", default="self", help="Query another user than self")
 @click.option(
-    "--config",
-    default=CFG_FILE,
-    help=f"Config file to use, defaults to {CFG_FILE}")
+    "--config", default=CFG_FILE, help=f"Config file to use, defaults to {CFG_FILE}"
+)
 @click.option(
     "--server",
     "-s",
@@ -180,11 +179,11 @@ def cli(ctx, **kwargs):
     LOG.addHandler(handler)
 
     LOG.warning("Called with %s", ctx.params)
-    if ctx.params['debug']:
+    if ctx.params["debug"]:
         LOG.setLevel(level=logging.DEBUG)
 
-    if " " in ctx.params['user']:
-        ctx.params['user'] = f"\"{ctx.params['user']}\""
+    if " " in ctx.params["user"]:
+        ctx.params["user"] = f"\"{ctx.params['user']}\""
 
     # import pdb
     # pdb.set_trace()
@@ -194,8 +193,8 @@ def cli(ctx, **kwargs):
         LOG.info("I was invoked without subcommand, assuming implicit `owned` command")
         ctx.invoke(owned)
 
-    if ctx.params['output']:
-        term.save_html(path=ctx.params['output'], theme=TERMINAL_THEME)
+    if ctx.params["output"]:
+        term.save_html(path=ctx.params["output"], theme=TERMINAL_THEME)
 
 
 @cli.resultcallback()
diff --git a/gri/gerrit.py b/gri/gerrit.py
index 376d5ec..0715d29 100644
--- a/gri/gerrit.py
+++ b/gri/gerrit.py
@@ -1,9 +1,12 @@
 import json
+import logging
 import netrc
 import os
+import sys
 
 import requests
 from requests.auth import HTTPBasicAuth, HTTPDigestAuth
+from requests.exceptions import HTTPError
 
 try:
     from urllib.parse import urlencode, urlparse
@@ -16,6 +19,7 @@
     "https://code.engineering.redhat.com/gerrit/": {"auth": HTTPDigestAuth},
     "verify": False,
 }
+LOG = logging.getLogger(__package__)
 
 
 # pylint: disable=too-few-public-methods
@@ -39,17 +43,22 @@ def __init__(self, url, name=None):
         # workaround for netrc error: OSError("Could not find .netrc: $HOME is not set")
         if "HOME" not in os.environ:
             os.environ["HOME"] = os.path.expanduser("~")
+        netrc_file = os.path.expanduser("~/.netrc")
 
-        token = netrc.netrc().authenticators(parsed_uri.netloc)
-
-        # saving username (may be needed later)
-        self.username = token[0]
+        try:
+            token = netrc.netrc().authenticators(parsed_uri.netloc)
+        except FileNotFoundError:
+            token = None
 
         if not token:
-            raise SystemError(
-                f"Unable to load credentials for {url} from ~/.netrc file"
+            LOG.error(
+                "Unable to load credentials for %s from %s file, "
+                "likely to receive 401 errors later.",
+                url,
+                netrc_file,
             )
-        self.__session.auth = self.auth_class(token[0], token[2])
+        else:
+            self.__session.auth = self.auth_class(token[0], token[2])
 
         self.__session.headers.update(
             {
@@ -71,7 +80,11 @@ def query(self, query=None):
 
     @staticmethod
     def parsed(result):
-        result.raise_for_status()
+        try:
+            result.raise_for_status()
+        except HTTPError as exc:
+            LOG.error(exc)
+            sys.exit(2)
 
         if hasattr(result, "text") and result.text[:4] == ")]}'":
             return json.loads(result.text[5:])
diff --git a/gri/review.py b/gri/review.py
index 4a0eeb1..33c0953 100644
--- a/gri/review.py
+++ b/gri/review.py
@@ -5,7 +5,7 @@
 
 from gri.console import link
 
-LOG = logging.getLogger(__name__)
+LOG = logging.getLogger(__package__)
 
 
 class Review:
diff --git a/tox.ini b/tox.ini
index 5b24f3d..6a9b3bf 100644
--- a/tox.ini
+++ b/tox.ini
@@ -9,8 +9,9 @@ description = run the tests with pytest under {basepython}
 setenv =
     PIP_DISABLE_PIP_VERSION_CHECK = 1
     VIRTUALENV_NO_DOWNLOAD = 1
+    # isolate testing to avoid using developer config files:
+    HOME = {toxworkdir}
 passenv =
-    HOME
     PYTEST_*
     PYTHONHTTPSVERIFY
     REQUESTS_CA_BUNDLE
@@ -23,7 +24,8 @@ deps =
     pip == 19.1.1
 whitelist_externals = bash
 commands =
-    gri --config {toxinidir}/test/.gertty.yaml -o report.html owned incoming merged abandon draft watched
+    gri --help
+    -gri -o report.html owned incoming merged abandon draft watched
 
 [testenv:lint]
 basepython = python3.7