Merge pull request #2288 from dnephin/warning_about_vars

Add console color to warnings and errors so they are more obvious

compose/cli/formatter.py

@@ -1,10 +1,13 @@
 from __future__ import absolute_import
 from __future__ import unicode_literals
 
+import logging
 import os
 
 import texttable
 
+from compose.cli import colors
+
 
 def get_tty_width():
     tty_size = os.popen('stty size', 'r').read().split()
@@ -15,6 +18,7 @@ def get_tty_width():
 
 
 class Formatter(object):
+    """Format tabular data for printing."""
     def table(self, headers, rows):
         table = texttable.Texttable(max_width=get_tty_width())
         table.set_cols_dtype(['t' for h in headers])
@@ -23,3 +27,22 @@ class Formatter(object):
         table.set_chars(['-', '|', '+', '-'])
 
         return table.draw()
+
+
+class ConsoleWarningFormatter(logging.Formatter):
+    """A logging.Formatter which prints WARNING and ERROR messages with
+    a prefix of the log level colored appropriate for the log level.
+    """
+
+    def get_level_message(self, record):
+        separator = ': '
+        if record.levelno == logging.WARNING:
+            return colors.yellow(record.levelname) + separator
+        if record.levelno == logging.ERROR:
+            return colors.red(record.levelname) + separator
+
+        return ''
+
+    def format(self, record):
+        message = super(ConsoleWarningFormatter, self).format(record)
+        return self.get_level_message(record) + message

compose/cli/main.py

@@ -28,6 +28,7 @@ from .command import project_from_options
 from .docopt_command import DocoptCommand
 from .docopt_command import NoSuchCommand
 from .errors import UserError
+from .formatter import ConsoleWarningFormatter
 from .formatter import Formatter
 from .log_printer import LogPrinter
 from .utils import get_version_info
@@ -41,7 +42,7 @@ log = logging.getLogger(__name__)
 console_handler = logging.StreamHandler(sys.stderr)
 
 INSECURE_SSL_WARNING = """
-Warning: --allow-insecure-ssl is deprecated and has no effect.
+--allow-insecure-ssl is deprecated and has no effect.
 It will be removed in a future version of Compose.
 """
 
@@ -58,9 +59,8 @@ def main():
         log.error(e.msg)
         sys.exit(1)
     except NoSuchCommand as e:
-        log.error("No such command: %s", e.command)
-        log.error("")
-        log.error("\n".join(parse_doc_section("commands:", getdoc(e.supercommand))))
+        commands = "\n".join(parse_doc_section("commands:", getdoc(e.supercommand)))
+        log.error("No such command: %s\n\n%s", e.command, commands)
         sys.exit(1)
     except APIError as e:
         log.error(e.explanation)
@@ -91,13 +91,18 @@ def setup_logging():
     logging.getLogger("requests").propagate = False
 
 
-def setup_console_handler(verbose):
-    if verbose:
-        console_handler.setFormatter(logging.Formatter('%(name)s.%(funcName)s: %(message)s'))
-        console_handler.setLevel(logging.DEBUG)
+def setup_console_handler(handler, verbose):
+    if handler.stream.isatty():
+        format_class = ConsoleWarningFormatter
     else:
-        console_handler.setFormatter(logging.Formatter())
-        console_handler.setLevel(logging.INFO)
+        format_class = logging.Formatter
+
+    if verbose:
+        handler.setFormatter(format_class('%(name)s.%(funcName)s: %(message)s'))
+        handler.setLevel(logging.DEBUG)
+    else:
+        handler.setFormatter(format_class())
+        handler.setLevel(logging.INFO)
 
 
 # stolen from docopt master
@@ -153,7 +158,7 @@ class TopLevelCommand(DocoptCommand):
         return options
 
     def perform_command(self, options, handler, command_options):
-        setup_console_handler(options.get('--verbose'))
+        setup_console_handler(console_handler, options.get('--verbose'))
 
         if options['COMMAND'] in ('help', 'version'):
             # Skip looking up the compose file.

compose/config/interpolation.py

@@ -78,7 +78,7 @@ class BlankDefaultDict(dict):
         except KeyError:
             if key not in self.missing_keys:
                 log.warn(
-                    "The {} variable is not set. Substituting a blank string."
+                    "The {} variable is not set. Defaulting to a blank string."
                     .format(key)
                 )
                 self.missing_keys.append(key)

compose/config/validation.py

@@ -57,9 +57,11 @@ def format_boolean_in_environment(instance):
     """
     if isinstance(instance, bool):
         log.warn(
-            "Warning: There is a boolean value in the 'environment' key.\n"
-            "Environment variables can only be strings.\nPlease add quotes to any boolean values to make them string "
-            "(eg, 'True', 'yes', 'N').\nThis warning will become an error in a future release. \r\n"
+            "There is a boolean value in the 'environment' key.\n"
+            "Environment variables can only be strings.\n"
+            "Please add quotes to any boolean values to make them string "
+            "(eg, 'True', 'yes', 'N').\n"
+            "This warning will become an error in a future release. \r\n"
         )
     return True
 

compose/service.py

@@ -862,7 +862,7 @@ class ServiceNet(object):
         if containers:
             return 'container:' + containers[0].id
 
-        log.warn("Warning: Service %s is trying to use reuse the network stack "
+        log.warn("Service %s is trying to use reuse the network stack "
                  "of another service that is not running." % (self.id))
         return None
 

tests/unit/cli/formatter_test.py

@@ -0,0 +1,35 @@
+from __future__ import absolute_import
+from __future__ import unicode_literals
+
+import logging
+
+from compose.cli import colors
+from compose.cli.formatter import ConsoleWarningFormatter
+from tests import unittest
+
+
+MESSAGE = 'this is the message'
+
+
+def makeLogRecord(level):
+    return logging.LogRecord('name', level, 'pathame', 0, MESSAGE, (), None)
+
+
+class ConsoleWarningFormatterTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.formatter = ConsoleWarningFormatter()
+
+    def test_format_warn(self):
+        output = self.formatter.format(makeLogRecord(logging.WARN))
+        expected = colors.yellow('WARNING') + ': '
+        assert output == expected + MESSAGE
+
+    def test_format_error(self):
+        output = self.formatter.format(makeLogRecord(logging.ERROR))
+        expected = colors.red('ERROR') + ': '
+        assert output == expected + MESSAGE
+
+    def test_format_info(self):
+        output = self.formatter.format(makeLogRecord(logging.INFO))
+        assert output == MESSAGE

tests/unit/cli/main_test.py

@@ -1,11 +1,15 @@
 from __future__ import absolute_import
 
+import logging
+
 from compose import container
 from compose.cli.errors import UserError
+from compose.cli.formatter import ConsoleWarningFormatter
 from compose.cli.log_printer import LogPrinter
 from compose.cli.main import attach_to_logs
 from compose.cli.main import build_log_printer
 from compose.cli.main import convergence_strategy_from_opts
+from compose.cli.main import setup_console_handler
 from compose.project import Project
 from compose.service import ConvergenceStrategy
 from tests import mock
@@ -60,6 +64,31 @@ class CLIMainTestCase(unittest.TestCase):
             timeout=timeout)
 
 
+class SetupConsoleHandlerTestCase(unittest.TestCase):
+
+    def setUp(self):
+        self.stream = mock.Mock()
+        self.stream.isatty.return_value = True
+        self.handler = logging.StreamHandler(stream=self.stream)
+
+    def test_with_tty_verbose(self):
+        setup_console_handler(self.handler, True)
+        assert type(self.handler.formatter) == ConsoleWarningFormatter
+        assert '%(name)s' in self.handler.formatter._fmt
+        assert '%(funcName)s' in self.handler.formatter._fmt
+
+    def test_with_tty_not_verbose(self):
+        setup_console_handler(self.handler, False)
+        assert type(self.handler.formatter) == ConsoleWarningFormatter
+        assert '%(name)s' not in self.handler.formatter._fmt
+        assert '%(funcName)s' not in self.handler.formatter._fmt
+
+    def test_with_not_a_tty(self):
+        self.stream.isatty.return_value = False
+        setup_console_handler(self.handler, False)
+        assert type(self.handler.formatter) == logging.Formatter
+
+
 class ConvergeStrategyFromOptsTestCase(unittest.TestCase):
 
     def test_invalid_opts(self):

tests/unit/config/config_test.py

@@ -380,7 +380,7 @@ class ConfigTest(unittest.TestCase):
 
     @mock.patch('compose.config.validation.log')
     def test_logs_warning_for_boolean_in_environment(self, mock_logging):
-        expected_warning_msg = "Warning: There is a boolean value in the 'environment' key."
+        expected_warning_msg = "There is a boolean value in the 'environment' key."
         config.load(
             build_config_details(
                 {'web': {