From 06756cc08c27a451c777939d9ff928264d1af0b4 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Wed, 15 Nov 2023 16:14:39 +0000 Subject: [PATCH] Migrate `tasks/loader.py` -> `tools/precommit/loader.py` Refs #64374 Signed-off-by: Pedro Algarvio --- .pre-commit-config.yaml | 34 ++++++------- changelog/64374.fixed.md | 1 + tools/__init__.py | 1 + tools/precommit/__init__.py | 40 +++++++++++++++ tools/precommit/docstrings.py | 4 +- {tasks => tools/precommit}/loader.py | 74 ++++++++++++++-------------- 6 files changed, 94 insertions(+), 60 deletions(-) rename {tasks => tools/precommit}/loader.py (58%) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 038c8c1344d..7cf56cf8c9d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -115,6 +115,20 @@ repos: - docstrings - check + - id: tools + alias: loader-check-virtual + name: Check loader modules __virtual__ + files: salt/.*\.py$ + exclude: > + (?x)^( + templates/.*| + salt/ext/.*| + )$ + args: + - pre-commit + - salt-loaders + - check-virtual + # ----- Packaging Requirements ------------------------------------------------------------------------------------> - repo: https://github.com/saltstack/pip-tools-compile-impersonate @@ -1247,26 +1261,6 @@ repos: - packaging - looseversion - - id: invoke - alias: loader-check-virtual - name: Check loader modules __virtual__ - files: salt/.*\.py$ - exclude: > - (?x)^( - templates/.*| - salt/ext/.*| - )$ - args: - - loader.check-virtual - additional_dependencies: - - blessings==1.7 - - pyyaml==6.0.1 - - distro==1.7.0 - - jinja2==3.0.3 - - msgpack==1.0.3 - - packaging - - looseversion - - repo: https://github.com/saltstack/invoke-pre-commit rev: v1.9.0 hooks: diff --git a/changelog/64374.fixed.md b/changelog/64374.fixed.md index 479dc6c8c1b..8b94be869d7 100644 --- a/changelog/64374.fixed.md +++ b/changelog/64374.fixed.md @@ -2,3 +2,4 @@ Migrated some [`invoke`](https://www.pyinvoke.org/) tasks to [`python-tools-scri * `tasks/docs.py` -> `tools/precommit/docs.py` * `tasks/docstrings.py` -> `tools/precommit/docstrings.py` +* `tasks/loader.py` -> `tools/precommit/loader.py` diff --git a/tools/__init__.py b/tools/__init__.py index f78eaf92a2c..1b34b867966 100644 --- a/tools/__init__.py +++ b/tools/__init__.py @@ -44,6 +44,7 @@ ptscripts.register_tools_module("tools.precommit.changelog") ptscripts.register_tools_module("tools.precommit.workflows") ptscripts.register_tools_module("tools.precommit.docs") ptscripts.register_tools_module("tools.precommit.docstrings") +ptscripts.register_tools_module("tools.precommit.loader") ptscripts.register_tools_module("tools.release", venv_config=RELEASE_VENV_CONFIG) ptscripts.register_tools_module("tools.testsuite") ptscripts.register_tools_module("tools.testsuite.download") diff --git a/tools/precommit/__init__.py b/tools/precommit/__init__.py index 57d9d1ae62a..c10eadeb479 100644 --- a/tools/precommit/__init__.py +++ b/tools/precommit/__init__.py @@ -3,7 +3,47 @@ These commands, and sub-commands, are used by pre-commit. """ from ptscripts import command_group +import tools.utils + # Define the command group cgroup = command_group( name="pre-commit", help="Pre-Commit Related Commands", description=__doc__ ) + +SALT_BASE_PATH = tools.utils.REPO_ROOT / "salt" + +SALT_INTERNAL_LOADERS_PATHS = ( + # This is a 1:1 copy of SALT_INTERNAL_LOADERS_PATHS found in salt/loader/__init__.py + str(SALT_BASE_PATH / "auth"), + str(SALT_BASE_PATH / "beacons"), + str(SALT_BASE_PATH / "cache"), + str(SALT_BASE_PATH / "client" / "ssh" / "wrapper"), + str(SALT_BASE_PATH / "cloud" / "clouds"), + str(SALT_BASE_PATH / "engines"), + str(SALT_BASE_PATH / "executors"), + str(SALT_BASE_PATH / "fileserver"), + str(SALT_BASE_PATH / "grains"), + str(SALT_BASE_PATH / "log_handlers"), + str(SALT_BASE_PATH / "matchers"), + str(SALT_BASE_PATH / "metaproxy"), + str(SALT_BASE_PATH / "modules"), + str(SALT_BASE_PATH / "netapi"), + str(SALT_BASE_PATH / "output"), + str(SALT_BASE_PATH / "pillar"), + str(SALT_BASE_PATH / "proxy"), + str(SALT_BASE_PATH / "queues"), + str(SALT_BASE_PATH / "renderers"), + str(SALT_BASE_PATH / "returners"), + str(SALT_BASE_PATH / "roster"), + str(SALT_BASE_PATH / "runners"), + str(SALT_BASE_PATH / "sdb"), + str(SALT_BASE_PATH / "serializers"), + str(SALT_BASE_PATH / "spm" / "pkgdb"), + str(SALT_BASE_PATH / "spm" / "pkgfiles"), + str(SALT_BASE_PATH / "states"), + str(SALT_BASE_PATH / "thorium"), + str(SALT_BASE_PATH / "tokens"), + str(SALT_BASE_PATH / "tops"), + str(SALT_BASE_PATH / "utils"), + str(SALT_BASE_PATH / "wheel"), +) diff --git a/tools/precommit/docstrings.py b/tools/precommit/docstrings.py index 37aea8b8c16..9cbc5a848d0 100644 --- a/tools/precommit/docstrings.py +++ b/tools/precommit/docstrings.py @@ -16,8 +16,8 @@ from typing import TYPE_CHECKING from ptscripts import Context, command_group import tools.utils -from salt.loader import SALT_INTERNAL_LOADERS_PATHS from salt.version import SaltStackVersion +from tools.precommit import SALT_INTERNAL_LOADERS_PATHS SALT_CODE_DIR = tools.utils.REPO_ROOT / "salt" SALT_MODULES_PATH = SALT_CODE_DIR / "modules" @@ -865,7 +865,7 @@ def check_docstrings( Check salt's docstrings """ if not files: - _files = SALT_CODE_DIR.rglob("*.py") + _files = list(SALT_CODE_DIR.rglob("*.py")) else: _files = [fpath.resolve() for fpath in files if fpath.suffix == ".py"] diff --git a/tasks/loader.py b/tools/precommit/loader.py similarity index 58% rename from tasks/loader.py rename to tools/precommit/loader.py index d65e5e28591..bbec9c00f92 100644 --- a/tasks/loader.py +++ b/tools/precommit/loader.py @@ -1,24 +1,35 @@ """ - tasks.loader - ~~~~~~~~~~~~ - - Salt loader checks +Salt loader checks """ import ast import pathlib -from invoke import task # pylint: disable=3rd-party-module-not-gated +from ptscripts import Context, command_group -from salt.loader import SALT_INTERNAL_LOADERS_PATHS -from tasks import utils +import tools.utils +from tools.precommit import SALT_INTERNAL_LOADERS_PATHS -CODE_DIR = pathlib.Path(__file__).resolve().parent.parent -SALT_CODE_DIR = CODE_DIR / "salt" +SALT_CODE_DIR = tools.utils.REPO_ROOT / "salt" + +cgroup = command_group(name="salt-loaders", help=__doc__, parent="pre-commit") -@task(iterable=["files"], positional=["files"]) -def check_virtual(ctx, files, enforce_virtualname=False): +@cgroup.command( + name="check-virtual", + arguments={ + "files": { + "help": "List of files to check", + "nargs": "*", + }, + "enforce_virtualname": { + "help": "Enforce the usage of `__virtualname__`", + }, + }, +) +def check_virtual( + ctx: Context, files: list[pathlib.Path], enforce_virtualname: bool = False +) -> None: """ Check Salt loader modules for a defined `__virtualname__` attribute and `__virtual__` function. @@ -26,23 +37,10 @@ def check_virtual(ctx, files, enforce_virtualname=False): https://github.com/saltstack/salt/blob/27ae8260983b11fe6e32a18e777d550be9fe1dc2/tests/unit/test_virtualname.py """ - # CD into Salt's repo root directory - ctx.cd(CODE_DIR) - - # Unfortunately invoke does not support nargs. - # We migth have been passed --files="foo.py bar.py" - # Turn that into a list of paths - _files = [] - for path in files: - if not path: - continue - _files.extend(path.split()) - if not _files: - _files = SALT_CODE_DIR.rglob("*.py") + if not files: + _files = list(SALT_CODE_DIR.rglob("*.py")) else: - _files = [pathlib.Path(fname) for fname in _files] - - _files = [path.resolve() for path in _files] + _files = [fpath.resolve() for fpath in files if fpath.suffix == ".py"] errors = 0 exitcode = 0 @@ -78,14 +76,15 @@ def check_virtual(ctx, files, enforce_virtualname=False): continue if target.id == "__virtualname__": found_virtualname_attr = True - if node.value.s not in path.name: + if node.value.s not in path.name: # type: ignore[attr-defined] errors += 1 exitcode = 1 - utils.error( + ctx.error( 'The value of the __virtualname__ attribute, "{}"' - " is not part of {}", - node.value.s, - path.name, + " is not part of {}".format( + node.value.s, # type: ignore[attr-defined] + path.name, + ) ) if found_virtualname_attr: break @@ -93,11 +92,10 @@ def check_virtual(ctx, files, enforce_virtualname=False): if not found_virtualname_attr and enforce_virtualname: errors += 1 exitcode = 1 - utils.error( - "The salt loader module {} defines a __virtual__() function but does" - " not define a __virtualname__ attribute", - path.relative_to(CODE_DIR), + ctx.error( + f"The salt loader module {path.relative_to(tools.utils.REPO_ROOT)} defines " + "a __virtual__() function but does not define a __virtualname__ attribute" ) if exitcode: - utils.error("Found {} errors", errors) - utils.exit_invoke(exitcode) + ctx.error(f"Found {errors} errors") + ctx.exit(exitcode)