From 3cedecb5813a203e70789d001818babbc37f574d Mon Sep 17 00:00:00 2001 From: Simon Maillard Date: Fri, 22 Nov 2024 14:27:45 +0100 Subject: [PATCH 1/2] Add informations on cli output about module If the operation or fact is not found, it will display the ones available, and if no one is avaible, Warn the user about the possibility that he have a py file or a folder with the same name as the module --- pyinfra_cli/util.py | 63 ++++++++++++++++++++++++++- tests/test_cli/test_cli_exceptions.py | 8 +++- tests/test_cli/test_cli_util.py | 11 +++-- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/pyinfra_cli/util.py b/pyinfra_cli/util.py index 95ea7b62e..111dc96f1 100644 --- a/pyinfra_cli/util.py +++ b/pyinfra_cli/util.py @@ -15,6 +15,7 @@ import gevent from pyinfra import logger, state +from pyinfra.api import FactBase from pyinfra.api.command import PyinfraCommand from pyinfra.api.exceptions import PyinfraError from pyinfra.api.host import HostData @@ -153,6 +154,37 @@ def parse_cli_arg(arg): return arg +def get_module_available_actions(module: ModuleType) -> dict[str, list[str]]: + """ + Lists operations and facts available in a module. + + Args: + module (ModuleType): The module to inspect. + + Returns: + dict: A dictionary with keys 'operations' and 'facts', each containing a list of names. + """ + available_actions: dict[str, list[str]] = { + "operations": [], + "facts": [], + } + + for name, obj in module.__dict__.items(): + # Check if it's a decorated operation + if callable(obj) and getattr(obj, "is_idempotent", None) is not None: + available_actions["operations"].append(name) + + # Check if it's a FactBase subclass + elif ( + isinstance(obj, type) + and issubclass(obj, FactBase) + and obj.__module__ == module.__name__ + ): + available_actions["facts"].append(name) + + return available_actions + + def try_import_module_attribute(path, prefix=None, raise_for_none=True): if ":" in path: # Allow a.module.name:function syntax @@ -189,7 +221,36 @@ def try_import_module_attribute(path, prefix=None, raise_for_none=True): attr = getattr(module, attr_name, None) if attr is None: if raise_for_none: - raise CliError(f"No such attribute in module {possible_modules[0]}: {attr_name}") + extra_info = [] + module_name = getattr(module, "__name__", str(module)) + + if prefix == "pyinfra.operations": + # List classes of type OperationMeta + available_operations = get_module_available_actions(module)["operations"] + if available_operations: + extra_info.append( + f"Available operations are: {', '.join(available_operations)}" + ) + else: + extra_info.append( + "No operations found. Maybe you have a file or folder named " + f"`{str(module_name)}` in the current folder ?" + ) + + elif prefix == "pyinfra.facts": + # List classes of type FactBase + available_facts = get_module_available_actions(module)["facts"] + if available_facts: + extra_info.append(f"Available facts are: {', '.join(available_facts)}") + else: + extra_info.append( + "No facts found. Maybe you have a file or folder named " + f"`{str(module_name)}` in the current folder ?" + ) + + message = [f"No such attribute in module {possible_modules[0]}: {attr_name}"] + + raise CliError("\n".join(message + extra_info)) return return attr diff --git a/tests/test_cli/test_cli_exceptions.py b/tests/test_cli/test_cli_exceptions.py index 7cac5f15a..e20a5797d 100644 --- a/tests/test_cli/test_cli_exceptions.py +++ b/tests/test_cli/test_cli_exceptions.py @@ -44,7 +44,13 @@ def test_no_fact_module(self): def test_no_fact_cls(self): self.assert_cli_exception( ["my-server.net", "fact", "server.NotAFact"], - "No such attribute in module server: NotAFact", + ( + "No such attribute in module server: NotAFact\n" + "Available facts in module are: User, Home, Path, TmpDir, Hostname, Kernel, " + "KernelVersion, Os, OsVersion, Arch, Command, Which, Date, MacosVersion, Mounts, " + "KernelModules, LsbRelease, OsRelease, Sysctl, Groups, Users, LinuxDistribution, " + "Selinux, LinuxGui, Locales, SecurityLimits" + ), ) diff --git a/tests/test_cli/test_cli_util.py b/tests/test_cli/test_cli_util.py index 776510f14..b27d3baa2 100644 --- a/tests/test_cli/test_cli_util.py +++ b/tests/test_cli/test_cli_util.py @@ -32,11 +32,16 @@ def test_setup_no_module(self): get_func_and_args(("no.op",)) assert context.exception.message == "No such module: no" - def test_setup_no_op(self): + def test_setup_not_exists_op(self): with self.assertRaises(CliError) as context: - get_func_and_args(("server.no",)) + get_func_and_args(("server.not_exists",)) - assert context.exception.message == "No such attribute in module server: no" + assert context.exception.message == ( + "No such attribute in module server: not_exists\n" + "Available operations are: reboot, wait, shell, script, script_template, " + "modprobe, mount, hostname, sysctl, service, packages, crontab, group, " + "user_authorized_keys, user, locale, security_limit" + ) def test_setup_op_and_args(self): commands = ("pyinfra.operations.server.user", "one", "two", "hello=world") From 8b752b18ebdd52022ca45f441a377160be40ea56 Mon Sep 17 00:00:00 2001 From: Simon Maillard Date: Mon, 25 Nov 2024 11:22:10 +0100 Subject: [PATCH 2/2] Update tests Use of regex to avoid having to update the cli tests each time the operations and facts used in them are modified --- tests/test_cli/test_cli_exceptions.py | 33 ++++++++++++++++++++------- tests/test_cli/test_cli_util.py | 14 +++++++----- 2 files changed, 33 insertions(+), 14 deletions(-) diff --git a/tests/test_cli/test_cli_exceptions.py b/tests/test_cli/test_cli_exceptions.py index e20a5797d..c1c309761 100644 --- a/tests/test_cli/test_cli_exceptions.py +++ b/tests/test_cli/test_cli_exceptions.py @@ -1,3 +1,4 @@ +import re import sys from os import path from unittest import TestCase @@ -21,7 +22,24 @@ def setUpClass(cls): def assert_cli_exception(self, args, message): result = self.runner.invoke(cli, args, standalone_mode=False) self.assertIsInstance(result.exception, CliError) - assert getattr(result.exception, "message") == message + + if isinstance(message, str): + message = [message] + + for part in message: + + # Test if the string is a regex + is_regex = False + try: + re.compile(part) + is_regex = True + except re.error: + pass + + if is_regex: + assert re.search(part, result.exception.message, re.MULTILINE) + else: + assert part == result.exception.message def test_bad_deploy_file(self): self.assert_cli_exception( @@ -44,13 +62,12 @@ def test_no_fact_module(self): def test_no_fact_cls(self): self.assert_cli_exception( ["my-server.net", "fact", "server.NotAFact"], - ( - "No such attribute in module server: NotAFact\n" - "Available facts in module are: User, Home, Path, TmpDir, Hostname, Kernel, " - "KernelVersion, Os, OsVersion, Arch, Command, Which, Date, MacosVersion, Mounts, " - "KernelModules, LsbRelease, OsRelease, Sysctl, Groups, Users, LinuxDistribution, " - "Selinux, LinuxGui, Locales, SecurityLimits" - ), + [ + r"^No such attribute in module server: NotAFact.*", + r".*Available facts are: .*", + r".* User,.*", + r".* Os,", + ], ) diff --git a/tests/test_cli/test_cli_util.py b/tests/test_cli/test_cli_util.py index b27d3baa2..ec7f6b484 100644 --- a/tests/test_cli/test_cli_util.py +++ b/tests/test_cli/test_cli_util.py @@ -1,4 +1,5 @@ import os +import re import sys from datetime import datetime from io import StringIO @@ -36,12 +37,13 @@ def test_setup_not_exists_op(self): with self.assertRaises(CliError) as context: get_func_and_args(("server.not_exists",)) - assert context.exception.message == ( - "No such attribute in module server: not_exists\n" - "Available operations are: reboot, wait, shell, script, script_template, " - "modprobe, mount, hostname, sysctl, service, packages, crontab, group, " - "user_authorized_keys, user, locale, security_limit" - ) + for part in [ + r"^No such attribute in module server: not_exists$", + r"Available operations are: .*", + r".* modprobe, .*", + r".* mount, .*", + ]: + assert re.search(part, context.exception.message, re.MULTILINE) def test_setup_op_and_args(self): commands = ("pyinfra.operations.server.user", "one", "two", "hello=world")