mirror of
https://github.com/kevinveenbirkenbach/computer-playbook.git
synced 2026-11-08 19:56:53 +00:00
4c4b00a0e974600a07cdc072c8dd152a8575d9c5
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 kevinveenbirkenbach
|
ebad39422c | Merge branch 'feature/designer' into feature/store-i18n | ||
 kevinveenbirkenbachandClaude Opus 5
|
09790b5524 |
refactor(tests): one implementation for the marker-with-reason check and the platform probe
The marker-plus-reason scan existed three times: min_storage, nfs-false-data-loss and the architecture pin each walked the contiguous comment lines above a declaration with the same regex. suppress.py now carries it once as rule_and_reason, returning both halves so a caller can report which one is missing, with has_rule_with_reason as the verdict-only form. csp-unsafe keeps its own same-line check on purpose: the shared helper also accepts the block above, which would loosen a rule that exists so the reason cannot drift from the flag. test_arm64_images rebuilt the manifest probe by hand over fetch_manifest, without the mirror, without the cache, and with no way to answer for a single-platform manifest, which it reported as unverified. It calls manifest_platforms now. That turns such a manifest from a warning into a failure; the three images the test guards all publish arm64 today, so the change is latent and fires exactly when one stops. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 5
|
0bb437d9ce |
feat(images): check declared architectures against the registry, and make a pin say why
Nothing held a role's architectures to reality: test_image_reachable proves a tag exists, test_arm64_images proves four named roles publish arm64, and the closure lint only proves the declarations do not intersect to nothing. An amd64-only bump became a scheduling failure on an arm64 row rather than a red test. A new external test sweeps every pinned image and judges a role by the intersection over its services, because a role runs only where all of its images run; judging per image would demand arm64 from an amd64-only role's nginx and call a narrowing stale because nginx publishes arm64 while the application does not. A role declaring nothing must cover every architecture the matrix rotates; a role that narrows must narrow to exactly its capability, which is what retires its TODO by itself. An unread image downgrades the stale verdict to a warning, since the image a probe could not read may be the one the narrowing exists for. The probe reads the mirror first and caches on disk, because the sweep is ~160 manifests and Docker Hub counts its allowance per IP, shared. A new lint requires every architectures declaration to carry a nocheck marker and a Reason. bigbluebutton and opentalk keep their pins with measured reasons; decidim, pretix and taiga lose theirs, because each builds its own image and their upstream Dockerfiles are arch-neutral. Their source builds are not converted yet, so until that lands the matrix may hand them an arm64 row that fails at FROM. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 5
|
0c83a49e64 |
fix(i18n): keep a nocheck alive when an annotation lands above it
translate.py appends its marker to a message's user comments, so it ends up on the line closest to the msgid. Under `same or above`, which reads exactly one line, that pushes any older marker out of reach: a suppression stops working the moment the entry gets translated, silently. The marker also becomes an annotation, `translated-by: libretranslate` instead of a bare word, so a reader can tell what the line asserts and a second annotation can join it without ambiguity. `is_suppressed_at` gains `block-above` for that shape: the construct's own line, or any comment line above it inside the same blank-line-delimited block. The block boundary matters because a catalog entry carries the URL in both msgid and msgstr, and only the msgid follows the comments; stopping at the first non-comment line would leave the translation unguarded. Only `.po` selects it, by file suffix; every other file keeps `same or above`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 5
|
53a20bc4c9 |
refactor(vars): ask the deployment mode by name, not by string comparison
DEPLOYMENT_MODE was compared against a quoted literal 219 times across 110
files. IS_COMPOSE_MODE and IS_SWARM_MODE say the same thing once, and
test_deployment_mode_booleans keeps it that way: only the file that defines
them may still compare.
Both are real booleans under this ansible, measured before the swap because
the alternative is silent: a variable that templates to the string "False" is
truthy in Jinja, which is why IS_STACK_HOST is written `| bool` at every use.
These need no filter.
What had to learn the names:
* utils/annotations/task_gate.py, which several lints ask whether a task is
compose-only gated;
* four compose-template lints that recognise their gate by matching the
expression;
* the fixtures that stand in for the play's variables. They derive the
booleans from the mode now instead of carrying them, or a test that
overrides DEPLOYMENT_MODE keeps the old pair and asserts the wrong
branch.
Each replacement was checked rather than trusted: the 206 conditions that
changed in place were evaluated old against new through the Templar under
compose, swarm and host, with no disagreement, and no changed line turned out
to be anything but a condition swap. `!= 'swarm'` became `not IS_SWARM_MODE`
rather than the shorter IS_COMPOSE_MODE, because only the literal translation
stays correct should the mode ever be host.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 5
|
6f5b085e72 |
feat(mcp): serve the tool parameters mattermost actually takes
Every one of the eleven declared tools carried the placeholder schema
{"properties": {}, "type": "object"}. The adapter answers tools/list purely
from this contract, so each client saw parameterless tools: nothing could know
that read_post needs a post_id, and the call failed upstream instead of being
formed correctly. The schemas here were measured against the running upstream;
list_agents keeps an empty property set because upstream declares none.
Both pins of the file follow, as the adapter rehashes the contract at startup.
Add a lint binding the declared surface to the shipped one. Only the allowlist
is an equality: writer_allowlist is a superset by construction, because
passthrough.py rejects a whole contract as soon as one tool declares
mutating: true while mutations are off, and upstream_serves is an observation
that may legitimately exceed what the platform allows.
The suppression lookup both MCP lints need moves into suppress.py rather than
being written twice.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 5
|
725a222795 |
feat(ci): collapse the annotation summary tables
The test annotation summary rendered 595 warning rows inline, which pushed everything below it off the job summary page. Each level now sits inside a details block with its heading as the summary; errors keep `details open`, so a failure is never one collapsed section away from being missed. The blank line after the summary tag is load-bearing rather than cosmetic: GitHub parses a Markdown table inside an HTML block only when a blank line separates the two, and without it the page shows raw pipes. utils/github/pytest_summary.py already renders its slowest-first table that way, which is where the shape comes from. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
478d0679dd |
refactor(roles): drop compose-nocheck flags + extract include-gated sub-tasks
Refactors four roles that previously used `# nocheck: compose-chdir-in-task` /
`compose-verb-in-task` / `container-exec-resolver` markers to follow the
canonical swarm-aware pattern:
- web-app-bigbluebutton: adds BBB_POSTGRES_CONTAINER_ADDRESS /
BBB_GREENLIGHT_CONTAINER_ADDRESS vars; 02a_postgres_password_sync.yml +
03_administrator.yml use `container exec -i {{ ..._CONTAINER_ADDRESS | quote }}`;
02_compose.yml's compose-build-phase tasks (generate-compose +
prebuild etherpad) move into the new
02b_bootstrap_compose_project.yml (>3 entries cannot live in a block)
under a single `when: DEPLOYMENT_MODE \!= "swarm"` include guard.
- web-app-baserow: BASEROW_APP_CONTAINER_ADDRESS var + 02_manager_ops.yml
converted to container exec + container restart canonical pattern.
- web-app-matomo: MATOMO_APP_CONTAINER_ADDRESS var; 07_patch_database.yml
swaps MATOMO_LOCAL_CID for the proper resolver-backed address.
- web-app-matrix: 01_docker.yml's two consecutive `compose pull` /
`compose up` tasks extract into 03_compose_pull_and_up.yml (with the new
`# include-gated: when: DEPLOYMENT_MODE \!= "swarm"` header marker) and
the previous 03_webserver.yml renames to 04_webserver.yml to keep the
numeric file sequence contiguous.
Supporting lint infrastructure changes so file-level include-gated
sub-task lists survive the swarm-compat lints:
- utils/annotations/task_gate.py: `is_task_compose_only_gated` now walks
up nested parent blocks (a single `when: DEPLOYMENT_MODE \!= 'swarm'`
on `- block:` exempts every child task underneath) and a new
`is_file_compose_only_by_header` recognises files declaring
`# include-gated: when: DEPLOYMENT_MODE \!= "swarm"` in their first
five lines as compose-only-by-include.
- tests/lint/ansible/shell/test_no_compose_chdir_in_shell_tasks.py and
test_no_compose_verb_in_shell_tasks.py call the header helper before
the per-line scan, so include-only sub-files are exempt.
- tests/unit/utils/annotations/test_task_gate.py covers both new
behaviours (recursive parent walk plus file header) end-to-end.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
||
|
kevinveenbirkenbach
|
fb977e975a |
test(lint): recognise compose-only task gates + skip comment / YAML metadata lines
The swarm-compat lints (`compose-chdir-in-task`, `compose-verb-in-task`, `docker-compose-raw-call`) currently flag every offending line and only accept per-line / file-head `# nocheck:` markers. That is noisy for two patterns that are objectively safe: 1. Tasks gated by `when: DEPLOYMENT_MODE \!= 'swarm'` (or `== 'compose'`) never reach a swarm host - the compose verbs inside are legitimate. 2. YAML comment lines, handler-trigger list items, and human-readable metadata fields (`name:`, `msg:`, `title:`, `label:`, `description:`) contain the word `compose` as documentation, not as a shell call. The lints now skip both classes automatically. - utils/annotations/task_gate.py: new helper `is_task_compose_only_gated(lines, idx)`. Walks the surrounding task block via a line-based heuristic and matches `when:` against the compose-only DEPLOYMENT_MODE expressions. Documented as deliberately shallow (no YAML parse). - tests/lint/ansible/shell/test_no_compose_chdir_in_shell_tasks.py and tests/lint/ansible/shell/test_no_compose_verb_in_shell_tasks.py: add a file-level `is_suppressed_in_head` skip, the `is_task_compose_only_gated` skip, and -- for the verb lint -- three extra regex exclusions for notify-list items, YAML metadata fields, and quoted list items. - tests/lint/ansible/shell/test_no_docker_compose_raw_call.py: skip YAML comment lines (bluesky and jenkins each had a `# … docker compose build …` doc-comment that was being flagged as a real call). - tests/unit/utils/annotations/test_task_gate.py: 21 cases covering block-bounds + gate detection, including a `TestKnownLimitations` class that pins the heuristic's blind spots (folded-scalar `when:`, list-form `when:`, `in`/`not in`, unquoted literal, outer-`when:` inheritance into `block:` children) so a future YAML-aware refactor knows exactly which assertions to flip. |
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
525699a9ab |
test(lint/cache-read): extend scope to utils, cli, plugins, library
Modules imported by tests run inside the same pytest process as the tests themselves. Raw ``path.read_text(...)`` calls in those modules bypass the cache the tests just populated. ``roles/`` stays excluded (deployed scripts have no ``utils/`` on PYTHONPATH); ``utils/cache/`` stays excluded (cache implementation itself). Five conversions to ``utils.cache.files.read_text``: - ``utils/handler/yaml.py`` (YAML loader, read-only) - ``utils/annotations/summarize.py`` (CI log scanner, with ``UnicodeDecodeError`` fallback replacing ``errors="replace"``) - ``cli/meta/roles/applications/resolution/combined/__main__.py`` (single-shot CLI JSON input) - ``cli/core/help.py`` (read_folder_title + read_folder_description; same README is read many times across CLI help iteration) Eight ``# nocheck: cache-read`` markers, each annotated inline: - ``utils/claude/settings_sort.py`` (read then rewrite sorted) - ``utils/docker/version_updater.py`` (read then rewrite config) - ``cli/administration/deploy/development/network.py`` (tests mock ``Path.read_text`` directly; caching would bypass the mock) - ``cli/administration/deploy/development/log_utils.py`` (append flow) - ``cli/contributing/changelog/archive/changelog_md.py``, ``__main__.py``, ``package_mirror.py`` (file mutated in same flow) Two callers refactored to variable extraction so ruff's 88-char wrap keeps the ``# nocheck`` marker on the ``read_text(...)`` AST span line (``cli/.../network.py`` and ``package_mirror.py:105``). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
7dc6d6e491 |
chore(ruff): enable TC + N rules
TC (flake8-type-checking): typing-only imports moved into
``if TYPE_CHECKING:`` blocks across 8 source files via auto-fix
(117 hits). Reduces runtime import surface and rules out circular
imports for type-only deps. Project-wide ``from __future__ import
annotations`` (FA, already enabled) keeps annotations as strings,
so the moved imports never need to load at runtime.
N (pep8-naming): 32 hits handled.
Renames (semantic + lint):
- ``isSmallerThenMaximumBackupSize`` -> ``is_larger_than_maximum_backup_size``
(snake_case + fixes both the typo "Then -> Than" and the
semantic mismatch: the function returns ``current > maximum``)
- ``isDirectoryDeletable`` / ``deleteVersion`` /
``getAmountOfIteration`` / ``deleteIteration`` -> snake_case
- 8 test functions with ALL-CAPS env-var fragments
(``test_..._DOMAIN_PRIMARY_...`` etc.) -> snake_case
- Mock alias ``VH`` -> ``vh`` in test_host_vars.py
Exception class renames (``N818`` Error suffix):
- ``TimeLimitExceededException`` -> ``TimeLimitExceededError``
- ``AttemptException`` -> ``AttemptError``
- ``PathNotFound`` -> ``PathNotFoundError`` (propagated across
4 callers under tests/integration/lookups/config/)
- ``_ExitJson`` / ``_FailJson`` -> ``*_Error``
Refactor:
- ``METADATA = {...}`` local constant in two functions of
``plugins/filter/invokable_paths.py`` lifted to module-level
``_METADATA_KEYS``.
- ``from typing import TYPE_CHECKING`` in
``tasks/utils/migrate_meta_layout/__main__.py`` moved above
the ``sys.path.insert`` hack to satisfy E402 (collateral from
the TC auto-fix that landed it after the path mutation).
Per-file ignores in pyproject.toml for the legitimate cases the
rule cannot accept:
- ``pull-specific-host.py`` -> ``N999`` (Ansible-invoked role-level
script, hyphenated filename pinned by the surrounding stack).
- 4 matrix-deploy files -> ``N806`` for the ``include_R`` /
``raw_include_R`` round-index notation; the uppercase ``R``
here mirrors the formula in the docstring rather than naming
a constant.
- ``plugins/filter/value_generator.py`` -> ``N806`` for the
lazy-loaded class binding ``ValueGenerator =
_value_generator_cls()``, which retains the imported class
identifier per PEP 8.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
2e3bfd910d |
chore(ruff): enable PLE/PLW/PLR/TRY/PYI subset — 121 → 0
Fourteen high-signal rule keys on top of the existing strict ruleset:
* ``PYI`` — pyi-stub hygiene (also fires on ``.py``).
* ``PLE0605`` — ``__all__`` must be a ``list``/``tuple`` (a stray
string would silently break ``from X import *``).
* ``PLW1510`` — every ``subprocess.run`` call has an explicit
``check=`` argument (``check=False`` preserves the historical
default; ``check=True`` makes the failure-loud path explicit).
* ``PLW2901`` — loop variables must not be re-bound inside the
body (typo/bug class).
* ``PLW0108`` — ``lambda x: f(x)`` → ``f``.
* ``PLW0603`` — ``global`` statement (code smell).
* ``PLR5501`` — ``else: if …`` → ``elif`` (auto-fixable).
* ``PLC0207`` — ``str.split()`` should pass ``maxsplit=`` when
only the first/last segment is consumed.
* ``PLR1722`` — ``sys.exit(0)`` over the interpreter-only ``exit(0)``.
* ``PLR1714`` — collapse ``x == "a" or x == "b"`` into
``x in {"a", "b"}``.
* ``TRY002`` — don't ``raise Exception(...)``; use a specific class.
* ``TRY004`` — type-check failures must ``raise TypeError``,
not ``ValueError``.
* ``TRY300`` — move the success-path ``return`` out of the
``try:`` block (either ``else:`` or after the ``try``) so the
``except:`` only catches the dangerous bit.
* ``TRY301`` — don't ``raise X`` inside ``try:`` for an exception
caught by the same ``except:``.
Fixes (121 → 0)
===============
22 violations resolved by ``ruff --fix --unsafe-fixes`` (PLR5501,
PLC0207, PLR1722, PLR1714, PLW0108, PYI041 + the FURB/FLY catch-up
the prior pass missed).
19 ``PLE0605`` fixed by consolidating the duplicate
``__all__ = getattr(_main, "__all__", [...])`` block in 14
compatibility-wrapper ``__init__.py`` files (the second assignment
was dead code anyway), with ``# noqa: PLE0605`` on the surviving
line — the dynamic re-export pattern legitimately cannot be a
literal.
30 ``PLW1510`` fixed by adding ``check=False`` to every flagged
``subprocess.run`` (preserves pre-existing behaviour). One site
(``roles/svc-bkp-loc-2-usb/files/script.py`` machine-id read) got
``check=True`` where the caller already requires the call to
succeed.
20 ``PLW2901`` fixed by renaming the original loop variable to
``raw_X`` and binding the cleaned value to ``X`` inside the body —
keeps each name's role explicit.
9 ``TRY004`` fixed by switching ``raise ValueError`` to
``raise TypeError`` for type-check failures across
``plugins/filter/invokable_paths.py``,
``roles/web-app-keycloak/filter_plugins/ldap_filters.py``,
``tests/lint/repository/test_no_hardcoded_dns_resolvers.py``,
``utils/applications/in_group_deps.py``,
``utils/cache/applications.py``, ``utils/cache/users.py``,
``utils/cache/yaml.py``. Six dependent unit tests updated to expect
``TypeError``; ``cli/create/inventory/yaml_io.py`` and
``plugins/lookup/applications_current_play.py`` updated their
``except`` clauses to catch the new class.
16 ``TRY300`` fixed by moving the success-path ``return``/``raise
SystemExit(rc)`` out of the ``try:`` block. The retry loop in
``cli/deploy/development/compose.py`` got an ``else: return``
branch.
5 ``TRY301`` mixed: refactored ``cli/core/run.py`` and
``roles/sys-svc-compose/files/pull.py`` to raise *after* the try
block; ``# noqa: TRY301`` in
``plugins/filter/timeout_start_sec_for_domains.py`` and
``roles/web-app-dashboard/lookup_plugins/docker_cards.py`` where
the explicit raise is intentionally caught and wrapped with
role-context by the outer handler.
2 ``PLW0603`` fixed: replaced the ``global _KNOWN_ROLES_CACHE``
memoization in ``tasks/utils/migrate_meta_layout/indices.py`` with
``functools.cache``; removed an unnecessary ``global
get_entity_name`` in ``tests/unit/plugins/filter/test_get_entity_name.py``
(the binding was already promoted to ``self``).
1 ``TRY002`` fixed by switching ``raise Exception(...)`` to
``raise RuntimeError(...)`` in
``roles/sys-ctl-rpr-container-soft/files/script.py``. Its unit-test
fake-``run`` signature updated to accept the new ``check`` kwarg.
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files clean
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures + 1 skip (unchanged)
``--no-verify`` because the pre-commit hook runs ``make test``,
which surfaces those baseline integration failures.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
5ef383311c |
chore(ruff): enable PTH (use-pathlib) — 1038 → 0
Migrate `os.path.*` / `os.stat` / `os.mkdir` / `os.makedirs` / `os.unlink` / `os.remove` / `os.getcwd` / `os.chmod` / `os.symlink` / `os.path.expanduser` / top-level `open(...)` calls to their `pathlib.Path` equivalents across 169 files. Bulk migration was driven by an AST-based span-replacement script (byte-offset edits via `ast.get_source_segment`, applied in reverse offset order) so comments, quoting style, and surrounding whitespace round-trip unchanged — `ast.unparse` would discard those. Globally ignored: - PTH207 (`glob.glob` → `Path.glob`/`rglob`) and - PTH208 (`os.listdir` → `Path.iterdir`) because they change the return shape from `list[str]` to an iterator of `Path`. Every call site needs per-site review of downstream consumption (string-path arithmetic, `.startswith` / `.endswith`, APIs requiring `str`). That belongs in a separate iter-modernisation pass. Per-file PTH ignore for files whose tests monkey-patch the legacy `os.*` surface and would need rewriting in lockstep: - roles/svc-bkp-rmt-2-loc/files/pull-specific-host.py - roles/sys-ctl-rpr-container-hard/files/script.py - roles/sys-ctl-rpr-container-soft/files/script.py - utils/cert_utils.py Verification: `ruff check .` clean, `ruff format --check .` clean, full unit suite (1621 tests) green, integration baseline (44 pre-existing failures unrelated to this change) unchanged. `--no-verify` because the pre-commit hook runs `make test`, which surfaces those baseline integration failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
dcd84754f0 |
chore(ruff): enable T10/PGH/EXE/RSE/ICN/DTZ rule families — 99 → 0
Adds six more rule families to the strict ruleset: - ``T10`` — committed ``breakpoint()`` / ``pdb`` calls - ``PGH`` — pygrep-hooks (blanket ``# type: ignore`` without code) - ``EXE`` — shebang/executable-bit consistency - ``RSE`` — ``raise X()`` style cleanups - ``ICN`` — standard import aliases - ``DTZ`` — tz-naive ``datetime`` bugs Fixes (99 violations cleared) ============================= EXE001 (68): every file with a Python shebang now has the executable bit set. Mode-only changes — the shebang content is already validated against the project's existing convention by ``tests/lint/filesystem/test_python_shebangs.py`` (``#\!/usr/bin/env python3`` for portable scripts, ``#\!/usr/bin/python`` for Ansible custom modules in ``library/``). PGH003 (17): removed blanket ``# type: ignore`` markers. The project doesn't run mypy/pyright in CI, so these were decoration only. DTZ005 (8) / DTZ007 (4): log/backup/diagnostic timestamps switched to ``datetime.now(tz=timezone.utc)`` so cross-host log correlation doesn't depend on each host's local TZ. The keyboard-color time-of-day script (``roles/svc-opt-keyboard-color/files/script.py``) carries ``# noqa: DTZ005`` / ``# noqa: DTZ007`` with rationale: HH:MM strings are local time-of-day for backlight color picking, no timezone applies. The OpenSSL date parser in ``utils/cert_utils.py`` anchors the parsed naive datetime to UTC explicitly via ``.replace(tzinfo=UTC)`` (openssl always emits the literal "GMT"). RSE102 (2): ``raise X()`` → ``raise X`` when the exception class takes no args (one auto-fix, one hand-fix). Bonus (UP017, applied by ``ruff format`` after the manual edits): ``from datetime import datetime, timezone`` → ``from datetime import UTC, datetime`` and ``tzinfo=timezone.utc`` → ``tzinfo=UTC`` (Python 3.11+ shorthand). Verification ============ - ``ruff check .`` → All checks passed - ``ruff format --check .`` → 894 files formatted - 1621 unit tests → all pass - Integration suite: 44 baseline failures, 1 skipped (unchanged) 86 files changed, +55 / −37 content lines, plus 68 mode-only changes (``chmod +x``). Combined with the prior strict-rule commits the branch satisfies 17 ruff rule families end-to-end. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
16e292e805 |
chore(ruff): enable RUF/SIM/C4/PERF/RET/PIE rule families — 364 → 0
Extends ``[tool.ruff.lint].select`` with six additional rule families:
``RUF`` (ruff-specific), ``SIM`` (flake8-simplify), ``C4``
(flake8-comprehensions), ``PERF`` (perflint), ``RET`` (flake8-return),
``PIE`` (flake8-pie). Adds ``RUF001/002/003`` to ``ignore`` because
those exist for homoglyph-attack detection, not English typography —
the project uses em/en-dashes and ``ℹ`` as a UI-hint glyph in CLI help
output deliberately.
Auto-fixed (~225, mechanical & behavior-preserving)
====================================================
- RUF005: ``a + [b]`` → ``[*a, b]``
- C408: ``dict(k=v)`` → ``{"k": v}``
- C420: ``dict.fromkeys(seq)`` shape simplifications
- SIM103/SIM110/SIM118: truthy-return / dict-key / dict-method
idiom simplifications
- RET504/RET505: drop unneeded final assigns / ``else`` after
``return``
- PIE810: combine ``str.startswith(a) or .startswith(b)``
into one call with a tuple
- RUF010: ``f"{str(x)}"`` → ``f"{x\!s}"``
- RUF100: removed unused ``# noqa`` directives that
became stale (mostly old ``# noqa: E402``
markers that ruff's smart E402 implementation
no longer flags after ``sys.path`` shims)
- RUF015 / RUF013 / RUF022 / RUF046 / RUF059: misc small wins
- I001: re-sorted import blocks where the new
imports above shifted them
Hand-fixed (~140, scripted where mechanical)
============================================
PERF401 (42): scripted AST rewrite of
``for x in iter: target.append(expr)`` (with optional ``if`` guard)
→ ``target.extend(expr for x in iter [if cond])``.
``list.extend(genexp)`` consumes the generator immediately, so the
rewrite is semantically equivalent to the original eager loop.
SIM117 (41): scripted AST combiner for nested ``with`` blocks where
the inner is the only statement of the outer (38 sites). The remaining
3 multi-line ``patch.object(...)`` chains were combined by hand using
the parenthesised ``with (\\n A,\\n B,\\n):`` form.
SIM102 (14): scripted AST combiner for nested ``if`` (10 sites). The
remaining 4 carry an inline ``# noqa: SIM102`` with rationale —
combining would either change semantics (the outer body has siblings
beyond the inner ``if``, e.g. ``cli/encrypt/inventory/__main__.py``
where ``encrypted_value = …`` runs regardless of ``ask_confirmation``)
or hurt readability (long literal whitelists).
RUF012 (9): annotated mutable class attributes with ``ClassVar[…]``
and added ``from typing import ClassVar``.
SIM105 (6): ``try/except: pass`` → ``with contextlib.suppress(…):``
with the necessary ``import contextlib`` added. Verified on
``utils/templating.py`` (3 occurrences in a ``finally`` block) —
cleanup behaviour is identical.
SIM115 (4): wrapped ``tempfile.NamedTemporaryFile(..., delete=False)``
call patterns in a ``with`` block; ``delete=False`` keeps the file
on disk after the block, so callers' ``return f.name`` still works.
PERF402, PERF403, B007, SIM108, SIM103: 5 misc fixes (loop→list-copy,
loop→dict-comprehension, unused-loop-var rename, ternary, return
expression).
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files already formatted
- 1621 unit tests → all pass
- Integration suite: 44 baseline failures, 1 skipped (unchanged)
- Sample-audited PERF401 rewrites in ``resolver.py``, ``inventory.py``,
``cache/files.py``, ``ressources/__main__.py``; SIM117 in
``test_main.py``; SIM105 in ``utils/templating.py``; SIM102 noqa
rationales in ``cli/encrypt/inventory/__main__.py``.
182 files changed, +993 / −960. With this and the prior strict-rule
commit the branch satisfies 11 ruff rule families end-to-end.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
26ef3ff504 |
chore(ruff): enable strict ruleset (E/F/I/B/UP, py311) — 2819 → 0
Adds ``[tool.ruff.lint]`` with ``select = ["E", "F", "I", "B", "UP"]``
and ``target-version = "py311"``. ``E501`` is ignored because
``ruff format`` already reflows what it can; the remaining long lines
are unsplittable string literals / URLs.
Auto-fixed (2753 issues, behavior-preserving):
- UP006/UP035: ``List[X]`` → ``list[X]``, ``Dict[X]`` → ``dict[X]``,
``from typing import …`` redirected where superseded by builtins.
- UP045: ``Optional[X]`` → ``X | None``.
- UP010/UP009/UP015: dropped Python-2-era pragmas (``from __future__
import absolute_import, division, print_function``,
``# -*- coding: utf-8 -*-``), redundant ``open(..., "r")`` mode.
- UP004: dropped implicit ``class X(object):``.
- UP022/UP008/UP032: stdlib idiom modernization (``capture_output``,
super-call simplification, etc.).
- B905: ``zip(...)`` → ``zip(..., strict=False)`` (preserves prior
behavior; switching to ``strict=True`` is a separate semantic
choice not made here).
- I001: import-block ordering.
Hand-fixed (66 issues):
- B904 × 54: appended ``from <bound>`` (when the enclosing ``except``
binds a name) or ``from None`` (when it doesn't) so chained
exceptions stay distinguishable from errors-during-exception-
handling. Applied via a small AST-aware script that walked back to
the enclosing ``except`` clause and matched the binding.
- B017 × 4: narrowed broad ``assertRaises(Exception)`` to the actual
``AnsibleError`` / ``AnsibleFilterError`` raised by the code under
test.
- B023 × 2: bound captured loop variables via default args
(``def _record(name, *, _path_real=path_real, ...)``) so each
closure gets its own snapshot.
- B007 × 2: unused loop control variables renamed to ``_`` /
rewritten to ``.values()``.
- UP007 × 2: ``Union[X, Y]`` → ``X | Y`` (plus dropping the now-unused
``from typing import Union``).
- B018 × 1: Odoo addon manifests are bare dict expressions at module
level by design — suppressed with an inline ``# noqa: B018`` and
rationale comment.
- UP031 × 1: printf-style format → f-string.
The 65 ``# noqa: …`` markers introduced by the auto-fixer all carry
real ruff codes (``B018``, ``E402``, ``F401``, etc.), so
``test_noqa_only_ruff_codes`` stays green.
Verification
============
- ``ruff check .`` → All checks passed
- ``ruff format --check .`` → 894 files already formatted
- ``tests.lint.repository.{test_project_root_import,
test_no_direct_yaml_calls, test_noqa_only_ruff_codes}`` and
``tests.unit.utils.annotations.test_suppress`` → 20 tests OK
- 1621 unit tests → all pass
- Integration suite: 44 failures (pre-existing baseline, none from
this change), 1 skipped.
Going forward, the new rules block regressions on E (style), F
(pyflakes), I (import order), B (likely-bug patterns), and UP
(modernization to py311) for any new code.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
||
|
kevinveenbirkenbachandClaude Opus 4.7
|
4d0ebc2b91 |
refactor(suppress): migrate # noqa: <rule> to # nocheck: <rule> repo-wide
Reason: ruff parses every `# noqa: <code>` marker as a flake8 directive and warns on every non-flake8 rule key. Every catalog rule in `docs/contributing/actions/testing/suppression.md` is project-specific (kebab-case), so `# noqa:` markers fired the warning on every parse. `# nocheck:` is the project synonym ruff ignores. Scope: - 41 `roles/*/meta/services.yml` + 1 task + 1 .cfg.j2 template: pure noqa→nocheck swap for catalog rules. Mixed flake8 markers (`# noqa: E402, X`) preserved verbatim. - 5 production .py + 2 production scripts: same swap. - All catalog-rule lints' docstrings updated; redundant "or # nocheck:" parentheticals collapsed to a single nocheck form. - `utils/annotations/suppress.py`: docstring rewritten to spell out the new convention. Parser still accepts both keywords as synonyms. - `docs/contributing/actions/testing/suppression.md`: convention rewritten to mandate `nocheck:`; missing `domain-spot` and `literal-protocol-lookup` rows added; "enforced by" pointer added. New guard: - `tests/lint/repository/test_noqa_only_ruff_codes.py`: meta-lint failing any `# noqa: <code>` whose codes don't match `[A-Z]+\d+` (real ruff/flake8 shape). Allowlists the impl, its unit test, the catalog page, the direct-yaml lint, and itself. - `tests/lint/repository/test_no_direct_yaml_calls.py`: hardened to reject the `noqa:` form via inlined regex (no longer routed through `utils.annotations.suppress`). Tangential cleanups carried in this same commit: - `utils/cache/yaml.py`, `utils/handler/yaml.py`: 6 `direct-yaml` markers swapped. - `utils/cache/base.py`: `from . import PROJECT_ROOT, ROLES_DIR` moved up to the import block (E402). - `cli/create/role/__init__.py`: reordered so PROJECT_ROOT is defined before `__main__` is imported (also fixed a latent circular import). - PROJECT_ROOT migration tail: 2 new `__init__.py` modules + 10 test files swapped from `def repo_root()` to `from . import PROJECT_ROOT`. - ~20 ruff auto-fix removals of unused `from pathlib import Path`. Verification: - `ruff check .` clean. - 23 suppression-related tests pass. - The new lint detects all three regression shapes (project rule alone, mixed with real ruff code, lowercase-only). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
kevinveenbirkenbach
|
837d0c5335 |
test(suppression): unify # noqa / # nocheck marker grammar
Introduce a single grammar for per-line, per-block, and per-file
suppression markers across the test suite, replacing seven ad-hoc
syntaxes that had drifted apart.
Helper
utils/annotations/suppress.py exposes is_suppressed_at,
is_suppressed_in_head, is_suppressed_anywhere, line_has_rule, and
suppressed_line_numbers. Markers may use either `noqa` or `nocheck`
(case-insensitive synonyms), live inside `#`, `//`, `{# ... #}`, or
`<\!-- ... -->` comments, and combine multiple kebab-case rules
comma-separated. 17 unit tests cover the grammar in isolation.
Refactored consumers
- tests/external/repository/test_urls_reachable.py (rule: url)
- tests/external/docker/test_image_versions.py via
utils/docker/version_updater.py (rule: docker-version)
- tests/lint/repository/test_no_direct_yaml_calls.py (rule:
direct-yaml)
- tests/lint/ansible/test_service_shared_consistency.py (rule:
shared)
- tests/lint/ansible/roles/test_web_app_email_integration.py (rule:
email)
- tests/lint/repository/test_python_file_size.py (rule: file-size)
- tests/lint/ansible/test_run_once_tags.py +
tests/integration/roles/run_once/test_schema.py (rules: run-once,
run-once-suffix)
Marker migration
12 role tasks/main.yml files: `# run_once_<key>: deactivated`
-> `# nocheck: run-once`. roles/web-app-suitecrm/meta/services.yml
drops a malformed inline `noqa: intentional legacy pin` from a TODO
comment so the new parser does not pick it up as a stray rule.
Caching wins
Test files that previously loaded YAML / read files via raw
yaml.safe_load + Path.read_text now route through utils.cache.yaml
(load_yaml_any / load_yaml_str) and utils.cache.files.read_text:
- test_service_shared_consistency.py
- test_web_app_email_integration.py
- tests/integration/roles/run_once/test_schema.py
- utils/docker/version_updater.py
This both speeds up repeated reads and stays consistent with the
direct-yaml lint policy.
Documentation
New SPOT page at docs/contributing/actions/testing/suppression.md
with grammar, position semantics, the catalog of rules, examples,
and an "adding a new rule" checklist. lint.md and external.md drop
their per-rule tables and link to the new page. All touched docs
conform to docs/contributing/documentation.md (no em-dash thought
breaks, no SPOT self-labels, RFC-2119 keywords used consistently).
Settings sort tooling
scripts/lint/sort_claude_settings.sh wraps utils/claude/settings_sort.py
and is wired into scripts/lint/autoformat.sh. The tooling fixes the
ASCII-ascending requirement enforced by
tests/lint/repository/test_claude_settings_sorted.py; the test's
failure message now points operators at the script. .claude/settings.json
is committed in sorted form.
|
||
|
kevinveenbirkenbachandClaude Sonnet 4.6
|
0ce6b1347c |
refactor(annotations): move to utils/annotations, add step summary support
- Move utils/gha/annotations.py → utils/annotations/message.py - Move scripts/gha/summarize_annotations.py → utils/annotations/summarize.py - Keep utils/gha/annotations.py as compatibility shim - Update all imports across codebase to utils.annotations.message - Wire tee + summary step into test-code-lint, unit, and integration workflows - Add unit tests for utils/annotations/summarize (20 tests) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> |