Skip to content

Commit c32e20b

Browse files
committed
refactor(tools): modernise build-basket (formerly check2basket)
Last of the tools/ refactors. Leaves the Director-API domain logic alone (get_data_template, get_command_template, get_service_template, get_notification_template, the regex_replace_words table, parse_plugin_args, generate_basket, manage_uuids) and only touches the surrounding scaffolding: - Add a proper module docstring that actually explains what the tool does (argparse introspection of a plugin script, not just "generate a basket"). - Drop the colorama fallback wrappers `warn_print` / `err_print` and the hand-rolled `save_json_to_file`; use `_common.err` / `_common.die` for messages and `_basket.save_basket` for the JSON writer so the output format stays byte-identical to every other basket-* tool. - Drop the local `skip_plugins = ['example', 'dummy']` list in favour of `_common.SKIP_PLUGINS`, and rename `skip_params` to `SKIP_PARAMS` (frozenset) to match the module-level constant style of the rest of tools/. - Make `--plugin-file` and `--auto` a proper mutually-exclusive argparse group with `required=True`, so `build-basket` with no flags now exits with argparse's standard "one of the arguments ... is required" message instead of printing a hand-rolled error and `sys.exit(2)`. - Factor the duplicated "load old basket / load config / call generate_basket / save output" flow out of `main()` into a new `build_basket_for_plugin()` helper shared by the `--plugin-file` and `--auto` branches. main() drops from 108 lines to ~25. - New helpers `_load_old_basket()` / `_load_config()` treat a missing file as "no previous state" (fine) and an unparseable file as a warning + "no previous state" (regenerate anyway), instead of the old "parse error -> sys.exit(3)" behaviour that aborted the whole --auto sweep over a single broken file. - `--auto` now walks via `_common.iter_check_plugins()` (sorted, deterministic, absolute paths) instead of `os.listdir('check-plugins')` (unsorted, cwd-relative). This also fixes a latent bug where `build-basket --auto` only worked when invoked from the repo root. Collect per-plugin failures and report them at the end instead of bailing on the first one. - `build_basket_for_plugin()` pre-flights the plugin script path with `is_file()` so a typo like `build-basket --plugin-file check-plugins/doesnotexist/doesnotexist` now prints a clear "plugin script not found" message and returns False, instead of the previous unhandled `FileNotFoundError` during `mkdir()`. - `manage_uuids()` duplicate-uuid abort now goes through `_common.die()` with the duplicate list in the message (previously print()+sys.exit(3), an inconsistent exit code for a tool). - `apply_overwrites()` gains a `# nosec B102` annotation on its `exec()` call; the exec'd string is a Python attribute path read from an admin-controlled per-plugin YAML file, not from untrusted input, but bandit cannot know that. Verified: - tools/run-linter-checks: ruff, bandit, vulture all green. - `tools/build-basket --plugin-file check-plugins/about-me/about-me` produces byte-identical output to the pre-refactor version. - Same for cpu-usage and mysql-connections. - `tools/build-basket --plugin-file check-plugins/nope/nope` exits 1 with "plugin script not found". - `tools/build-basket` with no flags exits 2 with the argparse "one of the arguments --plugin-file --auto is required" message. - `tools/build-basket --version` prints the expected banner. Line count: 1046 -> 1024 (-22). Most of the savings come from the merged main() branches and the dropped duplicate helpers; the domain logic carries the bulk of the bytes and stays untouched.
1 parent fc50150 commit c32e20b

1 file changed

Lines changed: 146 additions & 168 deletions

File tree

0 commit comments

Comments
 (0)