From 90ee2e3439e17b675db4357a03a5a2c9c1dc3c11 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Fri, 7 Jun 2024 15:44:16 +0530 Subject: [PATCH 1/6] Address ruff PTH120 --- tests/defaults.py | 6 +++--- tests/integration/conftest.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/defaults.py b/tests/defaults.py index 5d9285362..c1bf16523 100644 --- a/tests/defaults.py +++ b/tests/defaults.py @@ -9,11 +9,11 @@ from ansible_navigator.utils.functions import expand_path -FIXTURES_DIR = str(expand_path(os.path.join(os.path.dirname(__file__), "fixtures"))) +FIXTURES_DIR = str(expand_path(os.path.join(Path(__file__).parent, "fixtures"))) FIXTURES_COLLECTION_DIR = expand_path( - os.path.join(os.path.dirname(__file__), "fixtures", "common", "collections"), + os.path.join(Path(__file__).parent, "fixtures", "common", "collections"), ) -FIXTURES_COLLECTION_PATH = Path(FIXTURES_COLLECTION_DIR) +FIXTURES_COLLECTION_PATH = FIXTURES_COLLECTION_DIR class BaseScenario: diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index a8d976c82..697e76519 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -44,7 +44,7 @@ def test_fixtures_dir() -> str: :return: The test fixture directory. """ - return os.path.join(os.path.dirname(__file__), "..", "fixtures") + return os.path.join(Path(__file__).parent, "..", "fixtures") @pytest.fixture(scope="session") From 72cb0e4797745e3ab5cfa598ebe124d7102592c9 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Fri, 7 Jun 2024 16:04:01 +0530 Subject: [PATCH 2/6] Fixes --- src/ansible_navigator/utils/functions.py | 2 +- tests/defaults.py | 8 ++------ tests/integration/conftest.py | 6 ++---- 3 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/ansible_navigator/utils/functions.py b/src/ansible_navigator/utils/functions.py index 55a9cd7fa..570452135 100644 --- a/src/ansible_navigator/utils/functions.py +++ b/src/ansible_navigator/utils/functions.py @@ -60,7 +60,7 @@ def abs_user_path(file_path: str) -> str: return os.path.abspath(os.path.expanduser(os.path.expandvars(file_path))) # noqa:PTH100 -def expand_path(path: str) -> Path: +def expand_path(path: str | Path) -> Path: """Resolve a path. :param path: The file path to resolve diff --git a/tests/defaults.py b/tests/defaults.py index c1bf16523..a94aa0c07 100644 --- a/tests/defaults.py +++ b/tests/defaults.py @@ -1,7 +1,5 @@ """Constants with default values used throughout the tests.""" -import os - from enum import Enum from pathlib import Path from typing import Any @@ -9,10 +7,8 @@ from ansible_navigator.utils.functions import expand_path -FIXTURES_DIR = str(expand_path(os.path.join(Path(__file__).parent, "fixtures"))) -FIXTURES_COLLECTION_DIR = expand_path( - os.path.join(Path(__file__).parent, "fixtures", "common", "collections"), -) +FIXTURES_DIR = str(expand_path(Path(__file__).parent / "fixtures")) +FIXTURES_COLLECTION_DIR = expand_path(Path(__file__).parent / "fixtures" / "common" / "collections") FIXTURES_COLLECTION_PATH = FIXTURES_COLLECTION_DIR diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index 697e76519..b71398727 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -2,8 +2,6 @@ from __future__ import annotations -import os - from copy import deepcopy from pathlib import Path from typing import TYPE_CHECKING @@ -39,12 +37,12 @@ def action_run_stdout() -> Generator[type[ActionRunTest], None, None]: @pytest.fixture(scope="session") -def test_fixtures_dir() -> str: +def test_fixtures_dir() -> Path: """Return the test fixture directory. :return: The test fixture directory. """ - return os.path.join(Path(__file__).parent, "..", "fixtures") + return Path(__file__).parent / ".." / "fixtures" @pytest.fixture(scope="session") From c8a68ec434869a1e5877933ca6331513354e6214 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Fri, 7 Jun 2024 16:54:51 +0530 Subject: [PATCH 3/6] More Fixes --- pyproject.toml | 1 - src/ansible_navigator/actions/collections.py | 12 ++++-------- src/ansible_navigator/actions/doc.py | 5 +++-- src/ansible_navigator/actions/run.py | 4 ++-- .../navigator_post_processor.py | 2 +- src/ansible_navigator/initialization.py | 8 ++++---- src/ansible_navigator/runner/ansible_doc.py | 7 ++++++- 7 files changed, 20 insertions(+), 19 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 677b61cc5..510bdbb7f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -303,7 +303,6 @@ ignore = [ 'PTH110', # `os.path.exists()` should be replaced by `Path.exists()` 'PTH111', # `os.path.expanduser()` should be replaced by `Path.expanduser()` 'PTH118', # `os.path.join()` should be replaced by `Path` with `/` operator - 'PTH120', # `os.path.dirname()` should be replaced by `Path.parent` 'PTH122', # `os.path.splitext()` should be replaced by `Path.suffix` 'RET505', # Unnecessary `else` after `return` statement 'RUF005', # [*] Consider `[self._name, *shlex.split(self._interaction.action.match.groupdict()["params"] or "")]` instead of concatenation diff --git a/src/ansible_navigator/actions/collections.py b/src/ansible_navigator/actions/collections.py index fdca7f276..ca06299c9 100644 --- a/src/ansible_navigator/actions/collections.py +++ b/src/ansible_navigator/actions/collections.py @@ -415,9 +415,9 @@ def _run_runner(self) -> None: } if isinstance(self._args.playbook, str): - playbook_dir = os.path.dirname(self._args.playbook) + playbook_dir = Path(self._args.playbook).parent else: - playbook_dir = os.getcwd() + playbook_dir = Path.cwd() if isinstance(self._args.execution_environment_volume_mounts, list): kwargs["container_volume_mounts"] = self._args.execution_environment_volume_mounts @@ -443,11 +443,7 @@ def _run_runner(self) -> None: if self._args.execution_environment: self._logger.debug("running collections command with execution environment enabled") python_exec_path = "/usr/bin/python3" - utils_lib = os.path.join( - os.path.dirname(__file__), - "..", - "utils", - ) + utils_lib = Path(__file__).parent / ".." / "utils" container_volume_mounts = [ # cache directory which has introspection script @@ -570,7 +566,7 @@ def _parse(self, output: str) -> None: "path" ].startswith(self._adjacent_collection_dir): collection["__type"] = "bind_mount" - elif collection["path"].startswith(os.path.dirname(self._adjacent_collection_dir)): + elif collection["path"].startswith(Path(self._adjacent_collection_dir).parent): collection["__type"] = "bind_mount" error = ( f"{collection['known_as']} was mounted and catalogued in the" diff --git a/src/ansible_navigator/actions/doc.py b/src/ansible_navigator/actions/doc.py index 76fea322d..a2f1fbbf5 100644 --- a/src/ansible_navigator/actions/doc.py +++ b/src/ansible_navigator/actions/doc.py @@ -8,6 +8,7 @@ import shlex import shutil +from pathlib import Path from typing import TYPE_CHECKING from typing import Any @@ -177,9 +178,9 @@ def _run_runner(self) -> dict[Any, Any] | tuple[str, str, int] | None: if self._args.mode == "interactive": if isinstance(self._args.playbook, str): - playbook_dir = os.path.dirname(self._args.playbook) + playbook_dir = Path(self._args.playbook).parent else: - playbook_dir = os.getcwd() + playbook_dir = Path.cwd() kwargs.update({"host_cwd": playbook_dir}) self._runner = AnsibleDoc(**kwargs) diff --git a/src/ansible_navigator/actions/run.py b/src/ansible_navigator/actions/run.py index 4735a8727..a29e1f48b 100644 --- a/src/ansible_navigator/actions/run.py +++ b/src/ansible_navigator/actions/run.py @@ -886,7 +886,7 @@ def write_artifact(self, filename: str | None = None) -> None: playbook = next(k["playbook"] for k in self._plays.value) filename = filename or self._args.playbook_artifact_save_as filename = filename.format( - playbook_dir=os.path.dirname(playbook), + playbook_dir=Path(playbook).parent, playbook_name=os.path.splitext(Path(playbook).name)[0], playbook_status=status, time_stamp=now_iso(self._args.time_zone), @@ -896,7 +896,7 @@ def write_artifact(self, filename: str | None = None) -> None: self._logger.debug("Resolved artifact file name set to %s", filename) try: - Path(os.path.dirname(filename)).mkdir(parents=True, exist_ok=True) + Path(Path(filename).parent).mkdir(parents=True, exist_ok=True) artifact = { "version": "2.0.0", "plays": self._plays.value, diff --git a/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py b/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py index 569e56e92..d41337d9c 100644 --- a/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py +++ b/src/ansible_navigator/configuration_subsystem/navigator_post_processor.py @@ -736,7 +736,7 @@ def log_file(entry: SettingsEntry, config: ApplicationConfiguration) -> PostProc exit_messages: list[ExitMessage] = [] entry.value.current = abs_user_path(entry.value.current) try: - Path(os.path.dirname(entry.value.current)).mkdir(parents=True, exist_ok=True) + Path(Path(entry.value.current).parent).mkdir(parents=True, exist_ok=True) Path(entry.value.current).touch() except (OSError, FileNotFoundError) as exc: exit_msgs = [ diff --git a/src/ansible_navigator/initialization.py b/src/ansible_navigator/initialization.py index c9c626d9d..b1764ec96 100644 --- a/src/ansible_navigator/initialization.py +++ b/src/ansible_navigator/initialization.py @@ -107,17 +107,17 @@ def get_and_check_collection_doc_cache( messages.append(LogMessage(level=logging.DEBUG, message=message)) path_errors = [] - doc_cache_dir = os.path.dirname(collection_doc_cache_path) + doc_cache_dir = Path(collection_doc_cache_path).parent try: - Path(doc_cache_dir).mkdir(parents=True, exist_ok=True) + doc_cache_dir.mkdir(parents=True, exist_ok=True) except OSError as exc: path_errors.append(f"Problem making directory: {doc_cache_dir}") path_errors.append(f"Error was: {exc!s}") - if not os.access(os.path.dirname(collection_doc_cache_path), os.W_OK): + if not os.access(Path(collection_doc_cache_path).parent, os.W_OK): path_errors.append("Directory not writable") - if not os.access(os.path.dirname(collection_doc_cache_path), os.R_OK): + if not os.access(Path(collection_doc_cache_path).parent, os.R_OK): path_errors.append("Directory not readable") if path_errors: diff --git a/src/ansible_navigator/runner/ansible_doc.py b/src/ansible_navigator/runner/ansible_doc.py index 6e6b8a3c9..ac773e5b7 100644 --- a/src/ansible_navigator/runner/ansible_doc.py +++ b/src/ansible_navigator/runner/ansible_doc.py @@ -2,6 +2,7 @@ from __future__ import annotations +from typing import TYPE_CHECKING from typing import Any from ansible_runner import get_plugin_docs @@ -9,6 +10,10 @@ from .base import Base +if TYPE_CHECKING: + from pathlib import Path + + class AnsibleDoc(Base): # pylint: disable=too-many-arguments """An interface to ``ansible-runner`` for running the ``ansible-doc`` command.""" @@ -19,7 +24,7 @@ def fetch_plugin_doc( plugin_type: str | None = None, response_format: str | None = "json", snippet: bool | None = None, - playbook_dir: str | None = None, + playbook_dir: str | Path | None = None, module_path: str | None = None, ) -> tuple[dict[Any, Any] | str, dict[Any, Any] | str]: """Run ``ansible-doc`` command and get the plugin docs related details. From d1cfce5546297af2d42088f06165b1f05827ad9c Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Fri, 14 Jun 2024 13:26:21 +0530 Subject: [PATCH 4/6] rebase --- src/ansible_navigator/utils/functions.py | 16 ++++++++++++---- tests/defaults.py | 8 ++++---- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/ansible_navigator/utils/functions.py b/src/ansible_navigator/utils/functions.py index 8dd9d4495..049961d62 100644 --- a/src/ansible_navigator/utils/functions.py +++ b/src/ansible_navigator/utils/functions.py @@ -12,15 +12,23 @@ import shlex import shutil import zoneinfo + from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING +from typing import Any + +from jinja2 import Environment +from jinja2 import StrictUndefined +from jinja2 import TemplateError -from jinja2 import Environment, StrictUndefined, TemplateError +from .definitions import GOLDEN_RATIO +from .definitions import ExitMessage +from .definitions import LogMessage -from .definitions import GOLDEN_RATIO, ExitMessage, LogMessage if TYPE_CHECKING: - from collections.abc import Iterable, Mapping + from collections.abc import Iterable + from collections.abc import Mapping logger = logging.getLogger(__name__) diff --git a/tests/defaults.py b/tests/defaults.py index 576d2da7d..80def9b65 100644 --- a/tests/defaults.py +++ b/tests/defaults.py @@ -1,14 +1,14 @@ """Constants with default values used throughout the tests.""" from enum import Enum +from pathlib import Path from typing import Any from ansible_navigator.utils.functions import expand_path -FIXTURES_DIR = expand_path(os.path.join(os.path.dirname(__file__), "fixtures")) -FIXTURES_COLLECTION_DIR = expand_path( - os.path.join(os.path.dirname(__file__), "fixtures", "common", "collections"), -) + +FIXTURES_DIR = expand_path(Path(__file__).parent / "fixtures") +FIXTURES_COLLECTION_DIR = expand_path(Path(__file__).parent / "fixtures" / "common" / "collections") FIXTURES_COLLECTION_PATH = FIXTURES_COLLECTION_DIR From 08cc8f6b595889954ebaa5d4385d5118f5f5c967 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Tue, 18 Jun 2024 11:27:30 +0530 Subject: [PATCH 5/6] pre-commit fix --- src/ansible_navigator/actions/run.py | 38 ++++++++++++++++++---------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/ansible_navigator/actions/run.py b/src/ansible_navigator/actions/run.py index 1ee91d431..273e02090 100644 --- a/src/ansible_navigator/actions/run.py +++ b/src/ansible_navigator/actions/run.py @@ -11,39 +11,49 @@ import shutil import time import uuid + from math import floor from operator import itemgetter from pathlib import Path from queue import Queue -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING +from typing import Any from ansible_navigator.action_base import ActionBase from ansible_navigator.action_defs import RunStdoutReturn -from ansible_navigator.configuration_subsystem import to_effective, to_sources -from ansible_navigator.content_defs import ContentView, SerializationFormat +from ansible_navigator.configuration_subsystem import to_effective +from ansible_navigator.configuration_subsystem import to_sources +from ansible_navigator.content_defs import ContentView +from ansible_navigator.content_defs import SerializationFormat from ansible_navigator.runner import CommandAsync from ansible_navigator.steps import Step -from ansible_navigator.ui_framework import (CursesLine, CursesLinePart, - CursesLines, Interaction, - dict_to_form, form_to_dict, - nonblocking_notification, - warning_notification) -from ansible_navigator.utils.functions import (check_playbook_type, - expand_path, human_time, - is_jinja, now_iso, remove_ansi, - round_half_up) +from ansible_navigator.ui_framework import CursesLine +from ansible_navigator.ui_framework import CursesLinePart +from ansible_navigator.ui_framework import CursesLines +from ansible_navigator.ui_framework import Interaction +from ansible_navigator.ui_framework import dict_to_form +from ansible_navigator.ui_framework import form_to_dict +from ansible_navigator.ui_framework import nonblocking_notification +from ansible_navigator.ui_framework import warning_notification +from ansible_navigator.utils.functions import check_playbook_type +from ansible_navigator.utils.functions import expand_path +from ansible_navigator.utils.functions import human_time +from ansible_navigator.utils.functions import is_jinja +from ansible_navigator.utils.functions import now_iso +from ansible_navigator.utils.functions import remove_ansi +from ansible_navigator.utils.functions import round_half_up from ansible_navigator.utils.serialize import serialize_write_file from . import _actions as actions from . import run_action from .stdout import Action as stdout_action + if TYPE_CHECKING: from collections.abc import Callable from ansible_navigator.app_public import AppPublic - from ansible_navigator.configuration_subsystem.definitions import \ - ApplicationConfiguration + from ansible_navigator.configuration_subsystem.definitions import ApplicationConfiguration RESULT_TO_COLOR = [ From 169c9a97737494d9ce3724cd0970974ac47b3eb9 Mon Sep 17 00:00:00 2001 From: shatakshiiii Date: Tue, 18 Jun 2024 11:51:18 +0530 Subject: [PATCH 6/6] more changes --- src/ansible_navigator/actions/collections.py | 6 +++--- src/ansible_navigator/actions/doc.py | 4 ++-- src/ansible_navigator/initialization.py | 2 +- src/ansible_navigator/runner/ansible_doc.py | 7 +------ 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/ansible_navigator/actions/collections.py b/src/ansible_navigator/actions/collections.py index effd30457..e647b141e 100644 --- a/src/ansible_navigator/actions/collections.py +++ b/src/ansible_navigator/actions/collections.py @@ -415,9 +415,9 @@ def _run_runner(self) -> None: } if isinstance(self._args.playbook, str): - playbook_dir = Path(self._args.playbook).parent + playbook_dir = f"{Path(self._args.playbook).parent}" else: - playbook_dir = Path.cwd() + playbook_dir = os.getcwd() if isinstance(self._args.execution_environment_volume_mounts, list): kwargs["container_volume_mounts"] = self._args.execution_environment_volume_mounts @@ -566,7 +566,7 @@ def _parse(self, output: str) -> None: "path" ].startswith(self._adjacent_collection_dir): collection["__type"] = "bind_mount" - elif collection["path"].startswith(Path(self._adjacent_collection_dir).parent): + elif collection["path"].startswith(f"{Path(self._adjacent_collection_dir).parent}"): collection["__type"] = "bind_mount" error = ( f"{collection['known_as']} was mounted and catalogued in the" diff --git a/src/ansible_navigator/actions/doc.py b/src/ansible_navigator/actions/doc.py index a2f1fbbf5..e72e303e6 100644 --- a/src/ansible_navigator/actions/doc.py +++ b/src/ansible_navigator/actions/doc.py @@ -178,9 +178,9 @@ def _run_runner(self) -> dict[Any, Any] | tuple[str, str, int] | None: if self._args.mode == "interactive": if isinstance(self._args.playbook, str): - playbook_dir = Path(self._args.playbook).parent + playbook_dir = f"{Path(self._args.playbook).parent}" else: - playbook_dir = Path.cwd() + playbook_dir = os.getcwd() kwargs.update({"host_cwd": playbook_dir}) self._runner = AnsibleDoc(**kwargs) diff --git a/src/ansible_navigator/initialization.py b/src/ansible_navigator/initialization.py index b78772b39..cde62d0f5 100644 --- a/src/ansible_navigator/initialization.py +++ b/src/ansible_navigator/initialization.py @@ -111,7 +111,7 @@ def get_and_check_collection_doc_cache( try: doc_cache_dir.mkdir(parents=True, exist_ok=True) except OSError as exc: - path_errors.append(f"Problem making directory: {doc_cache_dir}") + path_errors.append(f"Problem making directory: {doc_cache_dir!s}") path_errors.append(f"Error was: {exc!s}") if not os.access(Path(collection_doc_cache_path).parent, os.W_OK): diff --git a/src/ansible_navigator/runner/ansible_doc.py b/src/ansible_navigator/runner/ansible_doc.py index ac773e5b7..6e6b8a3c9 100644 --- a/src/ansible_navigator/runner/ansible_doc.py +++ b/src/ansible_navigator/runner/ansible_doc.py @@ -2,7 +2,6 @@ from __future__ import annotations -from typing import TYPE_CHECKING from typing import Any from ansible_runner import get_plugin_docs @@ -10,10 +9,6 @@ from .base import Base -if TYPE_CHECKING: - from pathlib import Path - - class AnsibleDoc(Base): # pylint: disable=too-many-arguments """An interface to ``ansible-runner`` for running the ``ansible-doc`` command.""" @@ -24,7 +19,7 @@ def fetch_plugin_doc( plugin_type: str | None = None, response_format: str | None = "json", snippet: bool | None = None, - playbook_dir: str | Path | None = None, + playbook_dir: str | None = None, module_path: str | None = None, ) -> tuple[dict[Any, Any] | str, dict[Any, Any] | str]: """Run ``ansible-doc`` command and get the plugin docs related details.