Skip to content

Commit fc50150

Browse files
committed
refactor(tools): modernise basket-* helpers around a shared _basket module
Bring basket-compare, basket-join and basket-remove-uuids onto the same shape as the rest of the tools/ tree, and introduce `tools/_basket.py` for the load / save / duplicate-key plumbing that each of the three previously rolled on its own. _common.py: - Add `dummy` to SKIP_PLUGINS. The `check-plugins/dummy` plugin is the minimal argparse-plus-output skeleton and should never have been picked up by tools that walk check-plugins/. - Generalise `iter_check_plugins()` into `iter_plugin_dirs(subdir)` so tools that also walk notification-plugins/ can reuse the same sorted, SKIP_PLUGINS-aware iterator. _basket.py (new): - DuplicateKeyError + _raise_on_duplicate_keys (previously duplicated as private helpers inside each basket-* tool). - load_basket(path) / save_basket(path, data). Both abort via `_common.die()` on unrecoverable errors (missing file, invalid JSON, duplicate keys) so callers never grow the same `if not success: sys.exit()` dance again. basket-remove-uuids: - Drop the unused colorama warn_print / err_print wrappers, the dead `skip_plugins` list, the BasketUUIDRemover class wrapper (there were only two meaningful methods) and the leftover plugin-style `sys.exit(3)` path. - Use `_basket.load_basket` / `_basket.save_basket`, keep only the pure `remove_uuids()` walker. basket-join: - Replace the `os.listdir('check-plugins')` / `os.listdir( 'notification-plugins')` pair with the sorted `_common.iter_plugin_dirs()` iterator, which additionally respects the SKIP_PLUGINS set. This is a deterministic ordering change: the old version's per-plugin order depended on the file system (ext4/xfs hash order), so datafield numeric IDs in the merged output used to shift between machines; the new version always produces the same IDs given the same inputs. The set of merged objects is unchanged (same 1938 Datafields, 293 Commands, 96 DataLists, etc. with identical uuids). - Replace the colorama err_print / warn_print wrappers with `_common.err` / `_common.die` plain-stderr output. - Drop the skip_plugins hardcoded list now that SKIP_PLUGINS lives in _common. basket-compare: - Replace the plugin-style `try: parse_args() except SystemExit: sys.exit(STATE_UNKNOWN)` / `lib.base.cu()` / `lib.base.oao('')` scaffolding with a plain main() that uses `_common.die()` for real errors. - Replace the `exit()` builtin call in `get_service_sets_diff` with a proper `_common.die()` so the "unhandled deepdiff category" branch actually says what went wrong. - Clean up `get_val()` so the `d is None` case is handled on the first line instead of being overwritten after the type check. - Fold the five identical "normalise-compare-get_diff-print_table" blocks in `main()` into a SECTIONS tuple + loop. 175 lines of near-duplicated code drop to ~20, and adding a new Director object class is now one SECTIONS entry. build-basket: - Drive-by fix caught by the linter sweep: rename the unused `*args` / `**kwargs` parameters on `_return_parser_as_tuple` to `*_args` / `*_kwargs` so vulture stops flagging them at 100% confidence. Phase-6 cleanup will touch this tool properly; this commit only silences the warning. Verified: - `tools/run-linter-checks`: ruff, bandit, vulture all green. - `tools/basket-join --output-file /tmp/basket-new.json`: exits clean, 293 Commands, 1938 Datafields, 96 DataLists - identical set and uuids to the pre-refactor run. - `tools/basket-compare /tmp/basket-new.json /tmp/basket-new.json`: exits 0 with no output (self-diff is empty).
1 parent 2360883 commit fc50150

File tree

6 files changed

+425
-487
lines changed

6 files changed

+425
-487
lines changed

tools/_basket.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
#! /usr/bin/env python3
2+
# -*- coding: utf-8; py-indent-offset: 4 -*-
3+
#
4+
# Author: Linuxfabrik GmbH, Zurich, Switzerland
5+
# Contact: info (at) linuxfabrik (dot) ch
6+
# https://www.linuxfabrik.ch/
7+
# License: The Unlicense, see LICENSE file.
8+
9+
# https://github.com/Linuxfabrik/monitoring-plugins/blob/main/CONTRIBUTING.md
10+
11+
"""Shared helpers for the basket-* tools.
12+
13+
Icinga Director basket JSON files are the repo's exchange format for
14+
Director objects (datafields, commands, host templates, service
15+
templates, service sets). Every tool under `tools/` that reads or
16+
writes a basket goes through `load_basket()` / `save_basket()` here
17+
so the error handling (duplicate keys, missing file, invalid JSON)
18+
looks identical across tools and the on-disk format (4-space indent,
19+
trailing newline) stays byte-stable across regenerations.
20+
"""
21+
22+
import json
23+
24+
import _common
25+
26+
__author__ = 'Linuxfabrik GmbH, Zurich/Switzerland'
27+
__version__ = '2026041301'
28+
29+
30+
class DuplicateKeyError(ValueError):
31+
"""Raised by `_raise_on_duplicate_keys` when a basket carries two
32+
entries under the same key. Director baskets must not contain
33+
duplicates - the import fails silently and drops the earlier
34+
value if we let json.load() handle it on its own, so we fail
35+
fast instead."""
36+
37+
38+
def _raise_on_duplicate_keys(ordered_pairs):
39+
"""object_pairs_hook for json.load that refuses duplicate keys.
40+
41+
Returns a plain dict on success; raises DuplicateKeyError the
42+
moment any key appears twice in the ordered pair list. Use as
43+
`json.load(f, object_pairs_hook=_raise_on_duplicate_keys)`.
44+
"""
45+
result = {}
46+
for key, value in ordered_pairs:
47+
if key in result:
48+
raise DuplicateKeyError(f'Duplicate key: {key}')
49+
result[key] = value
50+
return result
51+
52+
53+
def load_basket(path):
54+
"""Load a Director basket JSON file and return the parsed dict.
55+
56+
Aborts via `_common.die()` on file-read error, on invalid JSON
57+
and on duplicate keys. All three are conditions the caller
58+
cannot meaningfully recover from, so every basket-* tool would
59+
otherwise grow the same `if not success: sys.exit()` dance
60+
around the call.
61+
"""
62+
try:
63+
text = path.read_text(encoding='utf-8')
64+
except OSError as exc:
65+
_common.die(f'basket: cannot read {path}: {exc}')
66+
try:
67+
return json.loads(text, object_pairs_hook=_raise_on_duplicate_keys)
68+
except json.JSONDecodeError as exc:
69+
_common.die(f'basket: cannot parse {path}: {exc}')
70+
except DuplicateKeyError as exc:
71+
_common.die(f'basket: {path}: {exc}')
72+
73+
74+
def save_basket(path, data):
75+
"""Write a Director basket JSON file with the project's style.
76+
77+
Writes 4-space indent, `(',', ': ')` separators and a trailing
78+
newline - matching what existing committed baskets look like,
79+
so `git diff` after a regeneration only shows real content
80+
changes and not whitespace churn.
81+
"""
82+
try:
83+
with path.open('w', encoding='utf-8') as f:
84+
json.dump(
85+
data,
86+
f,
87+
sort_keys=False,
88+
indent=4,
89+
separators=(',', ': '),
90+
)
91+
f.write('\n')
92+
except OSError as exc:
93+
_common.die(f'basket: cannot write {path}: {exc}')

tools/_common.py

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,14 @@
3434
CHECK_PLUGINS_DIR = REPO_ROOT / 'check-plugins'
3535
TOOLS_DIR = REPO_ROOT / 'tools'
3636

37-
# The `example` plugin is a code skeleton that new plugins are based
38-
# on; it is not meant to be discovered, tested or shipped. Tools that
39-
# walk the check-plugins tree should skip it by default.
40-
SKIP_PLUGINS = frozenset({'example'})
37+
# Plugins that exist only as code skeletons and must not be included
38+
# when tools iterate over `check-plugins/` (or its sibling directories
39+
# for notification/event plugins): `example` is the reference plugin
40+
# we copy-paste new plugins from, `dummy` is a minimal variant that
41+
# exercises argparse and the output library without talking to any
42+
# real data source. Tools that need to sweep the entire tree anyway
43+
# can pass `skip=frozenset()` to `iter_plugin_dirs()`.
44+
SKIP_PLUGINS = frozenset({'dummy', 'example'})
4145

4246

4347
def err(msg):
@@ -61,21 +65,36 @@ def die(msg, code=1):
6165
sys.exit(code)
6266

6367

64-
def iter_check_plugins(skip=SKIP_PLUGINS):
65-
"""Yield every `check-plugins/<name>` directory in sorted order.
68+
def iter_plugin_dirs(subdir, skip=SKIP_PLUGINS):
69+
"""Yield every `<subdir>/<name>` directory in sorted order.
6670
67-
Skips any plugin whose name is in `skip` (default: the `example`
68-
skeleton). Tools that need a different skip set - for example, to
69-
rerun a single plugin that is normally skipped - can pass
70-
`skip=frozenset()` to walk the entire tree.
71+
`subdir` is relative to the repo root and names one of the
72+
plugin families the repo ships - typically `check-plugins`,
73+
`notification-plugins` or `event-plugins`. Missing subdirs
74+
yield nothing (tools can iterate over all three without
75+
having to check for existence first).
7176
72-
Yields `pathlib.Path` objects, so callers can chain the usual
73-
`.name`, `.is_dir()`, `/ "sub"` operations without dragging `os`
74-
and `os.path` imports into the caller.
77+
Yields `pathlib.Path` objects so callers can chain the usual
78+
`.name`, `.is_dir()`, `/ "sub"` operations without dragging
79+
`os` and `os.path` imports into the caller.
7580
"""
76-
for entry in sorted(CHECK_PLUGINS_DIR.iterdir()):
81+
target = REPO_ROOT / subdir
82+
if not target.is_dir():
83+
return
84+
for entry in sorted(target.iterdir()):
7785
if not entry.is_dir():
7886
continue
7987
if entry.name in skip:
8088
continue
8189
yield entry
90+
91+
92+
def iter_check_plugins(skip=SKIP_PLUGINS):
93+
"""Yield every `check-plugins/<name>` directory in sorted order.
94+
95+
Thin wrapper around `iter_plugin_dirs('check-plugins', ...)`
96+
that documents the common-case entry point. Skips the
97+
`example` / `dummy` skeletons by default; pass
98+
`skip=frozenset()` to walk the entire check-plugins tree.
99+
"""
100+
yield from iter_plugin_dirs('check-plugins', skip=skip)

0 commit comments

Comments
 (0)