PR 1/2: in-process module reload (@reload)#233
Open
spierenburg wants to merge 8 commits into
Open
Conversation
Importable, unit-tested command-table builder extracted from the inline __init__ loader. Fixes the per-module HELP leak and stale-binds for removed modules; preserves runtime binds/chans; per-module failure is non-fatal. invalidate_caches() so a git-pulled new module dir is discoverable. 7 tests.
Adds Matterbot.lifecyclecmds (!reload/@reload). PR 2 will append restart/update tokens to the same list.
Replaces the duplicated two-pass inline command loader in MattermostManager.__init__ with a single load_command_table() call; adds non-mutating bindmap_serializable() and simplifies update_bindmap() to use it. Removes now-dead fnmatch / importlib.util imports (only the deleted walker used them). Behaviour-preserving for well-formed modules; inherits the HELP-leak and stale-binds fixes from command_loader.
Adds async reload_commands(): serialized by an asyncio.Lock, offloads the synchronous load_command_table to the command thread-pool, then atomically rebinds self.commands/self.binds and persists the bindmap. In-flight commands keep their already-submitted process ref. Adds a table-independence test and documents the (pre-existing) bind/unbind chans-mutation race that reload now makes reachable.
Adds MattermostManager.format_reload_report(report) — the human summary posted by @reload (refreshed/added/removed/failed counts + per-failure bullets; newlines in error text flattened so one failure stays one line). conftest.py now stubs configargparse/mattermostdriver only when absent, so `import matterbot` (and thus the suite) collects on any test interpreter regardless of installed deps. Strengthened report tests.
Adds a "Reloading modules without a restart" subsection under matterbot.py: how @reload/!reload works, that a broken module is skipped (not fatal), run-mode agnostic, and the honest importlib.reload limitation (no eviction of held refs / module-global state; restart for deep changes).
Adds a lifecyclecmds branch to the built-in command dispatch chain: @reload/!reload, gated by isadmin, awaits reload_commands() and posts format_reload_report(); failures are caught and surfaced (no escape to the event loop). Non-reload lifecyclecmds tokens are inert here so PR 2 can add restart/update without touching this branch's shape.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@reload/!reload(admin-gated): re-discovers and reloadscommands/*into the running bot with no restart and no dropped websocket, working under tmux/screen, daemonized, or direct. Discover-only — operatorsgit pullon the host themselves; the bot never pulls from chat (no remote-code path via Mattermost).__init__loader into a unit-testedcommand_loader.load_command_table(). This also fixes two latent loader bugs: the per-moduleHELPvariable leak, and stale binds for on-disk-removed modules. Per-module import failure is non-fatal — a broken module is reported and skipped, its last-good version retained, the bot stays up.reload_commands()isasyncio.Lock-serialized, offloads the walk/import to the command thread-pool, and atomically swaps the command table (in-flight commands keep their already-submitted callable).lifecyclecmdsconfig list (["!reload", "@reload"]); README documents the feature and the honestimportlib.reloadlimitation (cannot evict code from held references / module-global state — restart for deep changes).This is PR 1 of 2. PR 2 (systemd supervision +
@restart/@update) is sequenced after this and extends the samelifecyclecmdslist and dispatch branch — it must land second.10 unit tests (
command_loader×8,format_reload_report×2); test harness made interpreter-independent (conftest stubsconfigargparse/mattermostdriveronly when absent so the suite collects regardless of the pytest interpreter).Test Plan
Automated (passing):
pytest -q→ 10 passed;matterbot.py&matterfeed.pycompile;import matterbotclean; loader smoke loads the realcommands/tree (76 modules / 136 binds).Manual — requires a live Mattermost plus one admin (in
Matterbot.botadmins) and one non-admin account, andlifecyclecmdspresent in the deployedconfig.yaml:@reload→ summary posted (refreshed/added/removed/failed), websocket stays connected@reload→ permission-denied message, no reloadprocess()→ admin@reload→ command returns new behaviour, no process restartcommands/<x>/(command.py + defaults.py) →@reload→added=1, new bind works@reload→removed=1, bind no longer triggers@reload→failed=1(named), bot stays up, other commands work, last-good version retained@reloadundertmux/screen→ identical behaviour (run-mode parity)Known follow-up (non-blocking)
command.lstrip('!@')in the dispatch branch strips a character set rather than a prefix; harmless given config-controlled tokens, noted for a later cleanup.