fix(plugins): a plugin that can't start live says why and offers a restart (#6087)
Saving an enabled plugin threw away hot_reload_plugin's (ok, message), so a plugin that failed to load live read "Enabled, not loaded" with nothing more. Now: - a live-load failure is recorded with its reason, as a boot failure is, and logged; a later success clears it; - any save of an enabled plugin that isn't running tries to start it, not only the off-to-on toggle; - enabling one already running is a success, not a failure; - the plugin's page offers "Restart to load" while it is enabled and not running. Quart never marks the app as having served its first request, so register_blueprint after startup is allowed: hot-loading is not refused wholesale, as #6087 suspected. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -305,16 +305,29 @@ def load_plugins(app: "Quart") -> None:
|
||||
def hot_reload_plugin(app: "Quart", name: str) -> tuple[bool, str]:
|
||||
"""Activate a plugin that was just enabled without restarting the app.
|
||||
|
||||
This works for plugins that were NOT previously loaded. For plugins that
|
||||
are already registered (blueprint already mounted), a full restart is
|
||||
required to pick up code changes — use restart_app() in that case.
|
||||
A plugin that can't be loaded live is recorded with its reason, as a
|
||||
failed boot load is, so its settings page says why and offers a restart
|
||||
rather than reading "Enabled, not loaded" with nothing more (#6087).
|
||||
|
||||
Returns (success, message).
|
||||
"""
|
||||
ok, message = _hot_load(app, name)
|
||||
if ok:
|
||||
_FAILED_PLUGINS.pop(name, None)
|
||||
else:
|
||||
_FAILED_PLUGINS[name] = message
|
||||
logger.warning("Plugin %r could not be loaded live: %s", name, message)
|
||||
return ok, message
|
||||
|
||||
|
||||
def _hot_load(app: "Quart", name: str) -> tuple[bool, str]:
|
||||
"""Load one plugin into the running app. A plugin already loaded (enabled,
|
||||
disabled and enabled again without a restart) is still running, so that
|
||||
is a success; picking up its code changes needs restart_app()."""
|
||||
import steward
|
||||
|
||||
if name in _LOADED_PLUGINS:
|
||||
return False, "Plugin already loaded — restart required to apply updates"
|
||||
return True, "Plugin already running"
|
||||
|
||||
plugin_dirs = [Path(d).resolve() for d in app.config["PLUGIN_DIRS"]]
|
||||
plugin_path = resolve_plugin_path(plugin_dirs, name)
|
||||
@@ -373,9 +386,9 @@ def hot_reload_plugin(app: "Quart", name: str) -> tuple[bool, str]:
|
||||
)
|
||||
all_dirs = discover_all_in(plugin_dirs)
|
||||
run_plugin_migrations(app.config["DATABASE_URL"], all_dirs)
|
||||
except Exception:
|
||||
except Exception as exc:
|
||||
logger.exception("Plugin %r: migration failed during hot-reload", name)
|
||||
return False, "Migration failed — check logs"
|
||||
return False, f"Migration failed: {exc}"
|
||||
|
||||
# Import (file-path load avoids stdlib name collisions)
|
||||
try:
|
||||
|
||||
@@ -1091,8 +1091,11 @@ async def save_plugin_detail(name: str):
|
||||
action = "plugin.enabled" if new_enabled else "plugin.disabled"
|
||||
await log_audit(current_app, session.get("user_id"), session.get("username", ""),
|
||||
action, entity_type="plugin", entity_id=name)
|
||||
if new_enabled and not old_enabled:
|
||||
from steward.core.plugin_manager import hot_reload_plugin
|
||||
# Any save of an enabled plugin that isn't running tries to start it; a
|
||||
# failure is recorded with its reason, which the page shows beside a
|
||||
# restart (#6087).
|
||||
from steward.core.plugin_manager import _LOADED_PLUGINS, hot_reload_plugin
|
||||
if new_enabled and name not in _LOADED_PLUGINS:
|
||||
hot_reload_plugin(current_app._get_current_object(), name)
|
||||
|
||||
return redirect(url_for("settings.plugin_detail", name=name))
|
||||
|
||||
@@ -37,6 +37,20 @@
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if plugin._enabled and not is_loaded and not plugin._always_on %}
|
||||
<div id="restart-banner" style="display:flex;align-items:center;gap:0.75rem;flex-wrap:wrap;
|
||||
margin-bottom:1rem;max-width:720px;font-size:0.84rem;color:var(--text-muted);">
|
||||
<span>It's enabled but not running{% if fail_reason %}: it couldn't be started while Steward runs{% endif %}.
|
||||
A restart loads it.</span>
|
||||
<form method="post" action="/settings/plugins/restart/"
|
||||
hx-post="/settings/plugins/restart/" hx-target="#restart-banner" hx-swap="outerHTML"
|
||||
data-confirm="Restart the app now? This will briefly interrupt all monitoring."
|
||||
style="margin:0;">{{ csrf_field() }}
|
||||
<button type="submit" class="btn btn-sm">Restart to load</button>
|
||||
</form>
|
||||
</div>
|
||||
{% endif %}
|
||||
|
||||
{% if plugin.get('description') %}
|
||||
<p style="color:var(--text-muted);font-size:0.88rem;margin-bottom:1.25rem;max-width:620px;">{{ plugin.description }}</p>
|
||||
{% endif %}
|
||||
|
||||
@@ -0,0 +1,64 @@
|
||||
"""A plugin enabled while Steward runs is loaded live; one that can't be is
|
||||
recorded with its reason, so its settings page says why and offers a restart
|
||||
instead of a bare "Enabled, not loaded" (#6087)."""
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from steward.core import plugin_manager as pm
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clean_registries(monkeypatch):
|
||||
monkeypatch.setattr(pm, "_LOADED_PLUGINS", set())
|
||||
monkeypatch.setattr(pm, "_FAILED_PLUGINS", {})
|
||||
monkeypatch.setattr(pm, "_PLUGIN_NAV", [])
|
||||
monkeypatch.setattr(pm, "_PLUGIN_MODULES", {})
|
||||
yield
|
||||
for key in [k for k in sys.modules if k.startswith("_steward_plugin_hl_")]:
|
||||
sys.modules.pop(key)
|
||||
|
||||
|
||||
def _app(root):
|
||||
return SimpleNamespace(config={"PLUGIN_DIRS": [str(root)], "PLUGINS": {}},
|
||||
_task_registry=[], mcp_server=None)
|
||||
|
||||
|
||||
def _plugin(root, name, *, yaml_name=None, setup_body="pass"):
|
||||
pdir = root / name
|
||||
pdir.mkdir(parents=True)
|
||||
(pdir / "plugin.yaml").write_text(f"name: {yaml_name or name}\nversion: 1.0.0\n")
|
||||
(pdir / "__init__.py").write_text(
|
||||
f"def setup(app):\n {setup_body}\n\n"
|
||||
"def get_scheduled_tasks():\n return []\n")
|
||||
|
||||
|
||||
def test_a_plugin_that_cannot_load_live_is_recorded_with_its_reason(tmp_path):
|
||||
_plugin(tmp_path, "hl_broken", setup_body="raise RuntimeError('no controller')")
|
||||
ok, message = pm.hot_reload_plugin(_app(tmp_path), "hl_broken")
|
||||
assert not ok
|
||||
assert pm.get_plugin_failures() == {"hl_broken": "setup() raised: no controller"}
|
||||
assert "hl_broken" not in pm._LOADED_PLUGINS
|
||||
|
||||
|
||||
def test_an_early_refusal_is_recorded_too(tmp_path):
|
||||
_plugin(tmp_path, "hl_misnamed", yaml_name="other")
|
||||
ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_misnamed")
|
||||
assert not ok and "name mismatch" in pm.get_plugin_failures()["hl_misnamed"]
|
||||
|
||||
|
||||
def test_a_plugin_that_loads_clears_an_earlier_failure(tmp_path):
|
||||
_plugin(tmp_path, "hl_good")
|
||||
pm._FAILED_PLUGINS["hl_good"] = "setup() raised: earlier"
|
||||
ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_good")
|
||||
assert ok
|
||||
assert "hl_good" in pm._LOADED_PLUGINS and pm.get_plugin_failures() == {}
|
||||
|
||||
|
||||
def test_enabling_one_already_running_is_a_success_not_a_failure(tmp_path):
|
||||
pm._LOADED_PLUGINS.add("hl_running")
|
||||
ok, _ = pm.hot_reload_plugin(_app(tmp_path), "hl_running")
|
||||
assert ok and pm.get_plugin_failures() == {}
|
||||
Reference in New Issue
Block a user