From 57f15f9cce199b18de11281b679af39e2c7c6a22 Mon Sep 17 00:00:00 2001
From: Darko Poljak <darko.poljak@gmail.com>
Date: Thu, 7 Sep 2017 16:36:23 +0200
Subject: [PATCH] Make __cdist_loglevel value more expressive. (#571)

---
 .../conf/type/__install_config/gencode-local  |  8 +++---
 .../conf/type/__install_stage/gencode-remote  |  2 +-
 cdist/core/manifest.py                        |  2 +-
 cdist/emulator.py                             | 10 +++++--
 cdist/test/emulator/__init__.py               | 28 +++++++++++++++++++
 cdist/test/manifest/__init__.py               |  1 +
 docs/changelog                                |  1 +
 docs/src/cdist-reference.rst.sh               |  5 ++--
 docs/src/cdist-type.rst                       |  5 ++--
 9 files changed, 48 insertions(+), 14 deletions(-)

diff --git a/cdist/conf/type/__install_config/gencode-local b/cdist/conf/type/__install_config/gencode-local
index d7e98734..41517bd8 100755
--- a/cdist/conf/type/__install_config/gencode-local
+++ b/cdist/conf/type/__install_config/gencode-local
@@ -24,16 +24,16 @@ remote_copy="$__type/files/remote/copy"
 
 cdist_args=""
 case "$__cdist_loglevel" in
-   20)
+   INFO)
       cdist_args="-v"
    ;;
-   15)
+   VERBOSE)
       cdist_args="-vv"
    ;;
-   10)
+   DEBUG)
       cdist_args="-vvv"
    ;;
-   5)
+   TRACE)
       cdist_args="-vvvv"
    ;;
 esac
diff --git a/cdist/conf/type/__install_stage/gencode-remote b/cdist/conf/type/__install_stage/gencode-remote
index 1da6e7e9..214e4b52 100755
--- a/cdist/conf/type/__install_stage/gencode-remote
+++ b/cdist/conf/type/__install_stage/gencode-remote
@@ -23,7 +23,7 @@ uri="$(cat "$__object/parameter/uri" 2>/dev/null \
 target="$(cat "$__object/parameter/target")"
 
 case "$__cdist_loglevel" in
-    10|5)  # DEBUG or TRACE
+    DEBUG|TRACE)
         curl="curl"
         tar="tar -xvzp"
     ;;
diff --git a/cdist/core/manifest.py b/cdist/core/manifest.py
index 0a26601a..b7538887 100644
--- a/cdist/core/manifest.py
+++ b/cdist/core/manifest.py
@@ -114,7 +114,7 @@ class Manifest(object):
         }
 
         self.env.update(
-            {'__cdist_loglevel': str(self.log.getEffectiveLevel())})
+            {'__cdist_loglevel': logging.getLevelName(self.log.getEffectiveLevel())})
 
     def _open_logger(self):
         self.log = logging.getLogger(self.target_host[0])
diff --git a/cdist/emulator.py b/cdist/emulator.py
index f1419ceb..f3e9b9b0 100644
--- a/cdist/emulator.py
+++ b/cdist/emulator.py
@@ -111,12 +111,18 @@ class Emulator(object):
 
         if '__cdist_loglevel' in self.env:
             try:
-                level = int(self.env['__cdist_loglevel'])
+                loglevel = self.env['__cdist_loglevel']
+                # For a text level it returns its numerical value.
+                level = logging.getLevelName(loglevel)
             except ValueError:
                 level = logging.WARNING
         else:
             level = logging.WARNING
-        logging.root.setLevel(level)
+        try:
+            logging.root.setLevel(level)
+        except (ValueError, TypeError):
+            # if invalid __cdist_loglevel value
+            logging.root.setLevel(logging.WARNING)
 
         self.log = logging.getLogger(self.target_host[0])
 
diff --git a/cdist/test/emulator/__init__.py b/cdist/test/emulator/__init__.py
index 664ab20b..a63ce04a 100644
--- a/cdist/test/emulator/__init__.py
+++ b/cdist/test/emulator/__init__.py
@@ -27,6 +27,7 @@ import shutil
 import string
 import filecmp
 import random
+import logging
 
 import cdist
 from cdist import test
@@ -63,6 +64,8 @@ class EmulatorTestCase(test.CdistTestCase):
         self.manifest = core.Manifest(self.target_host, self.local)
         self.env = self.manifest.env_initial_manifest(self.script)
         self.env['__cdist_object_marker'] = self.local.object_marker_name
+        if '__cdist_loglevel' in self.env:
+            del self.env['__cdist_loglevel']
 
     def tearDown(self):
         shutil.rmtree(self.temp_dir)
@@ -115,6 +118,31 @@ class EmulatorTestCase(test.CdistTestCase):
         emu = emulator.Emulator(argv, env=self.env)
         # if we get here all is fine
 
+    def test_loglevel(self):
+        argv = ['__file', '/tmp/foobar']
+        self.env['require'] = '__file/etc/*'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu_loglevel = emu.log.getEffectiveLevel()
+        self.assertEqual(emu_loglevel, logging.WARNING)
+        self.env['__cdist_loglevel'] = logging.getLevelName(logging.DEBUG)
+        emu = emulator.Emulator(argv, env=self.env)
+        emu_loglevel = emu.log.getEffectiveLevel()
+        self.assertEqual(emu_loglevel, logging.DEBUG)
+        del self.env['__cdist_loglevel']
+
+    def test_invalid_loglevel_value(self):
+        argv = ['__file', '/tmp/foobar']
+        self.env['require'] = '__file/etc/*'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu_loglevel = emu.log.getEffectiveLevel()
+        self.assertEqual(emu_loglevel, logging.WARNING)
+        # lowercase is invalid
+        self.env['__cdist_loglevel'] = 'debug'
+        emu = emulator.Emulator(argv, env=self.env)
+        emu_loglevel = emu.log.getEffectiveLevel()
+        self.assertEqual(emu_loglevel, logging.WARNING)
+        del self.env['__cdist_loglevel']
+
     def test_requirement_via_order_dependency(self):
         self.env['CDIST_ORDER_DEPENDENCY'] = 'on'
         argv = ['__planet', 'erde']
diff --git a/cdist/test/manifest/__init__.py b/cdist/test/manifest/__init__.py
index 95bf2768..3462c9ca 100644
--- a/cdist/test/manifest/__init__.py
+++ b/cdist/test/manifest/__init__.py
@@ -137,6 +137,7 @@ class ManifestTestCase(test.CdistTestCase):
         self.log.setLevel(logging.DEBUG)
         manifest = cdist.core.manifest.Manifest(self.target_host, self.local)
         self.assertTrue("__cdist_loglevel" in manifest.env)
+        self.assertEqual(manifest.env["__cdist_loglevel"], 'DEBUG')
         self.log.setLevel(current_level)
 
 
diff --git a/docs/changelog b/docs/changelog
index 4a7f8c8b..3006e188 100644
--- a/docs/changelog
+++ b/docs/changelog
@@ -12,6 +12,7 @@ next:
 	* Type __package_pkg_openbsd: Fix pkg_version explorer (Philippe Gregoire)
 	* Documentation: Document __cdist_loglevel type variable (Darko Poljak)
 	* Type __install_stage: Fix __debug -> __cdist_loglevel (Darko Poljak)
+	* Core, types: Make __cdist_loglevel value more expressive (Darko Poljak)
 
 4.6.1: 2017-08-30
 	* Type __user: Explore with /etc files (passwd, group, shadow) (Philippe Gregoire)
diff --git a/docs/src/cdist-reference.rst.sh b/docs/src/cdist-reference.rst.sh
index cb717c4a..0117c04d 100755
--- a/docs/src/cdist-reference.rst.sh
+++ b/docs/src/cdist-reference.rst.sh
@@ -199,9 +199,8 @@ Environment variables (for reading)
 The following environment variables are exported by cdist:
 
 __cdist_loglevel
-    Value of cdist log level. One of 60, 40, 30, 20, 15, 10 and 5 where
-    the meaning is OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and TRACE,
-    respectively.
+    String value of cdist log level. One of OFF, ERROR, WARNING, INFO,
+    VERBOSE, DEBUG and TRACE.
     Available for: initial manifest, type manifest, type gencode.
 __explorer
     Directory that contains all global explorers.
diff --git a/docs/src/cdist-type.rst b/docs/src/cdist-type.rst
index 8c8605d1..2e6cbc96 100644
--- a/docs/src/cdist-type.rst
+++ b/docs/src/cdist-type.rst
@@ -334,9 +334,8 @@ So when you generate a script with the following content, it will work:
 Log level in types
 ------------------
 cdist log level can be accessed from __cdist_loglevel variable.
-Value is one of 60, 40, 30, 20, 15, 10 and 5 where the meaning is
-OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and TRACE, respectively.
-It is available for initial manifest, type manifest and type gencode.
+Value is a string, one of OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and
+TRACE. It is available for initial manifest, type manifest and type gencode.
 
 
 Hints for typewriters