Improve file handling: avoid "unclosed file" warnings

Backported from python 3 changes, where this warning shows up:

   ResourceWarning: unclosed file <_io.BufferedReader name='.../bogus.conf'>

Changes:
- Refactored parse_conf(), write_conf() into separate file+stream and stream-
  only (backend) functions so that all file open requests are properly handed
  in a 'with' block.
- Improved file handling of the TestWorkDir and other unit-testing helpers

ksconf/commands/sort.py

@@ -5,6 +5,13 @@
 from ksconf.consts import SMART_NOCHANGE, EXIT_CODE_BAD_CONF_FILE, EXIT_CODE_SORT_APPLIED
 
 
+def _has_nosort_marker(path):
+    # KISS:  Look for the KSCONF-NO-SORT string in the first 4k of this file.
+    with open(path, "rb") as stream:
+        prefix = stream.read(4096)
+    return b"KSCONF-NO-SORT" in prefix
+
+
 def do_sort(args):
     ''' Sort a single configuration file. '''
     stanza_delims = "\n" * args.newlines
@@ -13,8 +20,7 @@ def do_sort(args):
         changes = 0
         for conf in args.conf:
             try:
-                # KISS:  Look for the KSCONF-NO-SORT string in the first 4k of this file.
-                if not args.force and "KSCONF-NO-SORT" in open(conf.name).read(4096):
+                if not args.force and _has_nosort_marker(conf.name):
                     if not args.quiet:
                         sys.stderr.write("Skipping blacklisted file {}\n".format(conf.name))
                     continue

ksconf/conf/parser.py

@@ -139,14 +139,16 @@ def splitup_kvpairs(lines, comments_re=re.compile(r"^\s*[#;]"), keep_comments=Fa
 
 def parse_conf(stream, profile=PARSECONF_MID):
     # Placeholder stub for an eventual migration to proper class-oriented parser
-    return _parse_conf(stream, **profile)
+    if hasattr(stream, "read"):
+        return parse_conf_stream(stream, **profile)
+    else:
+        # Assume it's a filename
+        with open(stream, "r") as stream:
+            return parse_conf_stream(stream, **profile)
 
 
-def _parse_conf(stream, keys_lower=False, handle_conts=True, keep_comments=False,
+def parse_conf_stream(stream, keys_lower=False, handle_conts=True, keep_comments=False,
                 dup_stanza=DUP_EXCEPTION, dup_key=DUP_OVERWRITE, strict=False):
-    if not hasattr(stream, "read"):
-        # Assume it's a filename
-        stream = codecs.open(stream)  # , encoding="utf-8")
     if hasattr(stream, "name"):
         stream_name = stream.name
     else:
@@ -199,7 +201,13 @@ def _parse_conf(stream, keys_lower=False, handle_conts=True, keep_comments=False
 def write_conf(stream, conf, stanza_delim="\n", sort=True):
     if not hasattr(stream, "write"):
         # Assume it's a filename
-        stream = open(stream, "w")
+        with open(stream, "w") as stream:
+            write_conf_stream(stream, conf, stanza_delim, sort)
+    else:
+        write_conf_stream(stream, conf, stanza_delim, sort)
+
+
+def write_conf_stream(stream, conf, stanza_delim="\n", sort=True):
     conf = dict(conf)
 
     if sort:
@@ -241,7 +249,7 @@ def write_stanza_body(items):
 def smart_write_conf(filename, conf, stanza_delim="\n", sort=True, temp_suffix=".tmp"):
     if os.path.isfile(filename):
         temp = StringIO()
-        write_conf(temp, conf, stanza_delim, sort)
+        write_conf_stream(temp, conf, stanza_delim, sort)
         with open(filename, "rb") as dest:
             file_diff = fileobj_compare(temp, dest)
         if file_diff:
@@ -256,7 +264,7 @@ def smart_write_conf(filename, conf, stanza_delim="\n", sort=True, temp_suffix="
     else:
         tempfile = filename + temp_suffix
         with open(tempfile, "wb") as dest:
-            write_conf(dest, conf, stanza_delim, sort)
+            write_conf_stream(dest, conf, stanza_delim, sort)
         os.rename(tempfile, filename)
         return SMART_CREATE
 

tests/test_cli.py

@@ -22,7 +22,8 @@
 def _debug_file(flag, fn):       # pragma: no cover
     """ Dump file contents with a message string to the output.  For quick'n'diry unittest
     debugging only """
-    content = open(fn).read()
+    with open(fn) as fp:
+         content = fp.read()
     length = len(content)
     hash = file_hash(fn)
     print "\n{flag} {fn}  len={length} hash={hash} \n{content}".format(**vars())
@@ -142,10 +143,27 @@ def write_file(self, rel_path, content):
         content = dedent(content)
         path = self.get_path(rel_path)
         self.makedir(None, path=os.path.dirname(path))
-        with open(path, "w") as stream:
+        kw = {}
+        if isinstance(content, bytes):
+            kw["mode"] = "wb"
+        else:
+            kw["mode"] = "w"
+            content = dedent(content)
+        with open(path, **kw) as stream:
             stream.write(content)
         return path
 
+    def read_file(self, rel_path, as_bytes=False):
+        path = self.get_path(rel_path)
+        kw = {}
+        if as_bytes:
+            kw["mode"] = "rb"
+        else:
+            kw["mode"] = "r"
+        with open(path, **kw) as stream:
+            content = stream.read()
+        return content
+
     def write_conf(self, rel_path, conf):
         path = self.get_path(rel_path)
         self.makedir(None, path=os.path.dirname(path))
@@ -158,9 +176,9 @@ def read_conf(self, rel_path, profile=PARSECONF_MID):
 
     def copy_static(self, static, rel_path):
         src = static_data(static)
-        with open(src, "r") as stream:
+        with open(src, "rb") as stream:
             content = stream.read()
-            return self.write_file(rel_path, content)
+        return self.write_file(rel_path, content)
 
 
 class CliSimpleTestCase(unittest.TestCase):
@@ -239,7 +257,7 @@ def test_combine_3dir(self):
             self.assertEqual(cfg["aws:config"]["ANNOTATE_PUNCT"], "true")
             self.assertEqual(cfg["aws:config"]["EVAL-change_type"], '"configuration"')
             self.assertEqual(cfg["aws:config"]["TRUNCATE"], '9999999')
-            nav_content = open(twd.get_path("etc/apps/Splunk_TA_aws/default/data/ui/nav/default.xml")).read()
+            nav_content = twd.read_file("etc/apps/Splunk_TA_aws/default/data/ui/nav/default.xml")
             self.assertIn("My custom view", nav_content)
         twd.write_conf("etc/apps/Splunk_TA_aws/default.d/99-theforce/props.conf", {
             "aws:config": {"TIME_FORMAT": "%Y-%m-%dT%H:%M:%S.%6NZ"}

tests/test_parser.py

@@ -11,7 +11,7 @@
 from ksconf.conf.delta import compare_cfgs, summarize_cfg_diffs, \
     DIFF_OP_REPLACE, DIFF_OP_EQUAL, DIFF_OP_DELETE, DIFF_OP_INSERT
 from ksconf.conf.merge import merge_conf_dicts
-from ksconf.conf.parser import _parse_conf, DUP_EXCEPTION, DUP_MERGE, DUP_OVERWRITE, \
+from ksconf.conf.parser import parse_conf_stream, DUP_EXCEPTION, DUP_MERGE, DUP_OVERWRITE, \
     DuplicateStanzaException, DuplicateKeyException, parse_conf, write_conf, ConfParserException, \
     PARSECONF_MID, GLOBAL_STANZA
 from ksconf.util.file import relwalk
@@ -23,7 +23,7 @@ def parse_string(text, profile=None, **kwargs):
     if profile:
         return parse_conf(f, profile)
     else:
-        return _parse_conf(f, **kwargs)
+        return parse_conf_stream(f, **kwargs)
 
 
 class ParserTestCase(unittest.TestCase):