From 19eef1cb40666b968f22873ce67645384e9ac192 Mon Sep 17 00:00:00 2001 From: Lowell Alleman Date: Tue, 26 Jun 2018 13:54:00 -0400 Subject: [PATCH] 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 | 10 ++++++++-- ksconf/conf/parser.py | 24 ++++++++++++++++-------- tests/test_cli.py | 28 +++++++++++++++++++++++----- tests/test_parser.py | 4 ++-- 4 files changed, 49 insertions(+), 17 deletions(-) diff --git a/ksconf/commands/sort.py b/ksconf/commands/sort.py index 8d78983..30625fe 100644 --- a/ksconf/commands/sort.py +++ b/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 diff --git a/ksconf/conf/parser.py b/ksconf/conf/parser.py index 0a4c5ee..33650ca 100644 --- a/ksconf/conf/parser.py +++ b/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 diff --git a/tests/test_cli.py b/tests/test_cli.py index efee4ce..fce4d70 100755 --- a/tests/test_cli.py +++ b/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"} diff --git a/tests/test_parser.py b/tests/test_parser.py index 4b8622b..f0fbc37 100755 --- a/tests/test_parser.py +++ b/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):