From 5e71e43a2a2e9089185d34e5406ee212cc478a75 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Wed, 22 Mar 2017 16:29:37 +0100 Subject: Rename module_executor -> execute_module It is a function/callable, the name should imply action, should be a verb and not a noun. Keep supporting the old name while we have PRs in-flight that use the old name. --- .../action_plugins/openshift_health_check.py | 2 +- .../openshift_checks/__init__.py | 9 ++++-- .../openshift_checks/package_availability.py | 2 +- .../openshift_checks/package_update.py | 2 +- .../openshift_checks/package_version.py | 2 +- .../test/openshift_check_test.py | 37 +++++++++++++++++++++- 6 files changed, 47 insertions(+), 7 deletions(-) diff --git a/roles/openshift_health_checker/action_plugins/openshift_health_check.py b/roles/openshift_health_checker/action_plugins/openshift_health_check.py index 8b23533c8..027abf398 100644 --- a/roles/openshift_health_checker/action_plugins/openshift_health_check.py +++ b/roles/openshift_health_checker/action_plugins/openshift_health_check.py @@ -91,7 +91,7 @@ class ActionModule(ActionBase): check_name, cls.__module__, cls.__name__, other_cls.__module__, other_cls.__name__)) - known_checks[check_name] = cls(module_executor=self._execute_module) + known_checks[check_name] = cls(execute_module=self._execute_module) return known_checks diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py index 93547a2e0..72d0b26df 100644 --- a/roles/openshift_health_checker/openshift_checks/__init__.py +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -21,8 +21,13 @@ class OpenShiftCheckException(Exception): class OpenShiftCheck(object): """A base class for defining checks for an OpenShift cluster environment.""" - def __init__(self, module_executor): - self.module_executor = module_executor + def __init__(self, execute_module=None, module_executor=None): + if execute_module is module_executor is None: + raise TypeError( + "__init__() takes either execute_module (recommended) " + "or module_executor (deprecated), none given") + self.execute_module = execute_module or module_executor + self.module_executor = self.execute_module @abstractproperty def name(self): diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py index 771123d61..9891972a6 100644 --- a/roles/openshift_health_checker/openshift_checks/package_availability.py +++ b/roles/openshift_health_checker/openshift_checks/package_availability.py @@ -21,7 +21,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck): packages.update(self.node_packages(rpm_prefix)) args = {"packages": sorted(set(packages))} - return self.module_executor("check_yum_update", args, tmp, task_vars) + return self.execute_module("check_yum_update", args, tmp, task_vars) @staticmethod def master_packages(rpm_prefix): diff --git a/roles/openshift_health_checker/openshift_checks/package_update.py b/roles/openshift_health_checker/openshift_checks/package_update.py index c5a226954..fd0c0a755 100644 --- a/roles/openshift_health_checker/openshift_checks/package_update.py +++ b/roles/openshift_health_checker/openshift_checks/package_update.py @@ -11,4 +11,4 @@ class PackageUpdate(NotContainerizedMixin, OpenShiftCheck): def run(self, tmp, task_vars): args = {"packages": []} - return self.module_executor("check_yum_update", args, tmp, task_vars) + return self.execute_module("check_yum_update", args, tmp, task_vars) diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index 2e9d07deb..42193a1c6 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -17,4 +17,4 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck): "prefix": rpm_prefix, "version": openshift_release, } - return self.module_executor("aos_version", args, tmp, task_vars) + return self.execute_module("aos_version", args, tmp, task_vars) diff --git a/roles/openshift_health_checker/test/openshift_check_test.py b/roles/openshift_health_checker/test/openshift_check_test.py index c4c8cd1c2..9cbd5b11e 100644 --- a/roles/openshift_health_checker/test/openshift_check_test.py +++ b/roles/openshift_health_checker/test/openshift_check_test.py @@ -1,6 +1,6 @@ import pytest -from openshift_checks import get_var, OpenShiftCheckException +from openshift_checks import OpenShiftCheck, get_var, OpenShiftCheckException # Fixtures @@ -22,6 +22,41 @@ def missing_keys(request): # Tests +def test_OpenShiftCheck_init(): + class TestCheck(OpenShiftCheck): + name = "test_check" + run = NotImplemented + + # initialization requires at least one argument (apart from self) + with pytest.raises(TypeError) as excinfo: + TestCheck() + assert 'execute_module' in str(excinfo.value) + assert 'module_executor' in str(excinfo.value) + + execute_module = object() + + # initialize with positional argument + check = TestCheck(execute_module) + # new recommended name + assert check.execute_module == execute_module + # deprecated attribute name + assert check.module_executor == execute_module + + # initialize with keyword argument, recommended name + check = TestCheck(execute_module=execute_module) + # new recommended name + assert check.execute_module == execute_module + # deprecated attribute name + assert check.module_executor == execute_module + + # initialize with keyword argument, deprecated name + check = TestCheck(module_executor=execute_module) + # new recommended name + assert check.execute_module == execute_module + # deprecated attribute name + assert check.module_executor == execute_module + + @pytest.mark.parametrize("keys,expected", [ (("foo",), 42), (("bar", "baz"), "openshift"), -- cgit v1.2.3