From da50395dc719e92e159a01233a8f43561d914a05 Mon Sep 17 00:00:00 2001 From: Oleksandr Kozachuk Date: Tue, 14 Jul 2026 21:30:43 +0200 Subject: [PATCH] =?UTF-8?q?config=20reload:=20fix=20feedback=20loop=20?= =?UTF-8?q?=E2=80=94=20react=20to=20modified/created/moved=20only,=20not?= =?UTF-8?q?=20open/close=20(v3.12.1=20spun=20cpu=20on=20read-opens)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- fjerkroa_bot/discord_bot.py | 14 +++++++++++++- specs/SPEC-008-config.md | 9 ++++++--- tests/test_spec_per.py | 17 +++++++++++------ 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/fjerkroa_bot/discord_bot.py b/fjerkroa_bot/discord_bot.py index c0ccf44..1c92b84 100644 --- a/fjerkroa_bot/discord_bot.py +++ b/fjerkroa_bot/discord_bot.py @@ -88,10 +88,22 @@ class ConfigFileHandler(FileSystemEventHandler): return True return False - def on_any_event(self, event): + def _dispatch(self, event): if not event.is_directory and self._hits_config(event): self._on_change() + # Only write/rename events — NOT on_opened/on_closed, whose read-opens + # (our own load_config re-reads the file) would otherwise feed back into + # a reload loop (CFG-05). + def on_modified(self, event): + self._dispatch(event) + + def on_created(self, event): + self._dispatch(event) + + def on_moved(self, event): + self._dispatch(event) + class FjerkroaBot(commands.Bot): def __init__(self, config_file: str): diff --git a/specs/SPEC-008-config.md b/specs/SPEC-008-config.md index c294be7..f3503ee 100644 --- a/specs/SPEC-008-config.md +++ b/specs/SPEC-008-config.md @@ -35,9 +35,12 @@ applies directly — there are no concurrent readers yet. ### CFG-05 — Hot-reload is rename-safe (coverage: test) The watcher observes the config file's **directory**, not the file, and -reacts to any event (modified, created, moved) whose source or +reacts to a **modified, created, or moved** event whose source or destination path is the config file. This catches atomic saves — write a temp file, then rename it over the target — which replace the inode and fire a move/create rather than a modify; watching the file directly -would go deaf after the first such save. Events for other files in the -directory, and directory events themselves, are ignored. +would go deaf after the first such save. Open/close events are +deliberately not handled: reloading re-opens the file to read it, so +reacting to opens would feed back into an endless reload loop. Events +for other files in the directory, and directory events themselves, are +ignored. diff --git a/tests/test_spec_per.py b/tests/test_spec_per.py index dd9d960..59e3e4a 100644 --- a/tests/test_spec_per.py +++ b/tests/test_spec_per.py @@ -157,10 +157,15 @@ class TestConfigReloadRenameSafe(unittest.TestCase): event.dest_path = dest if dest is not None else "" return event - handler.on_any_event(evt(src=str(config))) # in-place modify - handler.on_any_event(evt(src=str(Path(tmp) / "kroa.toml.tmp"), dest=str(config))) # atomic rename over - self.assertEqual(len(hits), 2) + handler.on_modified(evt(src=str(config))) # in-place modify + handler.on_moved(evt(src=str(Path(tmp) / "kroa.toml.tmp"), dest=str(config))) # atomic rename over + handler.on_created(evt(src=str(config))) # write-new + self.assertEqual(len(hits), 3) - handler.on_any_event(evt(src=str(Path(tmp) / "other.txt"))) # unrelated file - handler.on_any_event(evt(is_dir=True, src=str(config))) # directory event - self.assertEqual(len(hits), 2) # neither fired + handler.on_modified(evt(src=str(Path(tmp) / "other.txt"))) # unrelated file + handler.on_modified(evt(is_dir=True, src=str(config))) # directory event + self.assertEqual(len(hits), 3) # neither fired + + # open/close of the config (our own load_config re-reads) must NOT be handled — else a reload loop. + self.assertNotIn("on_opened", vars(ConfigFileHandler)) + self.assertNotIn("on_closed", vars(ConfigFileHandler))