From 7207f4e24d9bd8315a5c75d3a1fb8ab76cd93466 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Sat, 27 May 2023 19:14:29 +0100 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 | 3 ++ tools/precommit/docstrings.py | 2 +- {tasks => tools/precommit}/loader.py | 72 ++++++++++++++-------------- 5 files changed, 54 insertions(+), 58 deletions(-) rename {tasks => tools/precommit}/loader.py (60%) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 34bf67d3abd5..bc61c3d7e5fd 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 @@ -1224,26 +1238,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 - - distro==1.7.0 - - jinja2==3.0.3 - - msgpack==1.0.3 - - packaging - - looseversion - - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.3.0 hooks: diff --git a/changelog/64374.fixed.md b/changelog/64374.fixed.md index 479dc6c8c1b3..8b94be869d76 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 c30c340dbf8e..88f533eb4a86 100644 --- a/tools/__init__.py +++ b/tools/__init__.py @@ -52,6 +52,9 @@ ptscripts.register_tools_module( "tools.precommit.docstrings", venv_config=SALT_PKG_DEPS_VENV_CONFIG ) +ptscripts.register_tools_module( + "tools.precommit.loader", venv_config=SALT_PKG_DEPS_VENV_CONFIG +) ptscripts.register_tools_module("tools.release", venv_config=RELEASE_VENV_CONFIG) ptscripts.register_tools_module("tools.vm") diff --git a/tools/precommit/docstrings.py b/tools/precommit/docstrings.py index cd5953f19912..9761630b36b5 100644 --- a/tools/precommit/docstrings.py +++ b/tools/precommit/docstrings.py @@ -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 60% rename from tasks/loader.py rename to tools/precommit/loader.py index d65e5e28591a..2342c4d8f8fe 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 +import tools.utils from salt.loader import SALT_INTERNAL_LOADERS_PATHS -from tasks import utils -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)