From 3f65cc05b3f24185fb50ec3f1312c73a698c99cf Mon Sep 17 00:00:00 2001 From: Jiri Podivin Date: Tue, 24 Aug 2021 14:44:05 +0200 Subject: [PATCH] validations_read_ini refactoring General refactoring of the module along with small changes to the existing unit tests. - file existence check logic optimized - docstrings expanded - missing config file handling - return type clarification Signed-off-by: Jiri Podivin Change-Id: I47a85cd71b5d508f332c91217e24bcf4302cf0ea --- .../library/validations_read_ini.py | 60 +++++++++++-------- .../library/test_validations_read_ini.py | 20 ++----- 2 files changed, 42 insertions(+), 38 deletions(-) diff --git a/validations_common/library/validations_read_ini.py b/validations_common/library/validations_read_ini.py index efe6ae6..39bc940 100644 --- a/validations_common/library/validations_read_ini.py +++ b/validations_common/library/validations_read_ini.py @@ -90,17 +90,33 @@ class ReturnValue(Enum): KEY_NOT_FOUND = 2 -def check_file(path, ignore_missing): - '''Validate entered path''' +def check_file(path): + """Check if the requested file exists. + :param path: path to configuration file + :dtype path: `string` - if not (os.path.exists(path) and os.path.isfile(path)): - return "Could not open the ini file: '{}'".format(path) - else: - return '' + :returns: True if file exists, false otherwise + :rtype: `bool` + """ + + return (os.path.exists(path) and os.path.isfile(path)) def get_result(path, section, key, default=None): - '''Get value based on section and key''' + """Get value based on a section and a key. + + :param path: path to configuration file + :dtype path: `string` + :param section: name of the config file section + :dtype section: `string` + :param key: key for which we want to know the value + :dtype key: `string` + :param default: default value to use if the key does not exist + :dtype default: `object` + + :returns: tuple of numeric return code, message and value of the requested key + :rtype: `tuple` + """ msg = '' value = None @@ -110,17 +126,14 @@ def get_result(path, section, key, default=None): config.read(path) except (OSError, ConfigParser.ParsingError) as exc_msg: msg = "The file '{}' is not in a valid INI format: {}".format( - path, exc_msg - ) - ret = ReturnValue.INVALID_FORMAT - return (ret, msg, value) + path, exc_msg) + return (ReturnValue.INVALID_FORMAT, msg, value) try: value = config.get(section, key) msg = ("The key '{}' under the section '{}' in file {} " "has the value: '{}'").format(key, section, path, value) ret = ReturnValue.OK - return (ret, msg, value) except ConfigParser.Error: if default: msg = ("There is no key '{}' under section '{}' in file {}. Using" @@ -132,27 +145,20 @@ def get_result(path, section, key, default=None): msg = "There is no key '{}' under the section '{}' in file {}.".format( key, section, path) ret = ReturnValue.KEY_NOT_FOUND - return (ret, msg, value) + return (ret, msg, value) def main(): module = AnsibleModule( - argument_spec=yaml_safe_load(DOCUMENTATION)['options'] - ) + argument_spec=yaml_safe_load(DOCUMENTATION)['options']) ini_file_path = module.params.get('path') ignore_missing = module.params.get('ignore_missing_file') # Check that file exists - msg = check_file(ini_file_path, ignore_missing) + file_exists = check_file(ini_file_path) - if msg != '': - # Opening file failed - if ignore_missing: - module.exit_json(msg=msg, changed=False, value=None) - else: - module.fail_json(msg=msg) - else: + if file_exists: # Try to parse the result from ini file section = module.params.get('section') key = module.params.get('key') @@ -166,7 +172,13 @@ def main(): module.exit_json(msg=msg, changed=False, value=None) elif ret == ReturnValue.OK: module.exit_json(msg=msg, changed=False, value=value) - + else: + # Opening file failed + msg = "Could not open the ini file: '{}'".format(ini_file_path) + if ignore_missing: + module.exit_json(msg=msg, changed=False, value=None) + else: + module.fail_json(msg=msg) if __name__ == '__main__': main() diff --git a/validations_common/tests/library/test_validations_read_ini.py b/validations_common/tests/library/test_validations_read_ini.py index beedfa6..3b70570 100644 --- a/validations_common/tests/library/test_validations_read_ini.py +++ b/validations_common/tests/library/test_validations_read_ini.py @@ -56,26 +56,18 @@ class TestValidationsReadIni(base.TestCase): def test_check_file_invalid_path(self): '''Test validations_read_ini when path is invalid''' - msg = validation.check_file('non/existing/path', False) - self.assertEqual("Could not open the ini file: 'non/existing/path'", - msg) - - def test_check_file_ignore_missing(self): - '''Test validations_read_ini when ignoring missing files''' - - msg = validation.check_file('non/existing/path', True) - self.assertEqual("Could not open the ini file: 'non/existing/path'", - msg) + ret_val = validation.check_file('non/existing/path') + self.assertEqual(False, ret_val) def test_check_file_valid_path(self): '''Test validations_read_ini when path is valid''' - tmpfile = self.create_tmp_ini() - tmp_name = os.path.relpath(tmpfile.name) - msg = validation.check_file(tmp_name, False) + with self.create_tmp_ini() as tmpfile: + tmp_name = os.path.relpath(tmpfile.name) + ret_val = validation.check_file(tmp_name) tmpfile.close() - self.assertEqual('', msg) + self.assertEqual(True, ret_val) def test_get_result_invalid_format(self): '''Test validations_read_ini when file format is valid'''