Merge pull request #2674 from n3hrox/enhance/footnote_deduplication

logger: output footer once for multithreaded push/pull errors

dvc/logger.py

@@ -11,6 +11,17 @@ import logging.config
 import colorama
 
 
+FOOTER = (
+    "\n{yellow}Having any troubles?{nc}"
+    " Hit us up at {blue}https://dvc.org/support{nc},"
+    " we are always happy to help!"
+).format(
+    blue=colorama.Fore.BLUE,
+    nc=colorama.Fore.RESET,
+    yellow=colorama.Fore.YELLOW,
+)
+
+
 class LoggingException(Exception):
     def __init__(self, record):
         msg = "failed to log {}".format(str(record))
@@ -48,16 +59,6 @@ class ColorFormatter(logging.Formatter):
         "CRITICAL": colorama.Fore.RED,
     }
 
-    footer = (
-        "{yellow}Having any troubles?{nc}"
-        " Hit us up at {blue}https://dvc.org/support{nc},"
-        " we are always happy to help!"
-    ).format(
-        blue=colorama.Fore.BLUE,
-        nc=colorama.Fore.RESET,
-        yellow=colorama.Fore.YELLOW,
-    )
-
     def format(self, record):
         if record.levelname == "INFO":
             return record.msg
@@ -66,10 +67,7 @@ class ColorFormatter(logging.Formatter):
             exception, stack_trace = self._parse_exc(record.exc_info)
 
             return (
-                "{color}{levelname}{nc}: {description}"
-                "{stack_trace}\n"
-                "\n"
-                "{footer}"
+                "{color}{levelname}{nc}: {description}" "{stack_trace}\n"
             ).format(
                 color=self.color_code.get(record.levelname, ""),
                 nc=colorama.Fore.RESET,
@@ -77,7 +75,6 @@ class ColorFormatter(logging.Formatter):
                 description=self._description(record.msg, exception),
                 msg=record.msg,
                 stack_trace=stack_trace,
-                footer=self.footer,
             )
 
         return "{color}{levelname}{nc}: {msg}".format(

dvc/main.py

@@ -6,6 +6,7 @@ import logging
 
 from dvc.cli import parse_args
 from dvc.lock import LockError
+from dvc.logger import FOOTER
 from dvc.config import ConfigError
 from dvc.analytics import Analytics
 from dvc.exceptions import NotDvcRepoError, DvcParserError
@@ -75,6 +76,9 @@ def main(argv=None):
         # so won't be reused by any other subsequent run anyway.
         clean_repos()
 
+    if ret != 0:
+        logger.info(FOOTER)
+
     Analytics().send_cmd(cmd, args, ret)
 
     return ret

tests/func/test_remote.py

@@ -245,5 +245,5 @@ def test_partial_push_n_pull(dvc_repo, repo_dir, caplog):
 
 
 def get_last_exc(caplog):
-    _, exc, _ = caplog.records[-1].exc_info
+    _, exc, _ = caplog.records[-2].exc_info
     return exc

tests/unit/test_logger.py

@@ -45,11 +45,7 @@ class TestColorFormatter:
         with caplog.at_level(logging.INFO, logger="dvc"):
             logger.error("message")
 
-            expected = (
-                "{red}ERROR{nc}: message\n"
-                "\n"
-                "{footer}".format(footer=formatter.footer, **colors)
-            )
+            expected = "{red}ERROR{nc}: message\n".format(**colors)
 
             assert expected == formatter.format(caplog.records[0])
 
@@ -60,11 +56,7 @@ class TestColorFormatter:
             except Exception:
                 logger.exception("message")
 
-            expected = (
-                "{red}ERROR{nc}: message\n"
-                "\n"
-                "{footer}".format(footer=formatter.footer, **colors)
-            )
+            expected = "{red}ERROR{nc}: message\n".format(**colors)
 
             assert expected == formatter.format(caplog.records[0])
 
@@ -75,11 +67,7 @@ class TestColorFormatter:
             except Exception:
                 logger.exception("")
 
-            expected = (
-                "{red}ERROR{nc}: description\n"
-                "\n"
-                "{footer}".format(footer=formatter.footer, **colors)
-            )
+            expected = "{red}ERROR{nc}: description\n".format(**colors)
 
             assert expected == formatter.format(caplog.records[0])
 
@@ -90,10 +78,8 @@ class TestColorFormatter:
             except Exception:
                 logger.exception("message")
 
-            expected = (
-                "{red}ERROR{nc}: message - description\n"
-                "\n"
-                "{footer}".format(footer=formatter.footer, **colors)
+            expected = "{red}ERROR{nc}: message - description\n".format(
+                **colors
             )
 
             assert expected == formatter.format(caplog.records[0])
@@ -110,13 +96,8 @@ class TestColorFormatter:
                 "{red}ERROR{nc}: description\n"
                 "{red}{line}{nc}\n"
                 "{stack_trace}"
-                "{red}{line}{nc}\n"
-                "\n"
-                "{footer}".format(
-                    footer=formatter.footer,
-                    line="-" * 60,
-                    stack_trace=stack_trace,
-                    **colors
+                "{red}{line}{nc}\n".format(
+                    line="-" * 60, stack_trace=stack_trace, **colors
                 )
             )
 
@@ -138,10 +119,7 @@ class TestColorFormatter:
                 "{red}ERROR{nc}: message - second: first\n"
                 "{red}{line}{nc}\n"
                 "{stack_trace}"
-                "{red}{line}{nc}\n"
-                "\n"
-                "{footer}".format(
-                    footer=formatter.footer,
+                "{red}{line}{nc}\n".format(
                     line="-" * 60,
                     stack_trace="\n".join([first_traceback, second_traceback]),
                     **colors