cloud-init-dev team mailing list archive

[Andrew Jorgensen <ajorgens@xxxxxxxxxx>]

Andrew Jorgensen has proposed merging ~ajorgens/cloud-init:write-files-logging into cloud-init:master.

Commit message:
write_files: Remove log from helper function signatures

Requested as a follow-on to https://code.launchpad.net/~ajorgens/cloud-init/+git/cloud-init/+merge/325424
Followed pattern found in cc_apt_configure.py

Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~ajorgens/cloud-init/+git/cloud-init/+merge/325863
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~ajorgens/cloud-init:write-files-logging into cloud-init:master.

diff --git a/cloudinit/config/cc_write_files.py b/cloudinit/config/cc_write_files.py
index 1835a31..66856f4 100644
--- a/cloudinit/config/cc_write_files.py
+++ b/cloudinit/config/cc_write_files.py
@@ -51,6 +51,7 @@ import os
 import six
 
 from cloudinit.settings import PER_INSTANCE
+from cloudinit import log as logging
 from cloudinit import util
 
 
@@ -60,17 +61,23 @@ DEFAULT_OWNER = "root:root"
 DEFAULT_PERMS = 0o644
 UNKNOWN_ENC = 'text/plain'
 
+LOG = logging.getLogger(__name__)
+
 
 def handle(name, cfg, _cloud, log, _args):
+    if log is not None:
+        global LOG
+        LOG = log
+
     files = cfg.get('write_files')
     if not files:
-        log.debug(("Skipping module named %s,"
+        LOG.debug(("Skipping module named %s,"
                    " no/empty 'write_files' key in configuration"), name)
         return
-    write_files(name, files, log)
+    write_files(name, files)
 
 
-def canonicalize_extraction(encoding_type, log):
+def canonicalize_extraction(encoding_type):
     if not encoding_type:
         encoding_type = ''
     encoding_type = encoding_type.lower().strip()
@@ -85,31 +92,31 @@ def canonicalize_extraction(encoding_type, log):
     if encoding_type in ['b64', 'base64']:
         return ['application/base64']
     if encoding_type:
-        log.warn("Unknown encoding type %s, assuming %s",
+        LOG.warning("Unknown encoding type %s, assuming %s",
                  encoding_type, UNKNOWN_ENC)
     return [UNKNOWN_ENC]
 
 
-def write_files(name, files, log):
+def write_files(name, files):
     if not files:
         return
 
     for (i, f_info) in enumerate(files):
         path = f_info.get('path')
         if not path:
-            log.warn("No path provided to write for entry %s in module %s",
+            LOG.warning("No path provided to write for entry %s in module %s",
                      i + 1, name)
             continue
         path = os.path.abspath(path)
-        extractions = canonicalize_extraction(f_info.get('encoding'), log)
+        extractions = canonicalize_extraction(f_info.get('encoding'))
         contents = extract_contents(f_info.get('content', ''), extractions)
         (u, g) = util.extract_usergroup(f_info.get('owner', DEFAULT_OWNER))
-        perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS, log)
+        perms = decode_perms(f_info.get('permissions'), DEFAULT_PERMS)
         util.write_file(path, contents, mode=perms)
         util.chownbyname(path, u, g)
 
 
-def decode_perms(perm, default, log):
+def decode_perms(perm, default):
     if perm is None:
         return default
     try:
@@ -126,7 +133,7 @@ def decode_perms(perm, default, log):
                 reps.append("%o" % r)
             except TypeError:
                 reps.append("%r" % r)
-        log.warning(
+        LOG.warning(
             "Undecodable permissions {0}, returning default {1}".format(*reps))
         return default
 
diff --git a/tests/unittests/test_handler/test_handler_write_files.py b/tests/unittests/test_handler/test_handler_write_files.py
index 88a4742..1129e77 100644
--- a/tests/unittests/test_handler/test_handler_write_files.py
+++ b/tests/unittests/test_handler/test_handler_write_files.py
@@ -49,13 +49,13 @@ class TestWriteFiles(FilesystemMockingTestCase):
         expected = "hello world\n"
         filename = "/tmp/my.file"
         write_files(
-            "test_simple", [{"content": expected, "path": filename}], LOG)
+            "test_simple", [{"content": expected, "path": filename}])
         self.assertEqual(util.load_file(filename), expected)
 
     def test_yaml_binary(self):
         self.patchUtils(self.tmp)
         data = util.load_yaml(YAML_TEXT)
-        write_files("testname", data['write_files'], LOG)
+        write_files("testname", data['write_files'])
         for path, content in YAML_CONTENT_EXPECTED.items():
             self.assertEqual(util.load_file(path), content)
 
@@ -87,7 +87,7 @@ class TestWriteFiles(FilesystemMockingTestCase):
                     files.append(cur)
                     expected.append((cur['path'], data))
 
-        write_files("test_decoding", files, LOG)
+        write_files("test_decoding", files)
 
         for path, content in expected:
             self.assertEqual(util.load_file(path, decode=False), content)
@@ -105,22 +105,22 @@ class TestDecodePerms(CiTestCase):
     def test_none_returns_default(self):
         """If None is passed as perms, then default should be returned."""
         default = object()
-        found = decode_perms(None, default, self.logger)
+        found = decode_perms(None, default)
         self.assertEqual(default, found)
 
     def test_integer(self):
         """A valid integer should return itself."""
-        found = decode_perms(0o755, None, self.logger)
+        found = decode_perms(0o755, None)
         self.assertEqual(0o755, found)
 
     def test_valid_octal_string(self):
         """A string should be read as octal."""
-        found = decode_perms("644", None, self.logger)
+        found = decode_perms("644", None)
         self.assertEqual(0o644, found)
 
     def test_invalid_octal_string_returns_default_and_warns(self):
         """A string with invalid octal should warn and return default."""
-        found = decode_perms("999", None, self.logger)
+        found = decode_perms("999", None)
         self.assertIsNone(found)
         self.assertIn("WARNING: Undecodable", self.logs.getvalue())