mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-22 10:58:15 +00:00
fix(plugins): warn once per directory, not once per scan
Self-review catch. Discovery runs on every web UI page load and every config reconcile, so warning unconditionally about an unloadable directory would put a line in the journal each time someone opened a page -- the same log-volume problem this change exists to help diagnose. The skip is now reported once per directory per process. The diagnostic value is unchanged: the reason a plugin is missing still appears in the journal, once, where before it appeared nowhere. Test added covering five consecutive scans producing one warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
This commit is contained in:
co-authored by
Claude Opus 5
parent
0a7d14d75e
commit
f7492e573a
@@ -76,6 +76,9 @@ class PluginManager:
|
|||||||
# Lock protecting plugin_manifests and plugin_directories from
|
# Lock protecting plugin_manifests and plugin_directories from
|
||||||
# concurrent mutation (background reconciliation) and reads (requests).
|
# concurrent mutation (background reconciliation) and reads (requests).
|
||||||
self._discovery_lock = threading.RLock()
|
self._discovery_lock = threading.RLock()
|
||||||
|
#: Directories already reported as unloadable, so the warning is
|
||||||
|
#: emitted once rather than on every discovery scan.
|
||||||
|
self._skip_reported: set = set()
|
||||||
|
|
||||||
# Lock protecting plugin_last_update from concurrent mutation/iteration.
|
# Lock protecting plugin_last_update from concurrent mutation/iteration.
|
||||||
# It's written from run_scheduled_updates()/update_all_plugins() (main
|
# It's written from run_scheduled_updates()/update_all_plugins() (main
|
||||||
@@ -196,15 +199,22 @@ class PluginManager:
|
|||||||
|
|
||||||
manifest_path = item / "manifest.json"
|
manifest_path = item / "manifest.json"
|
||||||
if not manifest_path.exists():
|
if not manifest_path.exists():
|
||||||
|
# Once per directory per process. Discovery runs on every
|
||||||
|
# web UI page load and every config reconcile, so warning
|
||||||
|
# unconditionally would put a line in the journal each
|
||||||
|
# time someone opened a page -- the same log-volume
|
||||||
|
# problem this is meant to help diagnose.
|
||||||
# A directory here that carries no manifest is not a
|
# A directory here that carries no manifest is not a
|
||||||
# plugin. Said once, because the alternative is a plugin
|
# plugin. Said once, because the alternative is a plugin
|
||||||
# that is enabled in config, enabled in plugin state,
|
# that is enabled in config, enabled in plugin state,
|
||||||
# present on disk, and simply absent from the running
|
# present on disk, and simply absent from the running
|
||||||
# process with nothing anywhere to say why. Working that
|
# process with nothing anywhere to say why. Working that
|
||||||
# out afterwards means reading cache-file mtimes.
|
# out afterwards means reading cache-file mtimes.
|
||||||
self.logger.warning(
|
if item.name not in self._skip_reported:
|
||||||
"Skipping %s: no manifest.json, so it cannot be loaded "
|
self._skip_reported.add(item.name)
|
||||||
"as a plugin", item.name)
|
self.logger.warning(
|
||||||
|
"Skipping %s: no manifest.json, so it cannot be "
|
||||||
|
"loaded as a plugin", item.name)
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
with open(manifest_path, 'r', encoding='utf-8') as f:
|
with open(manifest_path, 'r', encoding='utf-8') as f:
|
||||||
@@ -217,9 +227,11 @@ class PluginManager:
|
|||||||
if not plugin_id:
|
if not plugin_id:
|
||||||
# Parsed but unusable. This was the quietest path of all:
|
# Parsed but unusable. This was the quietest path of all:
|
||||||
# the manifest is read successfully and then dropped.
|
# the manifest is read successfully and then dropped.
|
||||||
self.logger.warning(
|
if item.name not in self._skip_reported:
|
||||||
"Skipping %s: its manifest.json has no \"id\", so there "
|
self._skip_reported.add(item.name)
|
||||||
"is nothing to register it under", item.name)
|
self.logger.warning(
|
||||||
|
"Skipping %s: its manifest.json has no \"id\", so "
|
||||||
|
"there is nothing to register it under", item.name)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
plugin_ids.append(plugin_id)
|
plugin_ids.append(plugin_id)
|
||||||
|
|||||||
@@ -28,6 +28,7 @@ def _manager(tmp_path):
|
|||||||
pm.plugin_manifests = {}
|
pm.plugin_manifests = {}
|
||||||
pm.plugin_directories = {}
|
pm.plugin_directories = {}
|
||||||
pm._discovery_lock = __import__("threading").RLock()
|
pm._discovery_lock = __import__("threading").RLock()
|
||||||
|
pm._skip_reported = set()
|
||||||
pm.schema_manager = MagicMock()
|
pm.schema_manager = MagicMock()
|
||||||
return pm
|
return pm
|
||||||
|
|
||||||
@@ -62,3 +63,19 @@ def test_a_good_plugin_still_registers(tmp_path, caplog):
|
|||||||
pm = _manager(tmp_path)
|
pm = _manager(tmp_path)
|
||||||
pm._scan_directory_for_plugins(tmp_path)
|
pm._scan_directory_for_plugins(tmp_path)
|
||||||
assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"
|
assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered"
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_warning_does_not_repeat_on_every_scan(tmp_path, caplog):
|
||||||
|
"""Discovery runs on every web UI page load and every config reconcile.
|
||||||
|
|
||||||
|
Warning unconditionally would put a line in the journal each time someone
|
||||||
|
opened a page -- the same log-volume problem this is meant to help
|
||||||
|
diagnose.
|
||||||
|
"""
|
||||||
|
(tmp_path / "not-a-plugin").mkdir()
|
||||||
|
pm = _manager(tmp_path)
|
||||||
|
with caplog.at_level(logging.WARNING, logger="test.discovery"):
|
||||||
|
for _ in range(5):
|
||||||
|
pm._scan_directory_for_plugins(tmp_path)
|
||||||
|
hits = [r for r in caplog.records if "not-a-plugin" in r.message]
|
||||||
|
assert len(hits) == 1, f"warned {len(hits)} times across 5 scans"
|
||||||
|
|||||||
Reference in New Issue
Block a user