From ca7df2fa3f76761719ec68930ff9e8cc66b0adb1 Mon Sep 17 00:00:00 2001 From: Nolan Brubaker Date: Fri, 23 Sep 2016 19:04:18 -0400 Subject: [PATCH] Use detailed arguments for main function The previous behavior for main took an arbitrary dictionary and looked for keys within it. To decouple the main function from the args function, this change instead formalizes the names for the arguments to main, allowing for documentation of the expectations. Since Ansible's default expected argument is `--list` and that would collide with the Python `list` builtin function, the `kwargs` argument is added to act as a catch-all. In practice, while Ansible passes the list argument in, it does not change the script's behavior. Change-Id: I8a4c677b44b7c0fc6d95c0a5213306165b79971d --- playbooks/inventory/dynamic_inventory.py | 19 +++++++++++++------ ...n-function-arguments-8c43e4c7175937d3.yaml | 6 ++++++ tests/test_inventory.py | 10 ++++++---- 3 files changed, 25 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/inventory-main-function-arguments-8c43e4c7175937d3.yaml diff --git a/playbooks/inventory/dynamic_inventory.py b/playbooks/inventory/dynamic_inventory.py index 18fba95030..fa0649715b 100755 --- a/playbooks/inventory/dynamic_inventory.py +++ b/playbooks/inventory/dynamic_inventory.py @@ -1166,9 +1166,17 @@ def get_inventory(config_path, inventory_file_path): return dynamic_inventory -def main(all_args): - """Run the main application.""" - if all_args.get('debug'): +def main(config=None, check=False, debug=False, **kwargs): + """Run the main application. + + :param config: ``str`` Directory from which to pull configs and overrides + :param check: ``bool`` Flag to enable check mode + :param debug: ``bool`` Flag to enable debug logging + :param kwargs: ``dict`` Dictionary of arbitrary arguments; mostly for + catching Ansible's required `--list` parameter without name shadowing + the `list` built-in. + """ + if debug: log_fmt = "%(lineno)d - %(funcName)s: %(message)s" logging.basicConfig(format=log_fmt, filename='inventory.log') logger.setLevel(logging.DEBUG) @@ -1176,7 +1184,7 @@ def main(all_args): # Get the path to the user configuration files config_path = find_config_path( - user_config_path=all_args.get('config') + user_config_path=config ) user_defined_config = load_user_configuration(config_path) @@ -1247,7 +1255,6 @@ def main(all_args): sort_keys=True ) - check = all_args.get('check') if check: return 'Configuration ok!' @@ -1284,5 +1291,5 @@ def main(all_args): if __name__ == '__main__': all_args = args(sys.argv[1:]) - output = main(all_args) + output = main(**all_args) print(output) diff --git a/releasenotes/notes/inventory-main-function-arguments-8c43e4c7175937d3.yaml b/releasenotes/notes/inventory-main-function-arguments-8c43e4c7175937d3.yaml new file mode 100644 index 0000000000..b4733266f4 --- /dev/null +++ b/releasenotes/notes/inventory-main-function-arguments-8c43e4c7175937d3.yaml @@ -0,0 +1,6 @@ +--- +deprecations: + - The ``main`` function in ``dynamic_inventory.py`` now + takes named arguments instead of dictionary. This is to support + future code changes that will move construction logic into + separate files. diff --git a/tests/test_inventory.py b/tests/test_inventory.py index b69d8d9d2f..a0d9b64a81 100644 --- a/tests/test_inventory.py +++ b/tests/test_inventory.py @@ -89,11 +89,13 @@ def cleanup(): def get_inventory(clean=True, extra_args=None): "Return the inventory mapping in a dict." - args = {'config': TARGET_DIR} + # Use the list argument to more closely mirror + # Ansible's use of the callable. + args = {'config': TARGET_DIR, 'list': True} if extra_args: args.update(extra_args) try: - inventory_string = di.main(args) + inventory_string = di.main(**args) inventory = json.loads(inventory_string) return inventory finally: @@ -1106,14 +1108,14 @@ class TestConfigCheckFunctional(TestConfigCheckBase): self.user_defined_config['log_hosts']['bogus'] = ip def test_checking_good_config(self): - output = di.main({'config': TARGET_DIR, 'check': True}) + output = di.main(config=TARGET_DIR, check=True) self.assertEqual(output, 'Configuration ok!') def test_duplicated_ip(self): self.duplicate_ip() self.write_config() with self.assertRaises(di.MultipleHostsWithOneIPError) as context: - di.main({'config': TARGET_DIR, 'check': True}) + di.main(config=TARGET_DIR, check=True) self.assertEqual(context.exception.ip, '172.29.236.100')