From e9d7d03666d81bfb98ec5a9c1924005db990f998 Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Wed, 26 Jun 2024 10:56:25 +0100 Subject: [PATCH] Refactor API to use standard types --- .config/pydoclint-baseline.txt | 9 ---- pyproject.toml | 4 +- src/molecule/api.py | 81 ++++++++++++++--------------- src/molecule/command/drivers.py | 8 ++- src/molecule/command/reset.py | 2 +- src/molecule/config.py | 17 +++++- src/molecule/driver/base.py | 6 +-- src/molecule/driver/delegated.py | 2 +- src/molecule/provisioner/ansible.py | 2 +- src/molecule/shell.py | 2 +- src/molecule/verifier/ansible.py | 2 +- src/molecule/verifier/base.py | 8 ++- src/molecule/verifier/testinfra.py | 2 +- tests/unit/test_api.py | 8 +-- 14 files changed, 77 insertions(+), 76 deletions(-) diff --git a/.config/pydoclint-baseline.txt b/.config/pydoclint-baseline.txt index 367bcff89..a2cbdcf68 100644 --- a/.config/pydoclint-baseline.txt +++ b/.config/pydoclint-baseline.txt @@ -1,9 +1,4 @@ src/molecule/api.py - DOC101: Method `UserListMap.__getitem__`: Docstring contains fewer arguments than in function signature. - DOC106: Method `UserListMap.__getitem__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature - DOC107: Method `UserListMap.__getitem__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints - DOC103: Method `UserListMap.__getitem__`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [i: ]. - DOC201: Method `UserListMap.__getitem__` does not have a return section in docstring DOC201: Function `drivers` does not have a return section in docstring DOC101: Function `verifiers`: Docstring contains fewer arguments than in function signature. DOC106: Function `verifiers`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature @@ -565,10 +560,6 @@ src/molecule/verifier/base.py DOC603: Class `Verifier`: Class docstring attributes are different from actual class attributes. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Attributes in the class definition but not in the docstring: [__metaclass__: ]. (Please read https://jsh9.github.io/pydoclint/checking_class_attributes.html on how to correctly document class attributes.) DOC106: Method `Verifier.__init__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature DOC107: Method `Verifier.__init__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints - DOC101: Method `Verifier.execute`: Docstring contains fewer arguments than in function signature. - DOC106: Method `Verifier.execute`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature - DOC107: Method `Verifier.execute`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints - DOC103: Method `Verifier.execute`: Docstring arguments are different from function arguments. (Or could be other formatting issues: https://jsh9.github.io/pydoclint/violation_codes.html#notes-on-doc103 ). Arguments in the function signature but not in the docstring: [action_args: ]. DOC101: Method `Verifier.__eq__`: Docstring contains fewer arguments than in function signature. DOC106: Method `Verifier.__eq__`: The option `--arg-type-hints-in-signature` is `True` but there are no argument type hints in the signature DOC107: Method `Verifier.__eq__`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints diff --git a/pyproject.toml b/pyproject.toml index 6eab3a2b3..0f5795877 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -72,6 +72,8 @@ source_pkgs = ["molecule"] [tool.mypy] cache_dir = "./.cache/.mypy" +# To ensure that calling `mypy .` produces desired results (vscode) +exclude = ["build"] files = ["src", "tests"] strict = true @@ -87,7 +89,7 @@ allow-init-docstring = true arg-type-hints-in-docstring = false baseline = ".config/pydoclint-baseline.txt" check-return-types = false -exclude = '\.git|\.tox|build|out|venv' +exclude = '\.cache|\.git|\.tox|build|out|venv' show-filenames-in-every-violation-message = true skip-checking-short-docstrings = false style = "google" diff --git a/src/molecule/api.py b/src/molecule/api.py index 88d388f40..fe320afec 100644 --- a/src/molecule/api.py +++ b/src/molecule/api.py @@ -3,41 +3,20 @@ import logging import traceback -from collections import UserList -from typing import Any +from collections import OrderedDict +from collections.abc import Hashable +from typing import Any, TypeVar import pluggy from ansible_compat.ports import cache -from molecule.driver.base import Driver # noqa: F401 -from molecule.verifier.base import Verifier # noqa: F401 +from molecule.driver.base import Driver +from molecule.verifier.base import Verifier LOG = logging.getLogger(__name__) - - -class UserListMap(UserList): # type: ignore[type-arg] - """A list where you can also access elements by their name. - - Example: - ------- - foo['boo'] - foo.boo - """ - - def __getitem__(self, i): # type: ignore[no-untyped-def] # noqa: ANN001, ANN204 - """Implement indexing.""" - if isinstance(i, int): - return super().__getitem__(i) - return self.__dict__[i] - - def get(self, key, default): # type: ignore[no-untyped-def] # noqa: ANN001, ANN201, D102 - return self.__dict__.get(key, default) - - def append(self, item) -> None: # type: ignore[no-untyped-def] # noqa: ANN001, D102 - self.__dict__[str(item)] = item - super().append(item) +K = TypeVar("K", bound=Hashable) class MoleculeRuntimeWarning(RuntimeWarning): @@ -49,13 +28,13 @@ class IncompatibleMoleculeRuntimeWarning(MoleculeRuntimeWarning): @cache -def drivers(config: Any | None = None) -> UserListMap: # noqa: ANN401 +def drivers(config: Any | None = None) -> dict[str, Driver]: # noqa: ANN401 """Return list of active drivers. Args: config: plugin config """ - plugins = UserListMap() + plugins = OrderedDict() pm = pluggy.PluginManager("molecule.driver") try: pm.load_setuptools_entrypoints("molecule.driver") @@ -63,19 +42,28 @@ def drivers(config: Any | None = None) -> UserListMap: # noqa: ANN401 # These are not fatal because a broken driver should not make the entire # tool unusable. LOG.error("Failed to load driver entry point %s", traceback.format_exc()) # noqa: TRY400 - for p in pm.get_plugins(): - try: - plugins.append(p(config)) - except (Exception, SystemExit) as e: # noqa: PERF203 - LOG.error("Failed to load %s driver: %s", pm.get_name(p), str(e)) # noqa: TRY400 - plugins.sort() + for plugin in pm.get_plugins(): + if issubclass(plugin, Driver): + try: + driver = plugin(config) + plugins[driver.name] = driver + except (Exception, SystemExit, TypeError) as e: + LOG.error( # noqa: TRY400 + "Failed to load %s driver: %s", + pm.get_name(plugin), + str(e), + ) + else: + msg = f"Skipped loading plugin class {plugin} because is not a subclass of Driver." + LOG.error(msg) + return plugins @cache -def verifiers(config=None) -> UserListMap: # type: ignore[no-untyped-def] # noqa: ANN001 +def verifiers(config=None) -> dict[str, Verifier]: # type: ignore[no-untyped-def] # noqa: ANN001 """Return list of active verifiers.""" - plugins = UserListMap() + plugins = OrderedDict() pm = pluggy.PluginManager("molecule.verifier") try: pm.load_setuptools_entrypoints("molecule.verifier") @@ -83,10 +71,21 @@ def verifiers(config=None) -> UserListMap: # type: ignore[no-untyped-def] # no # These are not fatal because a broken verifier should not make the entire # tool unusable. LOG.error("Failed to load verifier entry point %s", traceback.format_exc()) # noqa: TRY400 - for p in pm.get_plugins(): + for plugin_class in pm.get_plugins(): try: - plugins.append(p(config)) + if issubclass(plugin_class, Verifier): + plugin = plugin_class(config) + plugins[plugin.name] = plugin except Exception as e: # noqa: BLE001, PERF203 - LOG.error("Failed to load %s driver: %s", pm.get_name(p), str(e)) # noqa: TRY400 - plugins.sort() + LOG.error("Failed to load %s driver: %s", pm.get_name(plugin), str(e)) # noqa: TRY400 return plugins + + +__all__ = ( + "Driver", + "IncompatibleMoleculeRuntimeWarning", + "MoleculeRuntimeWarning", + "Verifier", + "drivers", + "verifiers", +) diff --git a/src/molecule/command/drivers.py b/src/molecule/command/drivers.py index 501880591..f27e38d7f 100644 --- a/src/molecule/command/drivers.py +++ b/src/molecule/command/drivers.py @@ -44,11 +44,9 @@ def drivers(ctx, format): # type: ignore[no-untyped-def] # pragma: no cover # noqa: ANN001, ANN201, A002, ARG001 """List drivers.""" drivers = [] # pylint: disable=redefined-outer-name - for driver in api.drivers(): - description = driver - if format != "plain": - description = driver - else: + for driver in api.drivers().values(): + description = str(driver) + if format == "plain": description = f"{driver!s:16s}[logging.level.notset] {driver.title} Version {driver.version} from {driver.module} python module.)[/logging.level.notset]" # noqa: E501 drivers.append([driver, description]) console.print(description) diff --git a/src/molecule/command/reset.py b/src/molecule/command/reset.py index 9bf824b21..d433f6486 100644 --- a/src/molecule/command/reset.py +++ b/src/molecule/command/reset.py @@ -45,5 +45,5 @@ def reset(ctx, scenario_name): # type: ignore[no-untyped-def] # pragma: no cove command_args = {"subcommand": subcommand} base.execute_cmdline_scenarios(scenario_name, args, command_args) - for driver in drivers(): + for driver in drivers().values(): driver.reset() diff --git a/src/molecule/config.py b/src/molecule/config.py index df06f0e66..0452ddcd9 100644 --- a/src/molecule/config.py +++ b/src/molecule/config.py @@ -38,6 +38,7 @@ from molecule.model import schema_v3 from molecule.provisioner import ansible from molecule.util import boolean +from molecule.verifier.base import Verifier LOG = logging.getLogger(__name__) @@ -257,8 +258,20 @@ def state(self): # type: ignore[no-untyped-def] # noqa: ANN201, D102 return state.State(self) @cached_property - def verifier(self): # type: ignore[no-untyped-def] # noqa: ANN201, D102 - return api.verifiers(self).get(self.config["verifier"]["name"], None) # type: ignore[no-untyped-call] + def verifier(self) -> Verifier: + """Retrieve current verifier. + + Raises: + RuntimeError: If is not able to find the driver. + + Returns: + Instance of Verifier driver. + """ + name = self.config["verifier"]["name"] + if name not in api.verifiers(self): + msg = f"Unable to find '{name}' verifier driver." + raise RuntimeError(msg) + return api.verifiers(self)[name] def _get_driver_name(self): # type: ignore[no-untyped-def] # noqa: ANN202 # the state file contains the driver from the last run diff --git a/src/molecule/driver/base.py b/src/molecule/driver/base.py index 20b73b007..6a052dc21 100644 --- a/src/molecule/driver/base.py +++ b/src/molecule/driver/base.py @@ -264,17 +264,17 @@ def get_playbook(self, step): # type: ignore[no-untyped-def] # noqa: ANN001, A return p return None - def schema_file(self): # type: ignore[no-untyped-def] # noqa: ANN201, D102 + def schema_file(self) -> None | str: # noqa: D102 return None - def modules_dir(self): # type: ignore[no-untyped-def] # noqa: ANN201 + def modules_dir(self) -> str | None: """Return path to ansible modules included with driver.""" p = os.path.join(self._path, "modules") # noqa: PTH118 if os.path.isdir(p): # noqa: PTH112 return p return None - def reset(self): # type: ignore[no-untyped-def] # noqa: ANN201 + def reset(self) -> None: """Release all resources owned by molecule. This is a destructive operation that would affect all resources managed diff --git a/src/molecule/driver/delegated.py b/src/molecule/driver/delegated.py index f729c17c1..4690ce261 100644 --- a/src/molecule/driver/delegated.py +++ b/src/molecule/driver/delegated.py @@ -23,7 +23,7 @@ import os from molecule import util -from molecule.api import Driver # type: ignore[attr-defined] +from molecule.api import Driver from molecule.data import __file__ as data_module diff --git a/src/molecule/provisioner/ansible.py b/src/molecule/provisioner/ansible.py index 20bd3a7b8..631e7b92e 100644 --- a/src/molecule/provisioner/ansible.py +++ b/src/molecule/provisioner/ansible.py @@ -920,7 +920,7 @@ def _get_modules_directories(self) -> list[str]: util.abs_path(os.path.join(self._get_plugin_directory(), "modules")), # noqa: PTH118 ) - for d in drivers(): + for d in drivers().values(): p = d.modules_dir() if p: paths.append(p) diff --git a/src/molecule/shell.py b/src/molecule/shell.py index c20539f57..ff48cbce9 100644 --- a/src/molecule/shell.py +++ b/src/molecule/shell.py @@ -63,7 +63,7 @@ def print_version(ctx, param, value): # type: ignore[no-untyped-def] # noqa: A msg = f"molecule [{color}]{v}[/] using python [repr.number]{sys.version_info[0]}.{sys.version_info[1]}[/] \n" # noqa: E501 msg += f" [repr.attrib_name]ansible[/][dim]:[/][repr.number]{app.runtime.version}[/]" - for driver in drivers(): + for driver in drivers().values(): msg += f"\n [repr.attrib_name]{driver!s}[/][dim]:[/][repr.number]{driver.version}[/][dim] from {driver.module}" # noqa: E501 if driver.required_collections: msg += " requiring collections:" diff --git a/src/molecule/verifier/ansible.py b/src/molecule/verifier/ansible.py index 93bd3df55..6eb8c6c83 100644 --- a/src/molecule/verifier/ansible.py +++ b/src/molecule/verifier/ansible.py @@ -23,7 +23,7 @@ import os from molecule import util -from molecule.api import Verifier # type: ignore[attr-defined] +from molecule.api import Verifier log = logging.getLogger(__name__) diff --git a/src/molecule/verifier/base.py b/src/molecule/verifier/base.py index 175c9f0fc..b95462756 100644 --- a/src/molecule/verifier/base.py +++ b/src/molecule/verifier/base.py @@ -65,8 +65,12 @@ def default_env(self): # type: ignore[no-untyped-def] # pragma: no cover # noq """ @abc.abstractmethod - def execute(self, action_args=None): # type: ignore[no-untyped-def] # pragma: no cover # noqa: ANN001, ANN201 - """Execute ``cmd`` and returns None.""" + def execute(self, action_args: None | list[str] = None) -> None: # pragma: no cover + """Execute ``cmd`` and returns None. + + Args: + action_args: list of arguments to be passed. + """ @abc.abstractmethod def schema(self): # type: ignore[no-untyped-def] # pragma: no cover # noqa: ANN201 diff --git a/src/molecule/verifier/testinfra.py b/src/molecule/verifier/testinfra.py index de78d48a0..ec1e4f2f1 100644 --- a/src/molecule/verifier/testinfra.py +++ b/src/molecule/verifier/testinfra.py @@ -24,7 +24,7 @@ import os from molecule import util -from molecule.api import Verifier # type: ignore[attr-defined] +from molecule.api import Verifier LOG = logging.getLogger(__name__) diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 15299a7f7..6096c5da0 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -22,17 +22,11 @@ from molecule import api -def test_api_molecule_drivers_as_attributes(): # type: ignore[no-untyped-def] # noqa: ANN201, D103 - results = api.drivers() - assert hasattr(results, "default") - assert isinstance(results.default, api.Driver) # type: ignore[attr-defined] # pylint:disable=no-member - - def test_api_drivers(): # type: ignore[no-untyped-def] # noqa: ANN201, D103 results = api.drivers() for result in results: - assert isinstance(result, api.Driver) # type: ignore[attr-defined] + assert isinstance(result, api.Driver) assert "default" in results