From 90a4b160268a2e60d7ed9b58cad8aa4b620d8c98 Mon Sep 17 00:00:00 2001 From: Kate Case Date: Fri, 22 Nov 2024 14:16:59 -0500 Subject: [PATCH] Additional cleanup The typing in utils is a broken mess but should be fixable --- .../dependency/ansible_galaxy/__init__.py | 9 ++--- .../dependency/ansible_galaxy/base.py | 36 ++++++++++--------- .../dependency/ansible_galaxy/collections.py | 10 +++--- .../dependency/ansible_galaxy/roles.py | 10 +++--- src/molecule/dependency/base.py | 17 +++++---- src/molecule/dependency/shell.py | 9 ++--- src/molecule/types.py | 3 +- src/molecule/util.py | 12 ++++--- 8 files changed, 59 insertions(+), 47 deletions(-) diff --git a/src/molecule/dependency/ansible_galaxy/__init__.py b/src/molecule/dependency/ansible_galaxy/__init__.py index b70f016b5..8a9df1fa6 100644 --- a/src/molecule/dependency/ansible_galaxy/__init__.py +++ b/src/molecule/dependency/ansible_galaxy/__init__.py @@ -12,6 +12,7 @@ if TYPE_CHECKING: from molecule.config import Config + from molecule.types import DependencyOptions class AnsibleGalaxy(Base): @@ -127,17 +128,17 @@ def default_env(self) -> dict[str, str]: """ env: dict[str, str] = {} for invoker in self.invocations: - env = util.merge_dicts(env, invoker.default_env) # type: ignore[type-var] + env = util.merge_dicts(env, invoker.default_env) return env @property - def default_options(self) -> dict[str, str | bool]: + def default_options(self) -> DependencyOptions: """Default options across all invokers. Returns: Merged dictionary of default options for all invokers. """ - opts: dict[str, str] = {} + opts: DependencyOptions = {} for invoker in self.invocations: - opts = util.merge_dicts(opts, invoker.default_options) # type: ignore[type-var] + opts = util.merge_dicts(opts, invoker.default_options) return opts diff --git a/src/molecule/dependency/ansible_galaxy/base.py b/src/molecule/dependency/ansible_galaxy/base.py index 9cb4fcb62..284692eb3 100644 --- a/src/molecule/dependency/ansible_galaxy/base.py +++ b/src/molecule/dependency/ansible_galaxy/base.py @@ -25,6 +25,7 @@ import logging import os +from pathlib import Path from typing import TYPE_CHECKING from molecule import util @@ -33,6 +34,7 @@ if TYPE_CHECKING: from molecule.config import Config + from molecule.types import DependencyOptions LOG = logging.getLogger(__name__) @@ -43,9 +45,11 @@ class AnsibleGalaxyBase(base.Base): Attributes: FILTER_OPTS: Keys to remove from the dictionary returned by options(). + COMMANDS: Arguments to send to ansible-galaxy to install the appropriate type of content. """ - FILTER_OPTS = () + FILTER_OPTS: tuple[str, ...] = () + COMMANDS: tuple[str, ...] = () def __init__(self, config: Config) -> None: """Construct AnsibleGalaxy. @@ -54,7 +58,7 @@ def __init__(self, config: Config) -> None: config: Molecule Config instance. """ super().__init__(config) - self._sh_command = None + self._sh_command = [] self.command = "ansible-galaxy" @@ -68,13 +72,13 @@ def requirements_file(self) -> str: # cover """ @property - def default_options(self) -> dict[str, str | bool]: + def default_options(self) -> DependencyOptions: """Default options for this dependency. Returns: Default options for this dependency. """ - d: dict[str, str | bool] = { + d: DependencyOptions = { "force": False, } if self._config.debug: @@ -84,9 +88,9 @@ def default_options(self) -> dict[str, str | bool]: def filter_options( self, - opts: dict[str, str | bool], + opts: DependencyOptions, keys: tuple[str, ...], - ) -> dict[str, str | bool]: + ) -> DependencyOptions: """Filter certain keys from a dictionary. Removes all the values of ``keys`` from the dictionary ``opts``, if @@ -103,26 +107,26 @@ def filter_options( c = copy.copy(opts) for key in keys: if key in c: - del c[key] + del c[key] # type: ignore[misc] return c # NOTE(retr0h): Override the base classes' options() to handle # ``ansible-galaxy`` one-off. @property - def options(self) -> dict[str, str | bool]: + def options(self) -> DependencyOptions: """Computed options for this dependency. Returns: Merged and filtered options for this dependency. """ - o = self._config.config["dependency"]["options"] + opts = self._config.config["dependency"]["options"] # NOTE(retr0h): Remove verbose options added by the user while in # debug. if self._config.debug: - o = util.filter_verbose_permutation(o) + opts = util.filter_verbose_permutation(opts) - o = util.merge_dicts(self.default_options, o) - return self.filter_options(o, self.FILTER_OPTS) + opts = util.merge_dicts(self.default_options, opts) + return self.filter_options(opts, self.FILTER_OPTS) @property def default_env(self) -> dict[str, str]: @@ -132,7 +136,7 @@ def default_env(self) -> dict[str, str]: Default environment variables for this dependency. """ env = dict(os.environ) - return util.merge_dicts(env, self._config.env) # type: ignore[type-var] + return util.merge_dicts(env, self._config.env) def bake(self) -> None: """Bake an ``ansible-galaxy`` command so it's ready to execute and returns None.""" @@ -141,7 +145,7 @@ def bake(self) -> None: self._sh_command = [ self.command, - *self.COMMANDS, # type: ignore[attr-defined] # pylint: disable=no-member + *self.COMMANDS, *util.dict2args(options), *verbose_flag, ] @@ -163,7 +167,7 @@ def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002 LOG.warning(msg) return - if self._sh_command is None: + if not self._sh_command: self.bake() self._setup() @@ -173,4 +177,4 @@ def _setup(self) -> None: """Prepare the system for using ``ansible-galaxy`` and returns None.""" def _has_requirements_file(self) -> bool: - return os.path.isfile(self.requirements_file) # noqa: PTH113 + return Path(self.requirements_file).is_file() diff --git a/src/molecule/dependency/ansible_galaxy/collections.py b/src/molecule/dependency/ansible_galaxy/collections.py index 7c99206da..704a1afa7 100644 --- a/src/molecule/dependency/ansible_galaxy/collections.py +++ b/src/molecule/dependency/ansible_galaxy/collections.py @@ -3,8 +3,8 @@ from __future__ import annotations import logging -import os +from pathlib import Path from typing import TYPE_CHECKING from molecule import util @@ -35,9 +35,11 @@ def default_options(self) -> DependencyOptions: specific = util.merge_dicts( general, { - "requirements-file": os.path.join( # noqa: PTH118 - self._config.scenario.directory, - "collections.yml", + "requirements-file": str( + Path( + self._config.scenario.directory, + "collections.yml", + ), ), }, ) diff --git a/src/molecule/dependency/ansible_galaxy/roles.py b/src/molecule/dependency/ansible_galaxy/roles.py index f7a6bed4e..939891685 100644 --- a/src/molecule/dependency/ansible_galaxy/roles.py +++ b/src/molecule/dependency/ansible_galaxy/roles.py @@ -3,8 +3,8 @@ from __future__ import annotations import logging -import os +from pathlib import Path from typing import TYPE_CHECKING from molecule import util @@ -35,9 +35,11 @@ def default_options(self) -> DependencyOptions: specific = util.merge_dicts( general, { - "role-file": os.path.join( # noqa: PTH118 - self._config.scenario.directory, - "requirements.yml", + "role-file": str( + Path( + self._config.scenario.directory, + "requirements.yml", + ), ), }, ) diff --git a/src/molecule/dependency/base.py b/src/molecule/dependency/base.py index e8f7069be..f2a5cb5e5 100644 --- a/src/molecule/dependency/base.py +++ b/src/molecule/dependency/base.py @@ -33,6 +33,7 @@ if TYPE_CHECKING: from molecule.config import Config + from molecule.types import DependencyOptions LOG = logging.getLogger(__name__) @@ -58,14 +59,12 @@ def __init__(self, config: Config) -> None: config: An instance of a Molecule config. """ self._config = config - self._sh_command: str | list[str] | None = None + self._sh_command: list[str] = [] def execute_with_retries(self) -> None: """Run dependency downloads with retry and timed back-off.""" - exception = None - try: - util.run_command(self._sh_command, debug=self._config.debug, check=True) # type: ignore[arg-type] + util.run_command(self._sh_command, debug=self._config.debug, check=True) msg = "Dependency completed successfully." LOG.info(msg) return # noqa: TRY300 @@ -82,7 +81,7 @@ def execute_with_retries(self) -> None: self.SLEEP += self.BACKOFF try: - util.run_command(self._sh_command, debug=self._config.debug, check=True) # type: ignore[arg-type] + util.run_command(self._sh_command, debug=self._config.debug, check=True) msg = "Dependency completed successfully." LOG.info(msg) return # noqa: TRY300 @@ -90,7 +89,7 @@ def execute_with_retries(self) -> None: exception = _exception LOG.error(str(exception)) - util.sysexit(exception.returncode) # type: ignore[union-attr] + util.sysexit(exception.returncode) @abc.abstractmethod def execute( @@ -107,7 +106,7 @@ def execute( @property @abc.abstractmethod - def default_options(self) -> dict[str, str | bool]: # pragma: no cover + def default_options(self) -> DependencyOptions: # pragma: no cover """Get default CLI arguments provided to ``cmd``. Returns: @@ -125,7 +124,7 @@ def default_env(self) -> dict[str, str]: # pragma: no cover # dict[str, str] should fit the typevar of merge_dicts, and all types are the same, yet # it still complains. env = dict(os.environ) - return util.merge_dicts(env, self._config.env) # type: ignore[type-var] + return util.merge_dicts(env, self._config.env) @property def name(self) -> str: @@ -145,7 +144,7 @@ def enabled(self) -> bool: return self._config.config["dependency"]["enabled"] @property - def options(self) -> dict[str, str | bool]: + def options(self) -> DependencyOptions: """Computed dependency options. Returns: diff --git a/src/molecule/dependency/shell.py b/src/molecule/dependency/shell.py index 48ea28134..990429f0e 100644 --- a/src/molecule/dependency/shell.py +++ b/src/molecule/dependency/shell.py @@ -29,6 +29,7 @@ if TYPE_CHECKING: from molecule.config import Config + from molecule.types import DependencyOptions LOG = logging.getLogger(__name__) @@ -81,7 +82,7 @@ def __init__(self, config: Config) -> None: config: Molecule Config instance. """ super().__init__(config) - self._sh_command: str | None = None + self._sh_command: list[str] = [] @property def command(self) -> str: @@ -93,7 +94,7 @@ def command(self) -> str: return self._config.config["dependency"]["command"] or "" @property - def default_options(self) -> dict[str, str | bool]: + def default_options(self) -> DependencyOptions: """Get default options for shell dependencies (none). Returns: @@ -103,7 +104,7 @@ def default_options(self) -> dict[str, str | bool]: def bake(self) -> None: """Bake a ``shell`` command so it's ready to execute.""" - self._sh_command = self.command + self._sh_command = [self.command] def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002 """Execute the dependency solver. @@ -117,7 +118,7 @@ def execute(self, action_args: list[str] | None = None) -> None: # noqa: ARG002 return super().execute() - if self._sh_command is None: + if not self._sh_command: self.bake() self.execute_with_retries() diff --git a/src/molecule/types.py b/src/molecule/types.py index cc7293da9..37b3ed916 100644 --- a/src/molecule/types.py +++ b/src/molecule/types.py @@ -9,9 +9,10 @@ from typing import Any +# We have to use the alternate form here because dashes are invalid in python identifiers DependencyOptions = TypedDict( "DependencyOptions", - {"force": bool, "requirements-file": str, "role-file": str, "vvv": bool}, + {"force": bool, "requirements-file": str, "role-file": str, "verbose": bool, "vvv": bool}, total=False, ) diff --git a/src/molecule/util.py b/src/molecule/util.py index 68b599ab1..2c1e2ff54 100644 --- a/src/molecule/util.py +++ b/src/molecule/util.py @@ -50,10 +50,12 @@ from ansible_compat.types import JSON - from molecule.types import CommandArgs, ConfigData, PlatformData + from molecule.types import CommandArgs, ConfigData, DependencyOptions, PlatformData NestedDict = MutableMapping[str, JSON] _T = TypeVar("_T", bound=NestedDict) + GenericOptions = dict[str, str | bool] + Options = TypeVar("Options", bound=(GenericOptions | DependencyOptions)) LOG = logging.getLogger(__name__) @@ -357,7 +359,7 @@ def instance_with_scenario_name(instance_name: str, scenario_name: str) -> str: return f"{instance_name}-{scenario_name}" -def verbose_flag(options: MutableMapping[str, Any]) -> list[str]: +def verbose_flag(options: Options) -> list[str]: """Return computed verbosity flag. Args: @@ -380,7 +382,7 @@ def verbose_flag(options: MutableMapping[str, Any]) -> list[str]: return flags -def filter_verbose_permutation(options: dict[str, Any]) -> dict[str, Any]: +def filter_verbose_permutation(options: Options) -> Options: """Clean verbose information. Args: @@ -557,7 +559,7 @@ def boolean(value: bool | AnyStr, *, strict: bool = True) -> bool: ) -def dict2args(data: dict[str, str | bool]) -> list[str]: +def dict2args(data: Options) -> list[str]: """Convert a dictionary of options to command like arguments. Args: @@ -566,7 +568,7 @@ def dict2args(data: dict[str, str | bool]) -> list[str]: Returns: A list of command-like flags represented by the dictionary. """ - result = [] + result: list[str] = [] # keep sorting in order to achieve a predictable behavior for k, v in sorted(data.items()): if v is not False: