Make __cdist_loglevel value more expressive. (#571)

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

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"
     ;;

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])

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])
 

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']

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)
 
 

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)

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
+    String value of cdist log level. One of OFF, ERROR, WARNING, INFO,
-    the meaning is OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and TRACE,
+    VERBOSE, DEBUG and TRACE.
-    respectively.
     Available for: initial manifest, type manifest, type gencode.
 __explorer
     Directory that contains all global explorers.

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
+Value is a string, one of OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and
-OFF, ERROR, WARNING, INFO, VERBOSE, DEBUG and TRACE, respectively.
+TRACE. It is available for initial manifest, type manifest and type gencode.
-It is available for initial manifest, type manifest and type gencode.
 
 
 Hints for typewriters