config reload: rename-safe (watch dir not file, react to moved/created not just modified) — cfg-05
This commit is contained in:
@@ -68,11 +68,29 @@ def split_answer(text: str, threshold: int, max_parts: int) -> list:
|
||||
|
||||
|
||||
class ConfigFileHandler(FileSystemEventHandler):
|
||||
def __init__(self, on_modified):
|
||||
self._on_modified = on_modified
|
||||
"""Rename-safe config watch (CFG-05).
|
||||
|
||||
def on_modified(self, event):
|
||||
self._on_modified(event)
|
||||
Editors and tools save atomically — write a temp file, then rename it
|
||||
over the target — which fires a *moved*/*created* event (not
|
||||
*modified*) and swaps the inode, so watching the file directly goes
|
||||
deaf after the first save. We watch the config's *directory* and react
|
||||
to any event whose src or dest path is the config file.
|
||||
"""
|
||||
|
||||
def __init__(self, config_path: str, on_change):
|
||||
self._config_path = str(Path(config_path).resolve())
|
||||
self._on_change = on_change
|
||||
|
||||
def _hits_config(self, event) -> bool:
|
||||
for attr in ("src_path", "dest_path"):
|
||||
path = getattr(event, attr, "")
|
||||
if path and str(Path(path).resolve()) == self._config_path:
|
||||
return True
|
||||
return False
|
||||
|
||||
def on_any_event(self, event):
|
||||
if not event.is_directory and self._hits_config(event):
|
||||
self._on_change()
|
||||
|
||||
|
||||
class FjerkroaBot(commands.Bot):
|
||||
@@ -102,8 +120,10 @@ class FjerkroaBot(commands.Bot):
|
||||
|
||||
def init_observer(self):
|
||||
self.observer = Observer()
|
||||
self.file_handler = ConfigFileHandler(self.on_config_file_modified)
|
||||
self.observer.schedule(self.file_handler, path=self.config_file, recursive=False)
|
||||
config_path = Path(self.config_file).resolve()
|
||||
self.file_handler = ConfigFileHandler(str(config_path), self.on_config_file_changed)
|
||||
# Watch the directory, not the file — atomic saves replace the inode (CFG-05)
|
||||
self.observer.schedule(self.file_handler, path=str(config_path.parent), recursive=False)
|
||||
self.observer.start()
|
||||
|
||||
def init_aichannels(self):
|
||||
@@ -415,12 +435,10 @@ class FjerkroaBot(commands.Bot):
|
||||
airesponder.image_cache.purge_message(str(message.id)) # IMG-14
|
||||
await airesponder.observe_event(message.author.name, "delete", f"deleted: {message.content}")
|
||||
|
||||
def on_config_file_modified(self, event):
|
||||
def on_config_file_changed(self):
|
||||
# Runs on the watchdog observer thread — the swap itself is
|
||||
# scheduled onto the event loop so no request reads a
|
||||
# half-swapped config (CFG-04 / D9)
|
||||
if event.src_path != self.config_file:
|
||||
return
|
||||
new_config = self.load_config(self.config_file)
|
||||
if repr(new_config) == repr(self.config):
|
||||
return
|
||||
|
||||
@@ -31,3 +31,13 @@ and all responder `.config` references is scheduled onto the event
|
||||
loop (`call_soon_threadsafe`), so no request ever reads a
|
||||
half-swapped config (D9). Before the loop runs (startup), the swap
|
||||
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
|
||||
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.
|
||||
|
||||
+29
-6
@@ -120,9 +120,7 @@ class TestConfigReloadRace(TestBotBase):
|
||||
new_config["history-limit"] = 99
|
||||
self.bot.load_config = lambda path: new_config
|
||||
self.bot.loop = MagicMock()
|
||||
event = MagicMock()
|
||||
event.src_path = self.bot.config_file
|
||||
self.bot.on_config_file_modified(event)
|
||||
self.bot.on_config_file_changed()
|
||||
self.bot.loop.call_soon_threadsafe.assert_called_once()
|
||||
apply_fn = self.bot.loop.call_soon_threadsafe.call_args.args[0]
|
||||
apply_fn()
|
||||
@@ -137,7 +135,32 @@ class TestConfigReloadRace(TestBotBase):
|
||||
loop = MagicMock()
|
||||
loop.call_soon_threadsafe.side_effect = RuntimeError("no running loop")
|
||||
self.bot.loop = loop
|
||||
event = MagicMock()
|
||||
event.src_path = self.bot.config_file
|
||||
self.bot.on_config_file_modified(event)
|
||||
self.bot.on_config_file_changed()
|
||||
self.assertEqual(self.bot.config["history-limit"], 42)
|
||||
|
||||
|
||||
class TestConfigReloadRenameSafe(unittest.TestCase):
|
||||
def test_atomic_rename_and_modify_trigger_reload(self):
|
||||
"""CFG-05: a modified OR a renamed-into-place config fires the reload; unrelated files do not."""
|
||||
from fjerkroa_bot.discord_bot import ConfigFileHandler
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
config = Path(tmp) / "kroa.toml"
|
||||
config.write_text("x = 1\n")
|
||||
hits = []
|
||||
handler = ConfigFileHandler(str(config), lambda: hits.append(1))
|
||||
|
||||
def evt(is_dir=False, src=None, dest=None):
|
||||
event = MagicMock()
|
||||
event.is_directory = is_dir
|
||||
event.src_path = src if src is not None else ""
|
||||
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_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
|
||||
|
||||
Reference in New Issue
Block a user