Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor API to use standard types #4240

Merged
merged 4 commits into from
Sep 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 0 additions & 17 deletions .config/pydoclint-baseline.txt
Original file line number Diff line number Diff line change
@@ -1,16 +1,3 @@
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
DOC107: Function `verifiers`: The option `--arg-type-hints-in-signature` is `True` but not all args in the signature have type hints
DOC103: Function `verifiers`: 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: [config: ].
DOC201: Function `verifiers` does not have a return section in docstring
--------------------
src/molecule/command/base.py
DOC303: Class `Base`: The __init__() docstring does not need a "Returns" section, because it cannot return anything
DOC302: Class `Base`: The class docstring does not need a "Returns" section, because __init__() cannot return anything
Expand Down Expand Up @@ -561,10 +548,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
Expand Down
4 changes: 3 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,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

Expand All @@ -88,7 +90,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'
should-document-private-class-attributes = true
show-filenames-in-every-violation-message = true
skip-checking-short-docstrings = false
Expand Down
93 changes: 52 additions & 41 deletions src/molecule/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,41 +5,22 @@
import logging
import traceback

from collections import UserList
from typing import Any
from collections import OrderedDict
from typing import TYPE_CHECKING

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
"""
if TYPE_CHECKING:
from molecule.config import Config

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)
LOG = logging.getLogger(__name__)


class MoleculeRuntimeWarning(RuntimeWarning):
Expand All @@ -51,44 +32,74 @@


@cache
def drivers(config: Any | None = None) -> UserListMap: # noqa: ANN401
def drivers(config: Config | None = None) -> dict[str, Driver]:
"""Return list of active drivers.

Args:
config: plugin config

Returns:
A dictionary of active drivers by name.
"""
plugins = UserListMap()
plugins = OrderedDict()
pm = pluggy.PluginManager("molecule.driver")
try:
pm.load_setuptools_entrypoints("molecule.driver")
except (Exception, SystemExit):
# 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

Check warning on line 58 in src/molecule/api.py

View check run for this annotation

Codecov / codecov/patch

src/molecule/api.py#L57-L58

Added lines #L57 - L58 were not covered by tests
"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)

Check warning on line 65 in src/molecule/api.py

View check run for this annotation

Codecov / codecov/patch

src/molecule/api.py#L64-L65

Added lines #L64 - L65 were not covered by tests

return plugins


@cache
def verifiers(config=None) -> UserListMap: # type: ignore[no-untyped-def] # noqa: ANN001
"""Return list of active verifiers."""
plugins = UserListMap()
def verifiers(config: Config | None = None) -> dict[str, Verifier]:
"""Return list of active verifiers.

Args:
config: plugin config

Returns:
A dictionary of active verifiers by name.
"""
plugins = OrderedDict()
pm = pluggy.PluginManager("molecule.verifier")
try:
pm.load_setuptools_entrypoints("molecule.verifier")
except Exception: # noqa: BLE001
# 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

Check warning on line 94 in src/molecule/api.py

View check run for this annotation

Codecov / codecov/patch

src/molecule/api.py#L94

Added line #L94 was not covered by tests
return plugins


__all__ = (
"Driver",
"IncompatibleMoleculeRuntimeWarning",
"MoleculeRuntimeWarning",
"Verifier",
"drivers",
"verifiers",
)
8 changes: 3 additions & 5 deletions src/molecule/command/drivers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
2 changes: 1 addition & 1 deletion src/molecule/command/reset.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,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()
18 changes: 16 additions & 2 deletions src/molecule/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
if TYPE_CHECKING:
from collections.abc import MutableMapping

from molecule.verifier.base import Verifier


LOG = logging.getLogger(__name__)
MOLECULE_DEBUG = boolean(os.environ.get("MOLECULE_DEBUG", "False"))
Expand Down Expand Up @@ -262,8 +264,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
Expand Down
6 changes: 3 additions & 3 deletions src/molecule/driver/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,17 +269,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
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/driver/delegated.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,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


Expand Down
2 changes: 1 addition & 1 deletion src/molecule/provisioner/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
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():

Check warning on line 68 in src/molecule/shell.py

View check run for this annotation

Codecov / codecov/patch

src/molecule/shell.py#L68

Added line #L68 was not covered by tests
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:"
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/verifier/ansible.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down
8 changes: 6 additions & 2 deletions src/molecule/verifier/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,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
Expand Down
2 changes: 1 addition & 1 deletion src/molecule/verifier/testinfra.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,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__)
Expand Down
5 changes: 2 additions & 3 deletions tests/unit/driver/test_delegated.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,6 @@ def test_ansible_connection_options(_instance): # type: ignore[no-untyped-def]
assert is_subset(x, _instance.ansible_connection_options("foo")) # type: ignore[no-untyped-call]


@pytest.mark.xfail(reason="Needs rewrite since switch to delegated")
@pytest.mark.parametrize(
"config_instance",
["_driver_managed_section_data"], # noqa: PT007
Expand Down Expand Up @@ -256,7 +255,7 @@ def test_ansible_connection_options_when_managed(mocker: MockerFixture, _instanc
),
}

assert ssh_expected_data == _instance.ansible_connection_options("foo")
assert ssh_expected_data.items() <= _instance.ansible_connection_options("foo").items()

winrm_case_data = mocker.patch(
"molecule.driver.delegated.Delegated._get_instance_config",
Expand All @@ -276,7 +275,7 @@ def test_ansible_connection_options_when_managed(mocker: MockerFixture, _instanc
"ansible_connection": "winrm",
}

assert winrm_expected_data == _instance.ansible_connection_options("foo")
assert winrm_expected_data.items() <= _instance.ansible_connection_options("foo").items()


def test_ansible_connection_options_handles_missing_instance_config_managed( # type: ignore[no-untyped-def] # noqa: ANN201, D103
Expand Down
14 changes: 4 additions & 10 deletions tests/unit/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,16 @@
from molecule import api


def test_api_molecule_drivers_as_attributes(): # type: ignore[no-untyped-def] # noqa: ANN201, D103
def test_api_drivers() -> None: # noqa: 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]
for result in results.values():
assert isinstance(result, api.Driver)

assert "default" in results


def test_api_verifiers(): # type: ignore[no-untyped-def] # noqa: ANN201, D103
def test_api_verifiers() -> None: # noqa: D103
x = ["testinfra", "ansible"]

assert all(elem in api.verifiers() for elem in x)
Loading
Loading