mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-21 10:29:06 +00:00
Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
073435a2ac | ||
|
|
3b4afa2f85 | ||
|
|
5b64dbe847 | ||
|
|
ceabe5d4f9 |
Binary file not shown.
|
Before Width: | Height: | Size: 467 B |
Executable → Regular
Executable → Regular
Executable → Regular
@@ -328,7 +328,7 @@ class ScrollHelper:
|
||||
elapsed_time = current_time - (self.scroll_start_time or current_time)
|
||||
# The image already includes display_width padding, so we only need total_scroll_width
|
||||
required_total_distance = self.total_scroll_width
|
||||
self.logger.info(
|
||||
self.logger.debug(
|
||||
"Scroll progress: elapsed=%.2fs, target=%.2fs, total_scrolled=%.0f/%d px (%.1f%%)",
|
||||
elapsed_time,
|
||||
self.calculated_duration,
|
||||
|
||||
+66
-1
@@ -130,7 +130,12 @@ def setup_logging(
|
||||
# Console handler (always add)
|
||||
console_handler = logging.StreamHandler(sys.stdout)
|
||||
console_handler.setLevel(level)
|
||||
console_handler.setFormatter(formatter)
|
||||
# Under systemd, tag each line so the journal records the real severity
|
||||
# rather than filing everything as informational. The file handler below
|
||||
# keeps the plain formatter: the prefix is meaningful to journald and noise
|
||||
# anywhere else.
|
||||
console_handler.setFormatter(
|
||||
JournalPriorityFormatter(formatter) if _under_systemd() else formatter)
|
||||
root_logger.addHandler(console_handler)
|
||||
|
||||
# File handler (if specified)
|
||||
@@ -145,6 +150,66 @@ def setup_logging(
|
||||
sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
|
||||
|
||||
|
||||
#: syslog priorities, which is what systemd parses from a "<N>" prefix on
|
||||
#: stdout. Mapped from Python's levels.
|
||||
_SYSLOG_PRIORITY = {
|
||||
logging.CRITICAL: 2, # LOG_CRIT
|
||||
logging.ERROR: 3, # LOG_ERR
|
||||
logging.WARNING: 4, # LOG_WARNING
|
||||
logging.INFO: 6, # LOG_INFO
|
||||
logging.DEBUG: 7, # LOG_DEBUG
|
||||
}
|
||||
|
||||
|
||||
class JournalPriorityFormatter(logging.Formatter):
|
||||
"""Wraps a formatter, prefixing each line with its syslog priority.
|
||||
|
||||
Under systemd everything this process writes to stdout lands in the journal
|
||||
as PRIORITY=6, whatever the Python level was. Measured on a live rig: 55
|
||||
ERROR lines and 13 WARNING lines in a day, every one of them recorded as
|
||||
informational, so `journalctl -p err -u ledmatrix` returned nothing at all
|
||||
while errors were being logged. Anyone triaging has to grep the message
|
||||
text instead, which is both slower and wrong -- a search for "oom" matches
|
||||
the radar logging "zoom=9".
|
||||
|
||||
systemd reads a leading "<N>" on each line and uses it as the priority
|
||||
(sd-daemon(3)), so this needs no extra dependency. Multi-line records get
|
||||
the prefix on every line, since the journal splits them and an unprefixed
|
||||
continuation would fall back to the default.
|
||||
"""
|
||||
|
||||
def __init__(self, inner: logging.Formatter):
|
||||
super().__init__()
|
||||
self._inner = inner
|
||||
|
||||
@property
|
||||
def inner(self) -> logging.Formatter:
|
||||
"""The formatter doing the actual work.
|
||||
|
||||
Whether journald tagging is applied depends on JOURNAL_STREAM, so it is
|
||||
on under systemd and off in a terminal -- and anything asserting which
|
||||
formatter setup_logging() selected would otherwise get a different
|
||||
answer in CI than on a developer's machine. Exposing the inner one lets
|
||||
those checks stay about format_type, which is what they mean.
|
||||
"""
|
||||
return self._inner
|
||||
|
||||
def format(self, record: logging.LogRecord) -> str:
|
||||
text = self._inner.format(record)
|
||||
prefix = f"<{_SYSLOG_PRIORITY.get(record.levelno, 6)}>"
|
||||
return "\n".join(prefix + line for line in text.split("\n"))
|
||||
|
||||
|
||||
def _under_systemd() -> bool:
|
||||
"""True when stdout is the journal.
|
||||
|
||||
systemd sets JOURNAL_STREAM for services whose output it captures. Without
|
||||
this check the "<N>" prefixes would show up as literal noise when the
|
||||
program is run from a terminal, in the emulator, or in tests.
|
||||
"""
|
||||
return bool(os.environ.get("JOURNAL_STREAM"))
|
||||
|
||||
|
||||
class PluginLoggerAdapter(logging.LoggerAdapter):
|
||||
"""LoggerAdapter that stamps every record with its plugin_id.
|
||||
|
||||
|
||||
@@ -178,11 +178,21 @@ class PluginHealthTracker:
|
||||
)
|
||||
return self._health_state[plugin_id]
|
||||
|
||||
# Fields the circuit breaker is rebuilt from after a restart. Everything
|
||||
# else in a health record is reporting, read only for display.
|
||||
_DURABLE_FIELDS = ('consecutive_failures', 'circuit_state',
|
||||
'circuit_opened_time', 'half_open_start_time')
|
||||
|
||||
def _durable(self, state: Dict[str, Any]) -> tuple:
|
||||
"""The part of a health record whose loss would change behaviour."""
|
||||
return tuple(state.get(field) for field in self._DURABLE_FIELDS)
|
||||
|
||||
def record_success(self, plugin_id: str) -> None:
|
||||
"""Record a successful plugin execution."""
|
||||
state = self.get_health_state(plugin_id)
|
||||
current_time = time.time()
|
||||
|
||||
durable_before = self._durable(state)
|
||||
|
||||
# Reset consecutive failures
|
||||
state['consecutive_failures'] = 0
|
||||
state['total_successes'] = state.get('total_successes', 0) + 1
|
||||
@@ -198,9 +208,20 @@ class PluginHealthTracker:
|
||||
# Shouldn't happen, but handle it
|
||||
state['circuit_state'] = CircuitState.CLOSED.value
|
||||
state['circuit_opened_time'] = None
|
||||
|
||||
self._save_health_state(plugin_id, state)
|
||||
|
||||
|
||||
# A healthy plugin reports success every cycle, and in that steady state
|
||||
# the only fields changed above are a counter and a timestamp that
|
||||
# nothing reads back after a restart. Persisting them anyway rewrites a
|
||||
# small file per plugin per cycle: on a rig running 24 plugins, a
|
||||
# five-minute sample measured 22 rewrites, about 4.4 a minute or 6,300 a
|
||||
# day. Those land on an SD card, where the cost is an erase-block cycle
|
||||
# rather than the 400 bytes involved, and where wear is what eventually
|
||||
# kills the card.
|
||||
# In-memory state is still updated every time, so the health API and web
|
||||
# UI show exactly what they did before; only the write is skipped.
|
||||
if self._durable(state) != durable_before:
|
||||
self._save_health_state(plugin_id, state)
|
||||
|
||||
def record_failure(self, plugin_id: str, error: Optional[Exception] = None) -> None:
|
||||
"""Record a failed plugin execution."""
|
||||
state = self.get_health_state(plugin_id)
|
||||
|
||||
@@ -83,7 +83,7 @@ class PluginAdapter:
|
||||
# into unrelated headlines once the strip refreshed to 9,505px.
|
||||
self._offset_shapes: dict = {}
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"PluginAdapter initialized: display=%dx%d",
|
||||
self.display_width, self.display_height
|
||||
)
|
||||
@@ -109,7 +109,7 @@ class PluginAdapter:
|
||||
Returns:
|
||||
List of PIL Images representing plugin content, or None if no content
|
||||
"""
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Getting content (class=%s)",
|
||||
plugin_id, plugin.__class__.__name__
|
||||
)
|
||||
@@ -118,7 +118,7 @@ class PluginAdapter:
|
||||
cached = self._get_cached(plugin_id)
|
||||
if cached is not None:
|
||||
total_width = sum(img.width for img in cached)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Using cached content: %d images, %dpx total",
|
||||
plugin_id, len(cached), total_width
|
||||
)
|
||||
@@ -126,46 +126,46 @@ class PluginAdapter:
|
||||
|
||||
# Try native Vegas content method first
|
||||
has_native = hasattr(plugin, 'get_vegas_content')
|
||||
logger.info("[%s] Has get_vegas_content: %s", plugin_id, has_native)
|
||||
logger.debug("[%s] Has get_vegas_content: %s", plugin_id, has_native)
|
||||
if has_native:
|
||||
content = self._get_native_content(plugin, plugin_id, offscreen_only)
|
||||
if content:
|
||||
total_width = sum(img.width for img in content)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native content SUCCESS: %d images, %dpx total",
|
||||
plugin_id, len(content), total_width
|
||||
)
|
||||
return self._finalize(content, plugin_id, 'native', plugin)
|
||||
logger.info("[%s] Native content returned None", plugin_id)
|
||||
logger.debug("[%s] Native content returned None", plugin_id)
|
||||
|
||||
# Try to get scroll_helper's cached image (for scrolling plugins like stocks/odds)
|
||||
has_scroll_helper = hasattr(plugin, 'scroll_helper')
|
||||
logger.info("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper)
|
||||
logger.debug("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper)
|
||||
content = self._get_scroll_helper_content(plugin, plugin_id, offscreen_only)
|
||||
if content:
|
||||
total_width = sum(img.width for img in content)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] ScrollHelper content SUCCESS: %d images, %dpx total",
|
||||
plugin_id, len(content), total_width
|
||||
)
|
||||
return self._finalize(content, plugin_id, 'scroll_helper', plugin)
|
||||
if has_scroll_helper:
|
||||
logger.info("[%s] ScrollHelper content returned None", plugin_id)
|
||||
logger.debug("[%s] ScrollHelper content returned None", plugin_id)
|
||||
|
||||
if offscreen_only:
|
||||
# Display capture needs the shared canvas; leave it to the caller.
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Needs display capture, deferring to the render thread",
|
||||
plugin_id
|
||||
)
|
||||
return None
|
||||
|
||||
# Fall back to display capture
|
||||
logger.info("[%s] Trying fallback display capture...", plugin_id)
|
||||
logger.debug("[%s] Trying fallback display capture...", plugin_id)
|
||||
content = self._capture_display_content(plugin, plugin_id)
|
||||
if content:
|
||||
total_width = sum(img.width for img in content)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback capture SUCCESS: %d images, %dpx total",
|
||||
plugin_id, len(content), total_width
|
||||
)
|
||||
@@ -226,7 +226,7 @@ class PluginAdapter:
|
||||
kept.append(result.image)
|
||||
|
||||
if not kept:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] All %d image(s) from %s were blank — contributing nothing",
|
||||
plugin_id, len(images), source
|
||||
)
|
||||
@@ -235,14 +235,14 @@ class PluginAdapter:
|
||||
trimmed_width = sum(img.width for img in kept)
|
||||
|
||||
if trimmed_width < self.config.min_plugin_width:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Trimmed content %dpx is below min_plugin_width %dpx — skipping",
|
||||
plugin_id, trimmed_width, self.config.min_plugin_width
|
||||
)
|
||||
return None
|
||||
|
||||
if trimmed_width != original_width or dropped_blank:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Trimmed %s content: %dpx -> %dpx (%.0f%% reclaimed), "
|
||||
"%d image(s) kept, %d blank dropped",
|
||||
plugin_id, source, original_width, trimmed_width,
|
||||
@@ -431,7 +431,7 @@ class PluginAdapter:
|
||||
"""
|
||||
if self._offset_shapes.get(plugin_id) != shape:
|
||||
if plugin_id in self._item_offsets:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Content is %s now, was %s — restarting the rotation "
|
||||
"rather than resuming at a position that no longer means "
|
||||
"anything", plugin_id, shape,
|
||||
@@ -579,7 +579,7 @@ class PluginAdapter:
|
||||
consumed += 1
|
||||
|
||||
if mode == 'truncate':
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Width budget %dpx: showing the first %d of %d row(s) "
|
||||
"(%dpx incl. gaps); the rest are not shown (overflow=truncate)",
|
||||
plugin_id, budget, len(selected), len(images), used
|
||||
@@ -587,7 +587,7 @@ class PluginAdapter:
|
||||
else:
|
||||
self._record_offset(
|
||||
plugin_id, (start + consumed) % len(images), shape)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Width budget %dpx: showing %d of %d row(s) (%dpx incl. gaps) "
|
||||
"from offset %d; remainder deferred to a later cycle",
|
||||
plugin_id, budget, len(selected), len(images), used, start
|
||||
@@ -636,7 +636,7 @@ class PluginAdapter:
|
||||
if mode != 'truncate':
|
||||
self._record_offset(
|
||||
plugin_id, 0 if end >= img.width else end, shape)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Width budget %dpx: cropped continuous %dpx image to "
|
||||
"[%d:%d] (no item gaps of %dpx+ to align to)%s",
|
||||
plugin_id, budget, img.width, offset, end, min_run,
|
||||
@@ -674,7 +674,7 @@ class PluginAdapter:
|
||||
self._record_offset(
|
||||
plugin_id, 0 if end >= img.width else end_index, shape)
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Width budget %dpx: cropped single %dpx image to [%d:%d] "
|
||||
"(%dpx) at item boundaries %d-%d of %d, %s",
|
||||
plugin_id, budget, img.width, start, end, end - start,
|
||||
@@ -698,7 +698,7 @@ class PluginAdapter:
|
||||
List of images or None
|
||||
"""
|
||||
try:
|
||||
logger.info("[%s] Native: calling get_vegas_content()", plugin_id)
|
||||
logger.debug("[%s] Native: calling get_vegas_content()", plugin_id)
|
||||
|
||||
# Tell the plugin how much width the ticker wants it to use, and
|
||||
# narrow the canvas for the duration of the call. A plugin that
|
||||
@@ -707,7 +707,7 @@ class PluginAdapter:
|
||||
# be explicit can read get_vegas_render_width().
|
||||
render_width = self.resolve_render_width(plugin, plugin_id)
|
||||
if render_width != self.display_width:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: requesting %dpx instead of %dpx",
|
||||
plugin_id, render_width, self.display_width
|
||||
)
|
||||
@@ -735,19 +735,19 @@ class PluginAdapter:
|
||||
plugin._vegas_render_width = None
|
||||
|
||||
if result is None:
|
||||
logger.info("[%s] Native: get_vegas_content() returned None", plugin_id)
|
||||
logger.debug("[%s] Native: get_vegas_content() returned None", plugin_id)
|
||||
return None
|
||||
|
||||
# Normalize to list
|
||||
if isinstance(result, Image.Image):
|
||||
images = [result]
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: got single Image %dx%d",
|
||||
plugin_id, result.width, result.height
|
||||
)
|
||||
elif isinstance(result, (list, tuple)):
|
||||
images = list(result)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: got %d items in list/tuple",
|
||||
plugin_id, len(images)
|
||||
)
|
||||
@@ -768,14 +768,14 @@ class PluginAdapter:
|
||||
)
|
||||
continue
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: item[%d] is %dx%d, mode=%s",
|
||||
plugin_id, i, img.width, img.height, img.mode
|
||||
)
|
||||
|
||||
# Ensure correct height
|
||||
if img.height != self.display_height:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: resizing item[%d]: %dx%d -> %dx%d",
|
||||
plugin_id, i, img.width, img.height,
|
||||
img.width, self.display_height
|
||||
@@ -793,13 +793,13 @@ class PluginAdapter:
|
||||
|
||||
if valid_images:
|
||||
total_width = sum(img.width for img in valid_images)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Native: SUCCESS - %d images, %dpx total width",
|
||||
plugin_id, len(valid_images), total_width
|
||||
)
|
||||
return valid_images
|
||||
|
||||
logger.info("[%s] Native: no valid images after validation", plugin_id)
|
||||
logger.debug("[%s] Native: no valid images after validation", plugin_id)
|
||||
return None
|
||||
|
||||
except (AttributeError, TypeError, ValueError, OSError) as e:
|
||||
@@ -833,20 +833,20 @@ class PluginAdapter:
|
||||
logger.debug("[%s] No scroll_helper attribute", plugin_id)
|
||||
return None
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Found scroll_helper: %s",
|
||||
plugin_id, type(scroll_helper).__name__
|
||||
)
|
||||
|
||||
cached_image = getattr(scroll_helper, 'cached_image', None)
|
||||
if cached_image is None:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] scroll_helper.cached_image is None, triggering content generation",
|
||||
plugin_id
|
||||
)
|
||||
if offscreen_only:
|
||||
# Generating it calls display(), which needs the canvas.
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] scroll_helper cache empty; deferring generation "
|
||||
"to the render thread", plugin_id
|
||||
)
|
||||
@@ -859,13 +859,13 @@ class PluginAdapter:
|
||||
return None
|
||||
|
||||
if not isinstance(cached_image, Image.Image):
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] scroll_helper.cached_image is not an Image: %s",
|
||||
plugin_id, type(cached_image).__name__
|
||||
)
|
||||
return None
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] scroll_helper.cached_image found: %dx%d, mode=%s",
|
||||
plugin_id, cached_image.width, cached_image.height, cached_image.mode
|
||||
)
|
||||
@@ -888,7 +888,7 @@ class PluginAdapter:
|
||||
|
||||
# Ensure correct height
|
||||
if img.height != self.display_height:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Resizing scroll_helper content: %dx%d -> %dx%d",
|
||||
plugin_id, img.width, img.height,
|
||||
img.width, self.display_height
|
||||
@@ -902,7 +902,7 @@ class PluginAdapter:
|
||||
if img.mode != 'RGB':
|
||||
img = img.convert('RGB')
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] ScrollHelper content ready: %dx%d",
|
||||
plugin_id, img.width, img.height
|
||||
)
|
||||
@@ -1002,7 +1002,7 @@ class PluginAdapter:
|
||||
with self._capture():
|
||||
# Method 1: Try _create_scrolling_display (stocks pattern)
|
||||
if hasattr(plugin, '_create_scrolling_display'):
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Triggering via _create_scrolling_display()",
|
||||
plugin_id
|
||||
)
|
||||
@@ -1010,7 +1010,7 @@ class PluginAdapter:
|
||||
plugin._create_scrolling_display()
|
||||
cached_image = getattr(scroll_helper, 'cached_image', None)
|
||||
if cached_image is not None and isinstance(cached_image, Image.Image):
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] _create_scrolling_display() SUCCESS: %dx%d",
|
||||
plugin_id, cached_image.width, cached_image.height
|
||||
)
|
||||
@@ -1022,7 +1022,7 @@ class PluginAdapter:
|
||||
|
||||
# Method 2: Try display(force_clear=True) which typically builds scroll content
|
||||
if hasattr(plugin, 'display'):
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Triggering via display(force_clear=True)",
|
||||
plugin_id
|
||||
)
|
||||
@@ -1031,12 +1031,12 @@ class PluginAdapter:
|
||||
plugin.display(force_clear=True)
|
||||
cached_image = getattr(scroll_helper, 'cached_image', None)
|
||||
if cached_image is not None and isinstance(cached_image, Image.Image):
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] display(force_clear=True) SUCCESS: %dx%d",
|
||||
plugin_id, cached_image.width, cached_image.height
|
||||
)
|
||||
return cached_image
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] display(force_clear=True) did not populate cached_image",
|
||||
plugin_id
|
||||
)
|
||||
@@ -1045,7 +1045,7 @@ class PluginAdapter:
|
||||
"[%s] display(force_clear=True) failed", plugin_id
|
||||
)
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Could not trigger scroll content generation",
|
||||
plugin_id
|
||||
)
|
||||
@@ -1077,15 +1077,15 @@ class PluginAdapter:
|
||||
try:
|
||||
# Save current display state
|
||||
original_image = self.display_manager.image.copy()
|
||||
logger.info("[%s] Fallback: saved original display state", plugin_id)
|
||||
logger.debug("[%s] Fallback: saved original display state", plugin_id)
|
||||
|
||||
# Ensure plugin has fresh data before capturing
|
||||
has_update_data = hasattr(plugin, 'update_data')
|
||||
logger.info("[%s] Fallback: has update_data=%s", plugin_id, has_update_data)
|
||||
logger.debug("[%s] Fallback: has update_data=%s", plugin_id, has_update_data)
|
||||
if has_update_data:
|
||||
try:
|
||||
plugin.update_data()
|
||||
logger.info("[%s] Fallback: update_data() called", plugin_id)
|
||||
logger.debug("[%s] Fallback: update_data() called", plugin_id)
|
||||
except (AttributeError, RuntimeError, OSError):
|
||||
logger.exception("[%s] Fallback: update_data() failed", plugin_id)
|
||||
|
||||
@@ -1097,41 +1097,41 @@ class PluginAdapter:
|
||||
# arrangement rather than one that has to be cropped afterwards.
|
||||
render_width = self.resolve_render_width(plugin, plugin_id)
|
||||
if render_width != self.display_width:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: rendering at %dpx instead of %dpx",
|
||||
plugin_id, render_width, self.display_width
|
||||
)
|
||||
|
||||
with self._capture(), self._render_at(render_width):
|
||||
self.display_manager.clear()
|
||||
logger.info("[%s] Fallback: display cleared, calling display()", plugin_id)
|
||||
logger.debug("[%s] Fallback: display cleared, calling display()", plugin_id)
|
||||
|
||||
# First try without force_clear (some plugins behave better this way)
|
||||
try:
|
||||
plugin.display()
|
||||
logger.info("[%s] Fallback: display() called successfully", plugin_id)
|
||||
logger.debug("[%s] Fallback: display() called successfully", plugin_id)
|
||||
except TypeError:
|
||||
# Plugin may require force_clear argument
|
||||
logger.info("[%s] Fallback: display() failed, trying with force_clear=True", plugin_id)
|
||||
logger.debug("[%s] Fallback: display() failed, trying with force_clear=True", plugin_id)
|
||||
plugin.display(force_clear=True)
|
||||
|
||||
# Capture the result
|
||||
captured = self.display_manager.image.copy()
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: captured frame %dx%d, mode=%s",
|
||||
plugin_id, captured.width, captured.height, captured.mode
|
||||
)
|
||||
|
||||
# Check if captured image has content (not all black)
|
||||
is_blank, bright_ratio = self._is_blank_image(captured, return_ratio=True)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: brightness check - %.3f%% bright pixels (threshold=0.5%%)",
|
||||
plugin_id, bright_ratio * 100
|
||||
)
|
||||
|
||||
if is_blank:
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: first capture blank, retrying with force_clear",
|
||||
plugin_id
|
||||
)
|
||||
@@ -1142,7 +1142,7 @@ class PluginAdapter:
|
||||
captured = self.display_manager.image.copy()
|
||||
|
||||
is_blank, bright_ratio = self._is_blank_image(captured, return_ratio=True)
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: retry brightness - %.3f%% bright pixels",
|
||||
plugin_id, bright_ratio * 100
|
||||
)
|
||||
@@ -1159,7 +1159,7 @@ class PluginAdapter:
|
||||
if captured.mode != 'RGB':
|
||||
captured = captured.convert('RGB')
|
||||
|
||||
logger.info(
|
||||
logger.debug(
|
||||
"[%s] Fallback: SUCCESS - captured %dx%d",
|
||||
plugin_id, captured.width, captured.height
|
||||
)
|
||||
|
||||
@@ -143,12 +143,6 @@ def mask_secret_fields(config: Dict[str, Any], schema_properties: Dict[str, Any]
|
||||
return result
|
||||
|
||||
|
||||
#: What a masked secret looks like on the wire. Named because the write path
|
||||
#: has to recognise it coming back: a client that renders the mask and posts
|
||||
#: it unchanged must not store the mask as if it were the secret.
|
||||
SECRET_MASK = '\u2022' * 8
|
||||
|
||||
|
||||
def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Blanket-mask every non-empty value in a secrets config dict.
|
||||
|
||||
@@ -167,7 +161,7 @@ def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]:
|
||||
if isinstance(v, dict):
|
||||
masked[k] = mask_all_secret_values(v)
|
||||
elif v not in (None, '') and not (isinstance(v, str) and v.startswith('YOUR_')):
|
||||
masked[k] = SECRET_MASK
|
||||
masked[k] = '••••••••'
|
||||
else:
|
||||
masked[k] = v
|
||||
return masked
|
||||
@@ -195,30 +189,3 @@ def remove_empty_secrets(secrets: Dict[str, Any]) -> Dict[str, Any]:
|
||||
elif v is not None and not (isinstance(v, str) and v.strip() == ''):
|
||||
result[k] = v
|
||||
return result
|
||||
|
||||
|
||||
def strip_masked_values(secrets: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""Remove values a client echoed back rather than changed.
|
||||
|
||||
The counterpart to :func:`mask_all_secret_values`. A client that GETs the
|
||||
masked secrets, edits one field and POSTs the whole object back is sending
|
||||
``SECRET_MASK`` for every field it did not touch. Storing those would
|
||||
replace each untouched credential with eight bullet characters.
|
||||
|
||||
Drops the mask and, like :func:`remove_empty_secrets`, blank values -- so
|
||||
the caller can merge the result onto what is already stored and have
|
||||
"unchanged" mean unchanged. Empty nested dicts are pruned.
|
||||
"""
|
||||
result: Dict[str, Any] = {}
|
||||
for k, v in secrets.items():
|
||||
if isinstance(v, dict):
|
||||
nested = strip_masked_values(v)
|
||||
if nested:
|
||||
result[k] = nested
|
||||
elif v is None:
|
||||
continue
|
||||
elif isinstance(v, str) and (v.strip() == '' or v == SECRET_MASK):
|
||||
continue
|
||||
else:
|
||||
result[k] = v
|
||||
return result
|
||||
|
||||
Executable → Regular
Executable → Regular
@@ -1,143 +0,0 @@
|
||||
"""GET /config/main must not hand out credentials.
|
||||
|
||||
The endpoint returned the raw config to anyone who could reach the port, and
|
||||
this web interface has no authentication of any kind. Measured against a live
|
||||
rig, an unauthenticated request returned:
|
||||
|
||||
github.api_token 40 chars
|
||||
incoming-packages.ha_token 183 chars
|
||||
jellyfin-now-playing.api_key 32 chars
|
||||
ledmatrix-weather.api_key 32 chars
|
||||
on-air.mqtt_password 8 chars
|
||||
youtube.api_key 20 chars
|
||||
youtube-stats.api_key 39 chars
|
||||
|
||||
A GitHub token and a Home Assistant long-lived token among them.
|
||||
|
||||
The x-secret masking the plugin config endpoints use does not apply here: this
|
||||
endpoint never consults a schema, and core keys such as github.api_token have
|
||||
no schema to carry the marker. Several of those fields *are* tagged x-secret in
|
||||
their plugin's schema and were still returned in full, which is what makes the
|
||||
schema route the wrong one to rely on for this endpoint.
|
||||
|
||||
Matching on field name is blunt. For a whole-config dump it is the right
|
||||
default: anything named like a credential should not leave the process, and a
|
||||
new plugin that adds a differently-shaped secret is covered without anyone
|
||||
remembering to tag it.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from web_interface.blueprints.api_v3 import (
|
||||
_looks_like_a_credential,
|
||||
_redact_credentials,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", [
|
||||
"password", "mqtt_password", "opensky_password", "passwd",
|
||||
"api_key", "apikey", "API_KEY", "flightaware_api_key",
|
||||
"token", "ha_token", "api_token", "access_token",
|
||||
"secret", "client_secret", "spotify_client_secret",
|
||||
"access_key", "private_key",
|
||||
])
|
||||
def test_credential_names_are_recognised(name):
|
||||
assert _looks_like_a_credential(name)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", [
|
||||
"timezone", "city", "brightness", "enabled", "update_interval",
|
||||
"favorite_teams", "display_duration", "keyword",
|
||||
])
|
||||
def test_ordinary_names_are_left_alone(name):
|
||||
assert not _looks_like_a_credential(name)
|
||||
|
||||
|
||||
def test_the_measured_leak_is_closed():
|
||||
"""The exact shape taken off the rig."""
|
||||
config = {
|
||||
"github": {"api_token": "ghp_" + "x" * 36},
|
||||
"incoming-packages": {"ha_token": "y" * 183, "enabled": True},
|
||||
"jellyfin-now-playing": {"api_key": "z" * 32},
|
||||
"on-air": {"mqtt_password": "hunter22"},
|
||||
"youtube": {"api_key": "k" * 20},
|
||||
"timezone": "America/New_York",
|
||||
}
|
||||
out = _redact_credentials(config)
|
||||
assert out["github"]["api_token"] == ""
|
||||
assert out["incoming-packages"]["ha_token"] == ""
|
||||
assert out["jellyfin-now-playing"]["api_key"] == ""
|
||||
assert out["on-air"]["mqtt_password"] == ""
|
||||
assert out["youtube"]["api_key"] == ""
|
||||
# Everything else survives, or the config editor breaks.
|
||||
assert out["timezone"] == "America/New_York"
|
||||
assert out["incoming-packages"]["enabled"] is True
|
||||
|
||||
|
||||
def test_nested_and_listed_credentials_are_reached():
|
||||
config = {"a": {"b": {"c": {"password": "p"}}},
|
||||
"feeds": [{"name": "x", "api_key": "k"}, {"name": "y"}]}
|
||||
out = _redact_credentials(config)
|
||||
assert out["a"]["b"]["c"]["password"] == ""
|
||||
assert out["feeds"][0]["api_key"] == ""
|
||||
assert out["feeds"][0]["name"] == "x"
|
||||
|
||||
|
||||
def test_the_original_is_not_mutated():
|
||||
"""The caller holds the live config; redaction must not edit it in place."""
|
||||
config = {"github": {"api_token": "keepme"}}
|
||||
_redact_credentials(config)
|
||||
assert config["github"]["api_token"] == "keepme"
|
||||
|
||||
|
||||
def test_a_credential_shaped_container_is_still_walked():
|
||||
"""`secrets: {...}` is a section name, not a value to blank."""
|
||||
config = {"secrets": {"api_key": "k", "note": "keep"}}
|
||||
out = _redact_credentials(config)
|
||||
assert out["secrets"]["api_key"] == ""
|
||||
assert out["secrets"]["note"] == "keep"
|
||||
|
||||
|
||||
def test_non_dict_input_passes_through():
|
||||
assert _redact_credentials("plain") == "plain"
|
||||
assert _redact_credentials(7) == 7
|
||||
assert _redact_credentials(None) is None
|
||||
|
||||
|
||||
def test_the_endpoint_itself_redacts():
|
||||
"""Through the view function, not the helper.
|
||||
|
||||
The helper tests above all passed with the route still returning
|
||||
`config` -- reverting the one line that calls the redactor changed
|
||||
nothing, because nothing exercised the route. A property asserted on a
|
||||
helper is not a property asserted on the endpoint, and it is the endpoint
|
||||
that is exposed to the network.
|
||||
"""
|
||||
import json as _json
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import flask
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod
|
||||
|
||||
raw = {"github": {"api_token": "ghp_secret_value"},
|
||||
"timezone": "America/New_York"}
|
||||
|
||||
manager = MagicMock()
|
||||
manager.load_config.return_value = raw
|
||||
previous = getattr(mod.api_v3, "config_manager", None)
|
||||
mod.api_v3.config_manager = manager
|
||||
|
||||
app = flask.Flask(__name__)
|
||||
try:
|
||||
with app.test_request_context("/config/main"):
|
||||
response = mod.get_main_config()
|
||||
payload = response.get_json() if hasattr(response, "get_json") else _json.loads(response[0].data)
|
||||
finally:
|
||||
mod.api_v3.config_manager = previous
|
||||
|
||||
data = payload["data"]
|
||||
assert data["github"]["api_token"] == "", (
|
||||
"the endpoint returned the token; the redactor is not wired in")
|
||||
assert data["timezone"] == "America/New_York"
|
||||
# And the config the manager handed over is untouched.
|
||||
assert raw["github"]["api_token"] == "ghp_secret_value"
|
||||
@@ -58,7 +58,7 @@ def repos(tmp_path):
|
||||
def test_branch_with_upstream_uses_a_plain_pull(repos):
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash']
|
||||
assert args == ['git', 'pull', '--rebase']
|
||||
assert note == ''
|
||||
|
||||
|
||||
@@ -73,7 +73,7 @@ def test_branch_without_upstream_falls_back_to_origin_branch(repos):
|
||||
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash', 'origin', 'audit']
|
||||
assert args == ['git', 'pull', '--rebase', 'origin', 'audit']
|
||||
assert 'audit' in note
|
||||
|
||||
|
||||
@@ -155,7 +155,7 @@ def test_switching_attaches_tracking_so_pull_needs_no_fallback(repos):
|
||||
|
||||
args, note, error = resolve_pull_command(str(repos))
|
||||
assert error is None
|
||||
assert args == ['git', 'pull', '--rebase', '--autostash']
|
||||
assert args == ['git', 'pull', '--rebase']
|
||||
assert note == ''
|
||||
|
||||
|
||||
@@ -200,37 +200,3 @@ def test_stash_option_lets_the_switch_through_and_keeps_the_work(repos):
|
||||
assert _git('branch', '--show-current', cwd=repos).stdout.strip() == 'other'
|
||||
# The edit is not lost — it is on the stash.
|
||||
assert 'switch to other' in _git('stash', 'list', cwd=repos).stdout
|
||||
|
||||
|
||||
class TestInstallerDoesNotBlockTheUpdateButton:
|
||||
"""first_time_install.sh chmods scripts that git tracked as 644.
|
||||
|
||||
With core.fileMode true -- the default on Linux -- that leaves five
|
||||
permanently modified tracked files on every machine that ran the
|
||||
installer, and `git pull --rebase` refuses to start:
|
||||
|
||||
error: cannot pull with rebase: You have unstaged changes.
|
||||
|
||||
Tracking them as executable makes the installer's chmod a no-op.
|
||||
"""
|
||||
|
||||
CHMODDED = [
|
||||
'first_time_install.sh',
|
||||
'start_display.sh',
|
||||
'stop_display.sh',
|
||||
'scripts/install/install_service.sh',
|
||||
'scripts/install/install_web_service.sh',
|
||||
]
|
||||
|
||||
def test_scripts_the_installer_chmods_are_tracked_executable(self):
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
root = Path(__file__).resolve().parent.parent
|
||||
out = subprocess.run(['git', 'ls-files', '-s', *self.CHMODDED],
|
||||
capture_output=True, text=True, cwd=str(root)).stdout
|
||||
modes = {line.split()[3]: line.split()[0] for line in out.strip().split('\n') if line}
|
||||
non_exec = sorted(f for f, m in modes.items() if m != '100755')
|
||||
assert not non_exec, (
|
||||
f"{non_exec} are chmodded by the installer but tracked non-executable, "
|
||||
"so every install leaves the working tree dirty and the update "
|
||||
"button cannot pull")
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
"""A healthy plugin must not rewrite its health record every cycle.
|
||||
|
||||
Every successful plugin update called record_success(), which persisted the
|
||||
record unconditionally. In steady state the only fields that had changed were
|
||||
total_successes and last_success_time -- a counter and a timestamp that
|
||||
health_monitor reads for display and that nothing reads back after a restart.
|
||||
|
||||
Measured on a rig running 24 plugins: about 17 health-file rewrites a minute,
|
||||
roughly 25,000 a day. Each is ~400 bytes, but they land on an SD card where
|
||||
the unit of cost is an erase-block cycle, not the byte count, and where wear is
|
||||
what eventually kills the card.
|
||||
|
||||
The circuit breaker still needs its own state to survive a restart, so the
|
||||
write is kept for exactly the fields it is rebuilt from -- and a failure, a
|
||||
circuit opening, or a recovery must still be written the moment it happens.
|
||||
"""
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
from src.plugin_system.plugin_health import PluginHealthTracker, CircuitState
|
||||
|
||||
|
||||
class _Cache:
|
||||
"""Counts writes; serves back whatever was last written."""
|
||||
|
||||
def __init__(self):
|
||||
self.store = {}
|
||||
self.writes = 0
|
||||
|
||||
def set(self, key, data, ttl=None, **kwargs):
|
||||
self.writes += 1
|
||||
self.store[key] = data
|
||||
|
||||
def get(self, key, max_age=None, memory_ttl=None, **kwargs):
|
||||
return self.store.get(key)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def tracker():
|
||||
cache = _Cache()
|
||||
t = PluginHealthTracker(cache_manager=cache)
|
||||
return t, cache
|
||||
|
||||
|
||||
def test_steady_state_success_stops_writing(tracker):
|
||||
"""The regression: 100 healthy cycles used to be 100 SD writes."""
|
||||
t, cache = tracker
|
||||
t.record_success("weather")
|
||||
first = cache.writes
|
||||
for _ in range(100):
|
||||
t.record_success("weather")
|
||||
assert cache.writes == first, (
|
||||
f"{cache.writes - first} redundant writes across 100 healthy cycles"
|
||||
)
|
||||
|
||||
|
||||
def test_the_counters_are_still_accurate_in_memory(tracker):
|
||||
"""Skipping the write must not skip the bookkeeping."""
|
||||
t, _ = tracker
|
||||
for _ in range(10):
|
||||
t.record_success("weather")
|
||||
state = t.get_health_state("weather")
|
||||
assert state["total_successes"] == 10
|
||||
assert state["last_success_time"] is not None
|
||||
assert state["last_success_time"] <= time.time()
|
||||
|
||||
|
||||
def test_a_failure_is_written_immediately(tracker):
|
||||
t, cache = tracker
|
||||
t.record_success("weather")
|
||||
before = cache.writes
|
||||
t.record_failure("weather", RuntimeError("boom"))
|
||||
assert cache.writes > before, "a failure must reach disk"
|
||||
|
||||
|
||||
def test_recovery_after_failure_is_written(tracker):
|
||||
"""consecutive_failures returning to 0 is durable state changing."""
|
||||
t, cache = tracker
|
||||
t.record_failure("weather", RuntimeError("boom"))
|
||||
before = cache.writes
|
||||
t.record_success("weather")
|
||||
assert cache.writes > before, "recovery must reach disk"
|
||||
assert t.get_health_state("weather")["consecutive_failures"] == 0
|
||||
|
||||
|
||||
def test_a_closing_circuit_is_written(tracker):
|
||||
"""Success in half-open closes the circuit -- that must survive a restart."""
|
||||
t, cache = tracker
|
||||
state = t.get_health_state("weather")
|
||||
state["circuit_state"] = CircuitState.HALF_OPEN.value
|
||||
state["half_open_start_time"] = time.time()
|
||||
before = cache.writes
|
||||
t.record_success("weather")
|
||||
assert cache.writes > before, "a circuit transition must reach disk"
|
||||
assert t.get_health_state("weather")["circuit_state"] == CircuitState.CLOSED.value
|
||||
|
||||
|
||||
def test_durable_state_survives_a_restart(tracker):
|
||||
"""What is skipped must genuinely not matter to the breaker."""
|
||||
t, cache = tracker
|
||||
for _ in range(3):
|
||||
t.record_failure("weather", RuntimeError("boom"))
|
||||
for _ in range(50):
|
||||
t.record_success("weather")
|
||||
|
||||
revived = PluginHealthTracker(cache_manager=cache)
|
||||
state = revived.get_health_state("weather")
|
||||
assert state["consecutive_failures"] == 0
|
||||
assert state["circuit_state"] == CircuitState.CLOSED.value
|
||||
@@ -0,0 +1,101 @@
|
||||
"""Log lines must reach the journal with their real severity.
|
||||
|
||||
Everything this process writes to stdout lands in the journal as PRIORITY=6,
|
||||
whatever the Python level was, because journald has no other signal. Measured
|
||||
on a live rig over 24 hours: 55 lines containing " - ERROR - " and 13
|
||||
containing " - WARNING - ", every one of them recorded as informational. So
|
||||
|
||||
journalctl -p err -u ledmatrix
|
||||
|
||||
returned nothing while errors were being logged, and anyone triaging has to
|
||||
grep the message text instead. That is slower and it is wrong: a search for
|
||||
"oom" also matches the radar logging "zoom=9", which is exactly the false
|
||||
positive it produced during this audit.
|
||||
|
||||
systemd reads a leading "<N>" on each stdout line and uses it as the priority
|
||||
(sd-daemon(3)), so this needs no extra dependency -- and it must only be
|
||||
applied when systemd is actually reading, or the prefixes become literal noise
|
||||
in a terminal, the emulator, and test output.
|
||||
"""
|
||||
import logging
|
||||
import os
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from src.logging_config import JournalPriorityFormatter, _SYSLOG_PRIORITY, _under_systemd
|
||||
|
||||
|
||||
class _Plain(logging.Formatter):
|
||||
def format(self, record):
|
||||
return record.getMessage()
|
||||
|
||||
|
||||
def _record(level, msg="hello"):
|
||||
return logging.LogRecord("t", level, "f.py", 1, msg, None, None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("level,expected", [
|
||||
(logging.CRITICAL, 2),
|
||||
(logging.ERROR, 3),
|
||||
(logging.WARNING, 4),
|
||||
(logging.INFO, 6),
|
||||
(logging.DEBUG, 7),
|
||||
])
|
||||
def test_each_level_maps_to_its_syslog_priority(level, expected):
|
||||
out = JournalPriorityFormatter(_Plain()).format(_record(level))
|
||||
assert out.startswith(f"<{expected}>"), out
|
||||
assert _SYSLOG_PRIORITY[level] == expected
|
||||
|
||||
|
||||
def test_error_and_info_are_distinguishable():
|
||||
"""The whole point: journalctl -p err must be able to tell them apart."""
|
||||
fmt = JournalPriorityFormatter(_Plain())
|
||||
assert fmt.format(_record(logging.ERROR))[:3] != fmt.format(_record(logging.INFO))[:3]
|
||||
|
||||
|
||||
def test_every_line_of_a_multiline_record_is_tagged():
|
||||
"""The journal splits them, and an untagged continuation loses its level.
|
||||
|
||||
A traceback is the case that matters -- it is the most important thing in
|
||||
the log and the longest.
|
||||
"""
|
||||
out = JournalPriorityFormatter(_Plain()).format(
|
||||
_record(logging.ERROR, "Traceback:\nline one\nline two"))
|
||||
lines = out.split("\n")
|
||||
assert len(lines) == 3
|
||||
assert all(line.startswith("<3>") for line in lines), lines
|
||||
|
||||
|
||||
def test_the_message_survives_intact():
|
||||
out = JournalPriorityFormatter(_Plain()).format(_record(logging.WARNING, "disk full"))
|
||||
assert out == "<4>disk full"
|
||||
|
||||
|
||||
def test_an_unknown_level_falls_back_to_info():
|
||||
out = JournalPriorityFormatter(_Plain()).format(_record(25))
|
||||
assert out.startswith("<6>")
|
||||
|
||||
|
||||
def test_prefixing_is_off_outside_systemd():
|
||||
"""Otherwise a terminal run, the emulator and pytest all show `<6>`."""
|
||||
with patch.dict(os.environ, {}, clear=True):
|
||||
assert not _under_systemd()
|
||||
with patch.dict(os.environ, {"JOURNAL_STREAM": "8:12345"}):
|
||||
assert _under_systemd()
|
||||
|
||||
|
||||
def test_setup_uses_the_wrapper_only_under_systemd():
|
||||
from src.logging_config import setup_logging
|
||||
|
||||
for env, expect_wrapped in (({}, False), ({"JOURNAL_STREAM": "8:1"}, True)):
|
||||
with patch.dict(os.environ, env, clear=True):
|
||||
setup_logging()
|
||||
handlers = [h for h in logging.getLogger().handlers
|
||||
if isinstance(h, logging.StreamHandler)]
|
||||
assert handlers, "no stream handler installed"
|
||||
wrapped = any(isinstance(h.formatter, JournalPriorityFormatter)
|
||||
for h in handlers)
|
||||
assert wrapped is expect_wrapped, (
|
||||
f"JOURNAL_STREAM={env}: wrapped={wrapped}, expected {expect_wrapped}")
|
||||
logging.getLogger().handlers.clear()
|
||||
@@ -183,15 +183,27 @@ class TestSetupLogging:
|
||||
setup_logging()
|
||||
assert len(logging.getLogger().handlers) == 1
|
||||
|
||||
@staticmethod
|
||||
def _selected_formatter():
|
||||
"""The formatter setup_logging() chose, past any journald wrapper.
|
||||
|
||||
Under systemd the console handler's formatter is wrapped so each line
|
||||
carries its syslog priority. That wrapper is applied only when
|
||||
JOURNAL_STREAM is set, which is true in CI and false in a terminal, so
|
||||
asserting on the handler's formatter directly passes locally and fails
|
||||
on the runner. These tests are about which formatter format_type
|
||||
selects, so they look through the wrapper.
|
||||
"""
|
||||
formatter = logging.getLogger().handlers[0].formatter
|
||||
return getattr(formatter, "inner", formatter)
|
||||
|
||||
def test_json_format_selects_structured_formatter(self):
|
||||
setup_logging(format_type="json")
|
||||
assert isinstance(
|
||||
logging.getLogger().handlers[0].formatter, StructuredFormatter)
|
||||
assert isinstance(self._selected_formatter(), StructuredFormatter)
|
||||
|
||||
def test_readable_format_selects_contextual_formatter(self):
|
||||
setup_logging(format_type="readable")
|
||||
assert isinstance(
|
||||
logging.getLogger().handlers[0].formatter, ContextualFormatter)
|
||||
assert isinstance(self._selected_formatter(), ContextualFormatter)
|
||||
|
||||
def test_log_file_adds_file_handler(self, tmp_path):
|
||||
log_file = tmp_path / "test.log"
|
||||
|
||||
@@ -1,85 +0,0 @@
|
||||
"""A check that could not run must not be reported as "up to date".
|
||||
|
||||
check-update returned update_available=False whenever git failed. The banner
|
||||
is the only route to the update button, so a checkout git refuses to touch
|
||||
looked exactly like a current one -- permanently, and with nothing for the
|
||||
user to act on. The usual cause is an install performed as root, after which
|
||||
every git command fails with "detected dubious ownership".
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod # noqa: E402
|
||||
from web_interface.blueprints.api_v3 import api_v3 # noqa: E402
|
||||
|
||||
DUBIOUS = ("fatal: detected dubious ownership in repository at "
|
||||
"'/home/pi/LEDMatrix'\nTo add an exception for this directory, call:\n"
|
||||
"\tgit config --global --add safe.directory /home/pi/LEDMatrix\n")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
app = Flask(__name__)
|
||||
app.config['TESTING'] = True
|
||||
app.register_blueprint(api_v3, url_prefix='/api/v3')
|
||||
mod._update_check_cache['result'] = None
|
||||
mod._update_check_cache['ts'] = 0
|
||||
return app.test_client()
|
||||
|
||||
|
||||
def _fetch_fails(stderr: bytes):
|
||||
def fake_run(args, **kwargs):
|
||||
if args[:2] == ['git', 'fetch']:
|
||||
return subprocess.CompletedProcess(args, 1, stdout=b'', stderr=stderr)
|
||||
return subprocess.CompletedProcess(args, 0, stdout='', stderr='')
|
||||
return fake_run
|
||||
|
||||
|
||||
class TestFailedCheckIsNotSilence:
|
||||
def test_dubious_ownership_is_reported_not_swallowed(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['check_failed'] is True, (
|
||||
"a git failure was reported as a successful 'no update' check")
|
||||
assert data['update_available'] is False
|
||||
|
||||
def test_the_message_tells_the_user_what_to_do(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert 'chown' in data['error'], (
|
||||
"dubious ownership is unactionable without the fix command")
|
||||
assert 'root' in data['error']
|
||||
|
||||
def test_an_ordinary_git_failure_still_surfaces(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_fetch_fails(b'fatal: some other git problem\n')):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['check_failed'] is True
|
||||
assert 'some other git problem' in data['error']
|
||||
|
||||
def test_offline_reads_as_offline(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_fetch_fails(b'fatal: could not resolve host: github.com\n')):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert 'Could not reach GitHub' in data['error']
|
||||
|
||||
|
||||
class TestSuccessPathUnchanged:
|
||||
def test_up_to_date_carries_no_failure_flag(self, client):
|
||||
def fake_run(args, **kwargs):
|
||||
if args[:2] == ['git', 'fetch']:
|
||||
return subprocess.CompletedProcess(args, 0, stdout=b'', stderr=b'')
|
||||
if args[:2] == ['git', 'rev-parse']:
|
||||
return subprocess.CompletedProcess(args, 0, stdout='abc123\n', stderr='')
|
||||
return subprocess.CompletedProcess(args, 0, stdout='0\n', stderr='')
|
||||
with patch.object(mod.subprocess, 'run', fake_run):
|
||||
data = client.get('/api/v3/system/check-update').get_json()
|
||||
assert data['update_available'] is False
|
||||
assert not data.get('check_failed'), "a healthy check must not look like a failure"
|
||||
@@ -1,84 +0,0 @@
|
||||
"""A pull that changed nothing on the running system is not an applied update.
|
||||
|
||||
git_pull replaces files on disk and restarts nothing -- there is no systemctl
|
||||
call anywhere in the handler. The display and web services keep running the
|
||||
code they loaded at boot, so the user is told "Code updated successfully" and
|
||||
sees no change until they happen to reboot. The response now says whether a
|
||||
restart is owed, and the UI raises the existing restart-pending banner.
|
||||
"""
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from web_interface.blueprints import api_v3 as mod # noqa: E402
|
||||
from web_interface.blueprints.api_v3 import api_v3 # noqa: E402
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client():
|
||||
app = Flask(__name__)
|
||||
app.config['TESTING'] = True
|
||||
app.register_blueprint(api_v3, url_prefix='/api/v3')
|
||||
# The handler consults these after a successful pull; None is the
|
||||
# "not wired up" case it already guards for.
|
||||
api_v3.plugin_store_manager = None
|
||||
api_v3.config_manager = None
|
||||
return app.test_client()
|
||||
|
||||
|
||||
def _git(heads, pull_rc=0, pull_out='Updating a1b2c3..d4e5f6\n'):
|
||||
"""Fake git. `heads` are the successive answers to rev-parse HEAD."""
|
||||
seq = list(heads)
|
||||
|
||||
def run(args, **kwargs):
|
||||
def ok(stdout='', rc=0, b=False):
|
||||
return subprocess.CompletedProcess(
|
||||
args, rc, stdout=(stdout.encode() if b else stdout),
|
||||
stderr=(b'' if b else ''))
|
||||
if args[:2] == ['git', 'rev-parse'] and args[-1] == 'HEAD':
|
||||
return ok(seq.pop(0) + '\n' if seq else 'deadbeef\n')
|
||||
if 'symbolic-full-name' in args or '@{u}' in args:
|
||||
return ok('origin/main\n')
|
||||
if args[:2] == ['git', 'status']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'diff']:
|
||||
return ok('')
|
||||
if args[:2] == ['git', 'pull']:
|
||||
return ok(pull_out, pull_rc)
|
||||
return ok('')
|
||||
return run
|
||||
|
||||
|
||||
def _pull(client):
|
||||
return client.post('/api/v3/system/action',
|
||||
json={'action': 'git_pull'}).get_json()
|
||||
|
||||
|
||||
class TestRestartIsRequestedWhenCodeChanged:
|
||||
def test_a_pull_that_moved_head_asks_for_a_restart(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is True, (
|
||||
"new code on disk, services still running the old code, and "
|
||||
"nothing told the user to restart")
|
||||
|
||||
def test_already_up_to_date_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run',
|
||||
_git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'success'
|
||||
assert data['restart_required'] is False, (
|
||||
"prompting after a no-op update trains users to ignore the prompt")
|
||||
|
||||
def test_a_failed_pull_does_not(self, client):
|
||||
with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
|
||||
data = _pull(client)
|
||||
assert data['status'] == 'error'
|
||||
assert data['restart_required'] is False
|
||||
@@ -0,0 +1,63 @@
|
||||
"""The Vegas content path must trace at DEBUG, not INFO.
|
||||
|
||||
plugin_adapter narrates every step of acquiring content from every plugin --
|
||||
"Has get_vegas_content", "Native: calling get_vegas_content()", "Native content
|
||||
returned None", "Has scroll_helper", the per-item sizes -- and it does that for
|
||||
each plugin on each cycle.
|
||||
|
||||
Measured on a live rig: 13,408 log lines an hour, of which 13,366 were INFO and
|
||||
35 were WARNING. plugin_adapter alone produced 2,457 of them. That is ~223
|
||||
lines a minute of string formatting on a Pi that is also driving the panel, all
|
||||
of it written through journald to the SD card, and it buries the 35 lines that
|
||||
actually indicate a problem.
|
||||
|
||||
Nothing is lost by moving it to DEBUG: the 19 warning/error/exception calls in
|
||||
the module are untouched, so real failures still surface at their own level.
|
||||
|
||||
One INFO call is deliberate and stays -- the padding-strip message chooses its
|
||||
level at runtime (`logger.warning if (left and right) else logger.info`) and
|
||||
test_vegas_plugin_adapter.py pins it.
|
||||
"""
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
ADAPTER = (Path(__file__).resolve().parent.parent / "src" / "vegas_mode"
|
||||
/ "plugin_adapter.py")
|
||||
|
||||
|
||||
def _info_calls(path):
|
||||
"""Direct logger.info(...) call sites in a module."""
|
||||
tree = ast.parse(path.read_text(encoding="utf-8"))
|
||||
found = []
|
||||
for node in ast.walk(tree):
|
||||
if (isinstance(node, ast.Call)
|
||||
and isinstance(node.func, ast.Attribute)
|
||||
and node.func.attr == "info"
|
||||
and getattr(node.func.value, "id", None) == "logger"):
|
||||
found.append(node.lineno)
|
||||
return found
|
||||
|
||||
|
||||
def test_the_content_path_does_not_trace_at_info():
|
||||
calls = _info_calls(ADAPTER)
|
||||
assert not calls, (
|
||||
"plugin_adapter should trace at DEBUG; found logger.info at lines "
|
||||
f"{calls}. This path runs per plugin per cycle and its output goes to "
|
||||
"the SD card via journald."
|
||||
)
|
||||
|
||||
|
||||
def test_real_failures_still_have_a_level_of_their_own():
|
||||
"""Demoting the trace must not have swept up the error reporting."""
|
||||
source = ADAPTER.read_text(encoding="utf-8")
|
||||
loud = sum(source.count(f"logger.{level}(")
|
||||
for level in ("warning", "error", "exception"))
|
||||
assert loud >= 15, f"only {loud} warning/error/exception calls remain"
|
||||
|
||||
|
||||
def test_the_deliberate_runtime_chosen_level_survives():
|
||||
"""The padding-strip message picks its level at runtime; leave it alone."""
|
||||
source = ADAPTER.read_text(encoding="utf-8")
|
||||
assert "logger.warning if (left and right) else logger.info" in source
|
||||
@@ -194,46 +194,15 @@ class TestSavePluginConfig:
|
||||
|
||||
def test_secret_count_message_counts_top_level_keys(self, env):
|
||||
# Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
|
||||
# the separated secrets dict. Here that is 1: the posted accounts
|
||||
# array, whose item tokens all count as ONE key.
|
||||
#
|
||||
# It was 2 before blank secrets were dropped, the second being the
|
||||
# schema's api_key default (""), which merge_with_defaults adds to
|
||||
# every save. Counting it was the visible edge of a real bug: that
|
||||
# injected blank was merged over the stored api_key, so saving any
|
||||
# unrelated field destroyed the credential. See
|
||||
# test_an_unrelated_edit_does_not_erase_a_stored_secret.
|
||||
# the separated secrets dict. Here that is 2: the posted accounts
|
||||
# array (all its item tokens count as ONE key) plus the schema's
|
||||
# api_key default ("") that merge_with_defaults adds before
|
||||
# separation.
|
||||
resp = self._save(env, {
|
||||
"accounts": [{"name": "a", "token": "t"}],
|
||||
})
|
||||
message = resp.get_json()["message"]
|
||||
assert "(1 secret field(s) saved to config_secrets.json)" in message
|
||||
|
||||
def test_an_unrelated_edit_does_not_erase_a_stored_secret(self, env):
|
||||
"""Editing one field must not wipe the plugin's API key.
|
||||
|
||||
The config form renders secrets masked, so the browser posts them
|
||||
back blank; merge_with_defaults injects a blank api_key even when
|
||||
the client omits it entirely. Either way a "" reached the secrets
|
||||
file and deep_merge wrote it over the stored credential.
|
||||
"""
|
||||
assert self._save(env, {"api_key": "REAL-KEY-0123456789",
|
||||
"city": "Austin"}).status_code == 200
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
||||
"REAL-KEY-0123456789"
|
||||
|
||||
# the user changes the city; the masked api_key rides along blank
|
||||
assert self._save(env, {"api_key": "", "city": "Dallas"}).status_code == 200
|
||||
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
|
||||
"REAL-KEY-0123456789", "an unrelated edit destroyed the API key"
|
||||
assert env.fresh_load()[PLUGIN_ID]["city"] == "Dallas"
|
||||
|
||||
def test_a_secret_can_still_be_changed(self, env):
|
||||
"""Dropping blanks must not stop a real new value from being saved."""
|
||||
self._save(env, {"api_key": "first-key"})
|
||||
self._save(env, {"api_key": "second-key"})
|
||||
assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == "second-key"
|
||||
assert "(2 secret field(s) saved to config_secrets.json)" in message
|
||||
|
||||
def test_resave_replaces_stored_secrets_list_wholesale(self, env):
|
||||
# Characterized: api_v3's deep_merge intentionally replaces lists,
|
||||
|
||||
@@ -1,82 +0,0 @@
|
||||
"""GET /config/secrets must not hand out credentials, and the client's
|
||||
read-modify-write cycle must not destroy them.
|
||||
|
||||
This interface has no authentication. The endpoint returned the whole
|
||||
config_secrets.json to anyone who could reach the port; on one rig that was a
|
||||
40-character GitHub token, a 183-character Home Assistant token and three API
|
||||
keys. Masking it alone is not enough: the only client fetches every secret,
|
||||
edits one field and posts all of them back, so the write path has to treat an
|
||||
echoed mask as "unchanged".
|
||||
"""
|
||||
import json
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).parent))
|
||||
|
||||
from test_api_v3_secret_roundtrip import env, _on_disk # noqa: F401,E402
|
||||
from src.web_interface.secret_helpers import SECRET_MASK # noqa: E402
|
||||
|
||||
STORED = {
|
||||
"github": {"api_token": "ghp_" + "x" * 36},
|
||||
"ledmatrix-weather": {"api_key": "w" * 32},
|
||||
"incoming-packages": {"ha_token": "h" * 183},
|
||||
"unset-plugin": {"api_key": ""},
|
||||
"placeholder-plugin": {"api_key": "YOUR_API_KEY_HERE"},
|
||||
}
|
||||
|
||||
|
||||
def _seed(env):
|
||||
env.secrets_file.write_text(json.dumps(STORED))
|
||||
|
||||
|
||||
def _get(env):
|
||||
r = env.client.get("/api/v3/config/secrets")
|
||||
assert r.status_code == 200, r.get_data(as_text=True)[:200]
|
||||
return r.get_json()["data"]
|
||||
|
||||
|
||||
def test_no_credential_leaves_the_process(env):
|
||||
_seed(env)
|
||||
body = json.dumps(_get(env))
|
||||
for secret in ("ghp_" + "x" * 36, "w" * 32, "h" * 183):
|
||||
assert secret not in body, "endpoint returned a stored credential"
|
||||
|
||||
|
||||
def test_set_and_unset_remain_distinguishable(env):
|
||||
_seed(env)
|
||||
data = _get(env)
|
||||
assert data["github"]["api_token"] == SECRET_MASK
|
||||
assert data["unset-plugin"]["api_key"] == ""
|
||||
assert data["placeholder-plugin"]["api_key"] == "YOUR_API_KEY_HERE"
|
||||
|
||||
|
||||
def test_the_clients_read_modify_write_preserves_every_other_secret(env):
|
||||
"""What the GitHub-token save button actually does."""
|
||||
_seed(env)
|
||||
secrets = _get(env) # everything arrives masked
|
||||
secrets["github"]["api_token"] = "ghp_" + "n" * 36 # user changes one
|
||||
r = env.client.post("/api/v3/config/raw/secrets", json=secrets)
|
||||
assert r.status_code == 200, r.get_data(as_text=True)[:200]
|
||||
|
||||
on_disk = _on_disk(env.secrets_file)
|
||||
assert on_disk["github"]["api_token"] == "ghp_" + "n" * 36, "new token not saved"
|
||||
assert on_disk["ledmatrix-weather"]["api_key"] == "w" * 32
|
||||
assert on_disk["incoming-packages"]["ha_token"] == "h" * 183
|
||||
|
||||
|
||||
def test_a_mask_echoed_back_is_never_stored(env):
|
||||
_seed(env)
|
||||
env.client.post("/api/v3/config/raw/secrets", json=_get(env))
|
||||
on_disk = _on_disk(env.secrets_file)
|
||||
assert SECRET_MASK not in json.dumps(on_disk), "the mask was stored as a secret"
|
||||
assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36
|
||||
|
||||
|
||||
def test_a_brand_new_secret_can_still_be_added(env):
|
||||
_seed(env)
|
||||
env.client.post("/api/v3/config/raw/secrets",
|
||||
json={"new-plugin": {"api_key": "brand-new"}})
|
||||
on_disk = _on_disk(env.secrets_file)
|
||||
assert on_disk["new-plugin"]["api_key"] == "brand-new"
|
||||
assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36
|
||||
@@ -21,9 +21,7 @@ logger = logging.getLogger(__name__)
|
||||
# Import new infrastructure
|
||||
from src.web_interface.api_helpers import success_response, error_response, validate_request_json
|
||||
from src.web_interface.errors import ErrorCode
|
||||
from src.web_interface.secret_helpers import (find_secret_fields, mask_all_secret_values,
|
||||
remove_empty_secrets, separate_secrets,
|
||||
strip_masked_values)
|
||||
from src.web_interface.secret_helpers import find_secret_fields, separate_secrets
|
||||
from src.web_interface.error_handler import describe_exception, redact_text
|
||||
from src.plugin_system.operation_types import OperationType
|
||||
from src.web_interface.validators import (
|
||||
@@ -264,54 +262,15 @@ def _stop_display_service():
|
||||
result['status'] = status
|
||||
return result
|
||||
|
||||
#: Field names whose value is a credential. Matched by name because this
|
||||
#: endpoint returns the whole config, core keys included, and core config has
|
||||
#: no schema to carry x-secret markers.
|
||||
_CREDENTIAL_NAME_PARTS = ("password", "passwd", "secret", "token", "api_key",
|
||||
"apikey", "access_key", "private_key", "client_secret")
|
||||
|
||||
|
||||
def _looks_like_a_credential(name: str) -> bool:
|
||||
lowered = name.lower()
|
||||
return any(part in lowered for part in _CREDENTIAL_NAME_PARTS)
|
||||
|
||||
|
||||
def _redact_credentials(value):
|
||||
"""A copy of `value` with credential-named fields blanked.
|
||||
|
||||
/config/main returned the raw config to anyone who could reach the port,
|
||||
and this interface has no authentication. On one rig that meant a 40-char
|
||||
GitHub token, a 183-char Home Assistant token and five API keys were
|
||||
readable by anything on the LAN.
|
||||
|
||||
The x-secret masking used by the plugin config endpoints does not help
|
||||
here: this endpoint never consults a schema, and core keys such as
|
||||
github.api_token have no schema to mark. Matching on the field name is
|
||||
blunt, but for a whole-config dump the right default is that anything
|
||||
named like a credential does not leave the process.
|
||||
|
||||
Blanked rather than removed, and safe to blank: POST /config/main merges
|
||||
into the loaded config and only writes the keys it was given, so a client
|
||||
that round-trips this response cannot erase a secret it never saw.
|
||||
"""
|
||||
if isinstance(value, dict):
|
||||
return {k: ("" if _looks_like_a_credential(k) and not isinstance(v, (dict, list))
|
||||
else _redact_credentials(v))
|
||||
for k, v in value.items()}
|
||||
if isinstance(value, list):
|
||||
return [_redact_credentials(item) for item in value]
|
||||
return value
|
||||
|
||||
|
||||
@api_v3.route('/config/main', methods=['GET'])
|
||||
def get_main_config():
|
||||
"""Get main configuration, with credentials redacted."""
|
||||
"""Get main configuration"""
|
||||
try:
|
||||
if not api_v3.config_manager:
|
||||
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
|
||||
|
||||
config = api_v3.config_manager.load_config()
|
||||
return jsonify({'status': 'success', 'data': _redact_credentials(config)})
|
||||
return jsonify({'status': 'success', 'data': config})
|
||||
except Exception as e:
|
||||
logger.error('Unhandled exception', exc_info=True)
|
||||
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
|
||||
@@ -756,12 +715,10 @@ def save_main_config():
|
||||
if not data:
|
||||
return jsonify({'status': 'error', 'message': 'No data provided'}), 400
|
||||
|
||||
# What arrives here is the config itself, and the headers carry the
|
||||
# session cookie -- neither belongs in the journal, least of all at
|
||||
# ERROR on every save. The shape of the request is the part with
|
||||
# diagnostic value, so log that, at the level it deserves.
|
||||
logger.debug("save_main_config: %s, %d top-level key(s)",
|
||||
request.content_type or 'no content-type', len(data))
|
||||
import logging
|
||||
logging.error(f"DEBUG: save_main_config received data: {data}")
|
||||
logging.error(f"DEBUG: Content-Type header: {request.content_type}")
|
||||
logging.error(f"DEBUG: Headers: {dict(request.headers)}")
|
||||
|
||||
# Merge with existing config (similar to original implementation)
|
||||
current_config = api_v3.config_manager.load_config()
|
||||
@@ -1259,11 +1216,6 @@ def save_main_config():
|
||||
|
||||
# Separate secrets from regular config (same logic as save_plugin_config)
|
||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||
# The config form renders secrets masked, so every save posts
|
||||
# them back blank. Without this the blank is merged over the
|
||||
# stored value and the credential is destroyed by the act of
|
||||
# changing an unrelated setting. A blank means "unchanged".
|
||||
secrets_config = remove_empty_secrets(secrets_config)
|
||||
|
||||
# PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
|
||||
# This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
|
||||
@@ -1381,12 +1333,7 @@ def get_secrets_config():
|
||||
return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
|
||||
|
||||
config = api_v3.config_manager.get_raw_file_content('secrets')
|
||||
# This interface has no authentication, and this file is nothing but
|
||||
# credentials. It was handing all of them to anyone who could reach
|
||||
# the port. Values are masked; empty and YOUR_* placeholders are left
|
||||
# alone so a client can still tell "set" from "not set".
|
||||
return jsonify({'status': 'success',
|
||||
'data': mask_all_secret_values(config)})
|
||||
return jsonify({'status': 'success', 'data': config})
|
||||
except Exception as e:
|
||||
logger.error('Unhandled exception', exc_info=True)
|
||||
return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
|
||||
@@ -1448,19 +1395,8 @@ def save_raw_secrets_config():
|
||||
if not data:
|
||||
return jsonify({'status': 'error', 'message': 'No data provided'}), 400
|
||||
|
||||
# The GET above masks what it returns, and this endpoint's only client
|
||||
# reads the whole file, edits one field and posts all of it back. So
|
||||
# most of what arrives here is the mask, echoed rather than changed --
|
||||
# storing it verbatim would replace every untouched credential with
|
||||
# eight bullets. Strip those, then merge onto what is already stored,
|
||||
# which makes "unchanged" mean unchanged.
|
||||
#
|
||||
# The cost is that a secret can no longer be cleared by blanking it.
|
||||
# That needs its own affordance; a control that erases credentials as
|
||||
# a side effect of saving an unrelated one is not it.
|
||||
current = api_v3.config_manager.get_raw_file_content('secrets') or {}
|
||||
merged = deep_merge(current, strip_masked_values(data))
|
||||
api_v3.config_manager.save_raw_file_content('secrets', merged)
|
||||
# Save the secrets config
|
||||
api_v3.config_manager.save_raw_file_content('secrets', data)
|
||||
|
||||
# Reload GitHub token in plugin store manager if it exists
|
||||
if api_v3.plugin_store_manager:
|
||||
@@ -1721,22 +1657,13 @@ def resolve_pull_command(project_dir):
|
||||
backup, or following an install guide that names one. The update button
|
||||
then reports a failure the user cannot act on.
|
||||
|
||||
``--autostash`` is passed for the same reason. Rebase refuses to start
|
||||
when any tracked file is modified, and on these installs something always
|
||||
is: first_time_install.sh chmods five scripts that git tracked as 644, so
|
||||
every machine that ran the installer carries five permanent mode changes
|
||||
and the update button reports "cannot pull with rebase: You have unstaged
|
||||
changes". Those modes are corrected in this commit, but a user cannot pull
|
||||
the correction while the pull is what is blocked, and any other local edit
|
||||
would reproduce it anyway. Autostash reapplies the changes afterwards.
|
||||
|
||||
Returns ``(args, note, error)``. When ``origin/<branch>`` exists the pull
|
||||
is made explicit against it, so the update proceeds and the branch is
|
||||
given tracking information afterwards.
|
||||
"""
|
||||
upstream = _git_upstream(project_dir)
|
||||
if upstream:
|
||||
return ['git', 'pull', '--rebase', '--autostash'], '', None
|
||||
return ['git', 'pull', '--rebase'], '', None
|
||||
|
||||
branch = _git_current_branch(project_dir)
|
||||
if not branch:
|
||||
@@ -1746,7 +1673,7 @@ def resolve_pull_command(project_dir):
|
||||
)
|
||||
if _git_remote_branch_exists(project_dir, branch):
|
||||
return (
|
||||
['git', 'pull', '--rebase', '--autostash', 'origin', branch],
|
||||
['git', 'pull', '--rebase', 'origin', branch],
|
||||
f"Branch '{branch}' had no upstream; pulled from origin/{branch} and set it as the upstream.",
|
||||
None,
|
||||
)
|
||||
@@ -1894,33 +1821,6 @@ def get_system_version():
|
||||
_update_check_cache: Dict[str, Any] = {'result': None, 'ts': 0.0}
|
||||
_UPDATE_CHECK_TTL = 300 # 5 minutes — avoids a git fetch on every page load
|
||||
|
||||
def _update_check_failed(detail: str) -> Dict[str, Any]:
|
||||
"""A check that could not run is not the same as being up to date.
|
||||
|
||||
Reporting update_available=False on a git failure hides the banner, and
|
||||
the banner is the only route to the update button -- so a checkout git
|
||||
refuses to touch looks exactly like a current one, permanently. The most
|
||||
common cause is an install performed as root: git then reports "dubious
|
||||
ownership" and every command fails, including the fetch here.
|
||||
"""
|
||||
return {'update_available': False, 'remote_sha': 'unknown',
|
||||
'commits_behind': 0, 'check_failed': True, 'error': detail}
|
||||
|
||||
|
||||
def _describe_git_failure(stderr: str) -> str:
|
||||
"""Turn git's stderr into something the user can act on."""
|
||||
text = (stderr or '').strip()
|
||||
if 'dubious ownership' in text or 'detected dubious ownership' in text:
|
||||
return ("This checkout is owned by a different user than the one "
|
||||
"running the web interface, so git refuses to use it. It is "
|
||||
"usually the result of installing as root. Fix the ownership "
|
||||
"and the update will work: sudo chown -R $USER:$USER "
|
||||
+ str(PROJECT_ROOT))
|
||||
if 'could not resolve host' in text.lower() or 'network is unreachable' in text.lower():
|
||||
return "Could not reach GitHub to check for updates."
|
||||
return "Could not check for updates: " + (text.splitlines()[0] if text else "git failed")
|
||||
|
||||
|
||||
@api_v3.route('/system/check-update', methods=['GET'])
|
||||
def check_for_update():
|
||||
"""Check whether a newer LEDMatrix commit is available on origin/main."""
|
||||
@@ -1936,13 +1836,12 @@ def check_for_update():
|
||||
capture_output=True, timeout=10, cwd=cwd,
|
||||
)
|
||||
if fetch_result.returncode != 0:
|
||||
stderr = fetch_result.stderr.decode(errors='replace').strip()
|
||||
logger.warning("check-update: git fetch failed (rc=%d): %s",
|
||||
fetch_result.returncode, stderr)
|
||||
failed = _update_check_failed(_describe_git_failure(stderr))
|
||||
_update_check_cache['result'] = failed
|
||||
fetch_result.returncode,
|
||||
fetch_result.stderr.decode(errors='replace').strip())
|
||||
_update_check_cache['result'] = _safe
|
||||
_update_check_cache['ts'] = now
|
||||
return jsonify(failed)
|
||||
return jsonify(_safe)
|
||||
local = subprocess.run(
|
||||
['git', 'rev-parse', 'HEAD'],
|
||||
capture_output=True, text=True, timeout=5, cwd=cwd,
|
||||
@@ -1970,8 +1869,7 @@ def check_for_update():
|
||||
return jsonify(result)
|
||||
except Exception as e:
|
||||
logger.warning("check-update failed: %s", e)
|
||||
return jsonify(_update_check_failed(
|
||||
"Could not check for updates; see logs for details."))
|
||||
return jsonify(_safe)
|
||||
|
||||
@api_v3.route('/system/action', methods=['POST'])
|
||||
def execute_system_action():
|
||||
@@ -2098,11 +1996,6 @@ def execute_system_action():
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning("git rev-parse timed out before pull")
|
||||
|
||||
# Whether the pull actually brought new code in. "Already up to
|
||||
# date" is a success too, and prompting for a restart then would
|
||||
# train users to ignore the prompt.
|
||||
code_changed = False
|
||||
|
||||
# Perform the git pull. Branches without an upstream were given
|
||||
# an explicit "origin <branch>" above so the update still works.
|
||||
result = subprocess.run(
|
||||
@@ -2146,7 +2039,6 @@ def execute_system_action():
|
||||
capture_output=True, text=True, timeout=10, cwd=project_dir)
|
||||
new_head = _post.stdout.strip() if _post.returncode == 0 else None
|
||||
if old_head and new_head and old_head != new_head:
|
||||
code_changed = True
|
||||
diff = subprocess.run(
|
||||
['git', 'diff', '--name-only', f'{old_head}..{new_head}'],
|
||||
capture_output=True, text=True, timeout=15, cwd=project_dir)
|
||||
@@ -2206,14 +2098,9 @@ def execute_system_action():
|
||||
if ln.strip()), '')
|
||||
pull_message = f"Update failed: {detail}" if detail else "Update failed; check logs for details"
|
||||
|
||||
# Nothing here restarts anything: the pull replaces files on
|
||||
# disk while the display and web services keep running the code
|
||||
# they loaded at boot. Without this the user is told the update
|
||||
# succeeded and sees no change until they happen to reboot.
|
||||
return jsonify({
|
||||
'status': 'success' if result.returncode == 0 else 'error',
|
||||
'message': pull_message,
|
||||
'restart_required': bool(result.returncode == 0 and code_changed),
|
||||
})
|
||||
elif action == 'checkout_branch':
|
||||
# Switch branches from the Tools tab. Needed because a checkout
|
||||
@@ -5712,11 +5599,6 @@ def save_plugin_config():
|
||||
# Separate secrets from regular config (handles nested configs and
|
||||
# array-item secrets — see src/web_interface/secret_helpers.py)
|
||||
regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
|
||||
# The config form renders secrets masked, so every save posts
|
||||
# them back blank. Without this the blank is merged over the
|
||||
# stored value and the credential is destroyed by the act of
|
||||
# changing an unrelated setting. A blank means "unchanged".
|
||||
secrets_config = remove_empty_secrets(secrets_config)
|
||||
|
||||
# Get current configs
|
||||
current_config = api_v3.config_manager.load_config()
|
||||
|
||||
@@ -116,25 +116,14 @@ document.body.addEventListener('htmx:afterRequest', function(event) {
|
||||
// ===== Restart-pending banner =====
|
||||
// Shown after restart-requiring saves; persists across tab switches (and
|
||||
// reloads, via sessionStorage) until the display restarts or it's dismissed.
|
||||
window.showRestartPending = function(message) {
|
||||
try {
|
||||
sessionStorage.setItem('ledmatrix-restart-pending', '1');
|
||||
// Persisted alongside the flag: a code update and a config save want
|
||||
// different wording, and the banner outlives the page that raised it.
|
||||
if (message) sessionStorage.setItem('ledmatrix-restart-pending-text', message);
|
||||
else sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* private browsing */ }
|
||||
window.showRestartPending = function() {
|
||||
try { sessionStorage.setItem('ledmatrix-restart-pending', '1'); } catch { /* private browsing */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && message) text.textContent = message;
|
||||
if (banner) banner.style.display = 'block';
|
||||
};
|
||||
|
||||
window.dismissRestartPending = function() {
|
||||
try {
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending');
|
||||
sessionStorage.removeItem('ledmatrix-restart-pending-text');
|
||||
} catch { /* no-op */ }
|
||||
try { sessionStorage.removeItem('ledmatrix-restart-pending'); } catch { /* no-op */ }
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
if (banner) banner.style.display = 'none';
|
||||
};
|
||||
@@ -162,9 +151,6 @@ document.addEventListener('DOMContentLoaded', function() {
|
||||
try {
|
||||
if (sessionStorage.getItem('ledmatrix-restart-pending') === '1') {
|
||||
const banner = document.getElementById('restart-pending-banner');
|
||||
const saved = sessionStorage.getItem('ledmatrix-restart-pending-text');
|
||||
const text = document.getElementById('restart-pending-text');
|
||||
if (text && saved) text.textContent = saved;
|
||||
if (banner) banner.style.display = 'block';
|
||||
}
|
||||
} catch { /* no-op */ }
|
||||
|
||||
@@ -4622,17 +4622,15 @@ window.loadGithubToken = function() {
|
||||
// Handle empty data (secrets file doesn't exist) - API returns {} in this case
|
||||
const secrets = data.data || {};
|
||||
const token = secrets.github?.api_token || '';
|
||||
const configured = token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN';
|
||||
|
||||
if (input) {
|
||||
// The endpoint masks what it returns, so this never holds
|
||||
// the real token -- and the field is deliberately left
|
||||
// empty rather than filled with the mask, which would be
|
||||
// saved verbatim the next time the user pressed Save.
|
||||
input.value = '';
|
||||
if (configured) {
|
||||
showNotification('A GitHub token is saved. Enter a new one to replace it.', 'success');
|
||||
if (token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN') {
|
||||
// Token exists and is valid
|
||||
input.value = token;
|
||||
showNotification('GitHub token loaded successfully', 'success');
|
||||
} else {
|
||||
// No token configured or placeholder value
|
||||
input.value = '';
|
||||
showNotification('No GitHub token configured. Enter a new token to save.', 'info');
|
||||
}
|
||||
}
|
||||
|
||||
@@ -413,8 +413,7 @@
|
||||
<div class="flex items-center justify-between">
|
||||
<div class="flex items-center space-x-3">
|
||||
<i class="fas fa-rotate text-lg"></i>
|
||||
<span class="text-sm font-medium" aria-live="polite"
|
||||
id="restart-pending-text">
|
||||
<span class="text-sm font-medium" aria-live="polite">
|
||||
Configuration saved — restart the display to apply the changes
|
||||
</span>
|
||||
</div>
|
||||
@@ -1108,29 +1107,15 @@
|
||||
fetch('/api/v3/system/check-update')
|
||||
.then(function(r) { return r.json(); })
|
||||
.then(function(data) {
|
||||
var banner = document.getElementById('update-banner');
|
||||
var btn = document.getElementById('update-banner-btn');
|
||||
if (data.check_failed) {
|
||||
// A check that could not run is not the same as being up
|
||||
// to date. Hiding the banner here made a checkout git
|
||||
// refuses to touch look permanently current, with no
|
||||
// route to the update button and nothing to act on.
|
||||
document.getElementById('update-banner-text').textContent =
|
||||
data.error || 'Could not check for updates.';
|
||||
if (btn) btn.style.display = 'none';
|
||||
banner.style.display = '';
|
||||
return;
|
||||
}
|
||||
if (btn) btn.style.display = '';
|
||||
if (data.update_available && getDismissedSha() !== data.remote_sha) {
|
||||
var n = data.commits_behind || 0;
|
||||
var msg = 'A new LEDMatrix update is available';
|
||||
if (n > 0) msg += ' (' + n + ' commit' + (n > 1 ? 's' : '') + ')';
|
||||
document.getElementById('update-banner-text').textContent = msg;
|
||||
banner.style.display = '';
|
||||
document.getElementById('update-banner').style.display = '';
|
||||
try { sessionStorage.setItem('update-sha', data.remote_sha); } catch(e) {}
|
||||
} else {
|
||||
banner.style.display = 'none';
|
||||
document.getElementById('update-banner').style.display = 'none';
|
||||
}
|
||||
})
|
||||
.catch(function() {});
|
||||
@@ -1161,13 +1146,6 @@
|
||||
if (data.status === 'success') {
|
||||
document.getElementById('update-banner').style.display = 'none';
|
||||
try { sessionStorage.removeItem('update-sha-dismissed'); } catch(e) {}
|
||||
// The pull replaced files on disk; the running services still
|
||||
// hold the code they loaded at boot. Ask for the restart that
|
||||
// makes the update actually take effect.
|
||||
if (data.restart_required && typeof window.showRestartPending === 'function') {
|
||||
window.showRestartPending(
|
||||
'Update installed \u2014 restart the display to run the new code');
|
||||
}
|
||||
}
|
||||
if (typeof showNotification === 'function') {
|
||||
showNotification(data.message || 'Update complete', data.status || 'success');
|
||||
|
||||
Reference in New Issue
Block a user