rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-25 18:40:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Configs without database rows read and validate only; imports run off the event loop

Browse source 
The base import for a config with no sections now reads its file once, reports a 0-byte file as empty, treats a
missing file as the defaults, and records success or failure itself (sending the failure Event on request).  The
per-config overrides that only set successful_import are removed.

wrolpi.yaml and the downloader settings import in a thread at startup (import_file_configs), and the Import, Save
and backup Import buttons run their config work in a thread.

The pre-diff import path (import_decision, record_import_version) has no callers left and is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-25 14:58:58 -06:00
parent ee10b34b15
commit 5aee978048
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
12 changed files with 251 additions and 145 deletions
Download patch file Download diff file Expand all files Collapse all files

37
main.py
View file

@ -4,7 +4,6 @@ import asyncio
import logging
import os
import sys
from contextlib import contextmanager
from sanic import Sanic
from sanic.signals import Event
@ -15,7 +14,7 @@ from wrolpi import root_api # noqa
from wrolpi.api_utils import api_app, perpetual_signal, per_worker_task
from wrolpi.common import logger, check_media_directory, set_log_level, limit_concurrent, \
cancel_refresh_tasks, cancel_background_tasks, get_wrolpi_config, can_connect_to_server, wrol_mode_enabled, \
create_empty_config_files, TRACE_LEVEL
create_empty_config_files, import_file_configs, TRACE_LEVEL
from wrolpi.contexts import attach_shared_contexts, reset_shared_contexts, initialize_configs_contexts
from wrolpi.dates import Seconds
from modules.videos.cookies import lock_cookies
@ -29,15 +28,6 @@ from wrolpi.version import get_version_string
logger = logger.getChild('wrolpi-main')
@contextmanager
def log_and_suppress(*exceptions, message="Exception suppressed"):
"""Like contextlib.suppress but logs the exception at INFO level first."""
try:
yield
except exceptions as e:
logger.info(f'{message}: {e}', exc_info=e)
def db_main(args):
"""
Handle database migrations. This uses Alembic, supported commands are "upgrade", "downgrade", and "check".
@ -266,29 +256,10 @@ async def start_single_tasks(app: Sanic):
logger.debug(f'start_single_tasks started')
# Import configs, ignore errors so the service will start. Configs will refuse to save if they failed to import.
with log_and_suppress(Exception, message='Failed to import wrolpi config'):
get_wrolpi_config().import_config()
logger.debug('wrolpi config imported')
# Import configs that don't require the database. Errors are logged so the service will start; configs refuse
# to save if they failed to import.
await import_file_configs()
wrolpi_config = get_wrolpi_config()
if wrolpi_config.successful_import:
# Only import other configs if WROLPi config was imported, and WROL mode is not enabled.
if wrolpi_config.wrol_mode:
logger.warning('Refusing to import other configs when WROL mode is enabled!')
else:
# Import configs that don't require the database.
with log_and_suppress(Exception, message='Failed to import videos downloader config'):
get_videos_downloader_config().import_config()
logger.debug('videos downloader config imported')
with log_and_suppress(Exception, message='Failed to import archive downloader config'):
from modules.archive.lib import get_archive_downloader_config
get_archive_downloader_config().import_config()
logger.debug('archive downloader config imported')
with log_and_suppress(Exception, message='Failed to import download cache config'):
from wrolpi.downloader import get_download_cache_config
get_download_cache_config().import_config()
logger.debug('download cache config imported')
logger.debug('start_single_tasks waiting for db...')
async with flags.db_up.wait_for():

4
modules/archive/lib.py
View file

@ -418,10 +418,6 @@ class ArchiveDownloaderConfig(ConfigFile):
"""Get the custom user agent, or None for system default."""
return self._config.get('user_agent')
def import_config(self, file: pathlib.Path = None, send_events=False):
super().import_config(file, send_events)
self.successful_import = True
# Global instance for archive downloader config
ARCHIVE_DOWNLOADER_CONFIG: ArchiveDownloaderConfig = ArchiveDownloaderConfig()

5
modules/flasher/config.py
View file

@ -26,11 +26,6 @@ class FlasherConfig(ConfigFile):
)
validator = FlasherConfigValidator
def import_config(self, file=None, send_events=False):
super().import_config(file, send_events)
# YAML-only config (no database); a successful read is a successful import.
self.successful_import = True
@property
def configurations(self) -> List[dict]:
return list(self._config.get('configurations', []))

1
modules/inventory/catalog.py
View file

@ -105,7 +105,6 @@ def import_catalog_config():
config = get_catalog_config()
if config.get_file().is_file():
config.import_config()
config.successful_import = True
seed_catalog_defaults(config)

4
modules/map/pins.py
View file

@ -22,10 +22,6 @@ class MapPinsConfig(ConfigFile):
)
validator = MapPinsConfigValidator
def import_config(self, file=None, send_events=False):
super().import_config(file, send_events)
self.successful_import = True
@property
def pins(self) -> List[dict]:
return list(self._config.get('pins', []))

4
modules/videos/lib.py
View file

@ -909,10 +909,6 @@ class VideoDownloaderConfig(ConfigFile):
def strict_codecs(self, value: bool):
self.update({**self._config, 'strict_codecs': value})
def import_config(self, file: pathlib.Path = None, send_events=False):
super().import_config(file, send_events)
self.successful_import = True
VIDEOS_DOWNLOADER_CONFIG: VideoDownloaderConfig = VideoDownloaderConfig()
TEST_VIDEOS_DOWNLOADER_CONFIG: VideoDownloaderConfig = None

6
wrolpi/bookmarks.py
View file

@ -149,12 +149,6 @@ class BookmarksConfig(ConfigFile):
return False
return True
def import_config(self, file=None, send_events=False):
# An invalid file raises here, leaving successful_import False so save() will not
# overwrite it. A missing file is imported as empty, so the first save creates it.
super().import_config(file, send_events)
self.successful_import = True
@property
def bookmarks(self) -> List[dict]:
# A deep copy: the tree is nested, and callers mutate what they get back before

132
wrolpi/common.py
View file

@ -772,26 +772,33 @@ class ConfigFile:
if self.sections:
return self.import_sections(file, send_events)
# A config without sections keeps no database rows: read and validate its file, nothing more.
from wrolpi.events import Events
file = file or self.get_file()
file_str = str(get_relative_to_media_directory(file))
# Caller will set to successful if it works.
self.successful_import = False
# Subclasses count entries they cannot import; a fresh import starts clean.
self.import_skipped = 0
if file.is_file():
if not self.is_valid(file):
raise InvalidConfig(f'Config is invalid: {file_str}')
data = self.read_config_file(file)
new_data, extra_data = partition(lambda i: i[0] in self.default_config, data.items())
new_data, extra_data = dict(new_data), dict(extra_data)
if extra_data:
logger_.warning(f'Ignoring extra config data ({file_str}): {extra_data}')
self._config.update(new_data)
# Import call above this will set successful_import.
# self.successful_import = True
else:
logger_.error(f'Failed to import {file_str} because it does not exist.')
try:
if file.is_file():
data = self.read_config_file(file) if file.stat().st_size else None
if not data or not self.validate(data):
raise InvalidConfig(f'Config is {"invalid" if data else "empty"}: {file_str}')
new_data, extra_data = partition(lambda i: i[0] in self.default_config, data.items())
new_data, extra_data = dict(new_data), dict(extra_data)
if extra_data:
logger_.warning(f'Ignoring extra config data ({file_str}): {extra_data}')
self._config.update(new_data)
else:
# The defaults apply; the first save creates the file.
logger_.info(f'No config file, using the defaults: {file_str}')
self.successful_import = True
except Exception as e:
self.successful_import = False
message = f'Failed to import {file_str}'
logger_.error(message, exc_info=e)
if send_events:
Events.send_config_import_failed(message)
raise
def import_sections(self, file: pathlib.Path = None, send_events=False):
"""Import a config with `sections`: read and validate the file once, decide what its version allows (see
@ -891,7 +898,7 @@ class ConfigFile:
f'{len(diff.removed)} removed' for name, diff in changed.items()) or 'no changes'))
self.after_import(diffs)
if decision != IMPORT:
# The database may have rows the file does not list; save them (see `record_import_version`).
# The database may have rows the file does not list; save them (see `record_saved_version`).
self.background_dump.activate_switch()
return True
@ -924,41 +931,16 @@ class ConfigFile:
try:
self.import_config(send_events=send_events)
finally:
# An import that returned before `import_decision` must not force a later one.
# An import that returned before reading `force_import` must not force a later one.
self.force_import = False
def is_versioned_by_database(self) -> bool:
from wrolpi.config_version import versioned_config_names
return self.file_name in versioned_config_names()
# What the last `import_decision` allowed; `record_import_version` records the version that follows from it.
last_import_decision: str = None
def import_decision(self) -> str:
"""After reading the file (`ConfigFile.import_config`), decide what importing it may do: IMPORT,
IMPORT_WITHOUT_DELETES, or SAVE (the database is newer; call `save_database_instead` and import nothing).
On IMPORT the database's version is removed until `record_import_version`: the import's own writes must not
count, or an import that fails partway would leave the database "newer" than the file it did not finish
applying, and the next startup would save that partial database over the file."""
from wrolpi.config_version import IMPORT, decide, delete_config_version, get_config_version
from wrolpi.db import get_db_session
with get_db_session(commit=True) as session:
db_version = get_config_version(session, self.file_name)
if self.force_import:
self.force_import = False
decision = IMPORT
else:
decision = decide(self.version or 0, db_version)
if decision == IMPORT:
delete_config_version(session, self.file_name)
logger_.debug(f'{self.file_name}: file version {self.version}, database version {db_version}: {decision}')
self.last_import_decision = decision
return decision
def version_decision(self, forced: bool = False) -> Tuple[str, Optional[int]]:
"""What importing this config's file (already read) may do, and the database version that decided it. For
`import_sections`, which records the version in its own write (`import_decision` is for the others)."""
"""What importing this config's file (already read) may do (IMPORT, IMPORT_WITHOUT_DELETES, or SAVE), and the
database version that decided it. `import_sections` records the file's version in its own write."""
from wrolpi.config_version import IMPORT, decide, get_config_version
from wrolpi.db import get_db_session
with get_db_session() as session:
@ -968,27 +950,11 @@ class ConfigFile:
return decision, db_version
def save_database_instead(self):
"""The database has changes this config's file does not (see `import_decision`); save them."""
"""The database has changes this config's file does not (see `version_decision`); save them."""
logger_.warning(f'{self.file_name} is older than the database; saving the database instead of importing it')
self.successful_import = True
self.dump_config()
def record_import_version(self):
"""The database now matches this config's file: record the file's version. Call last, after every write
the import makes.
Not after IMPORT_WITHOUT_DELETES: the database may keep rows the file does not list (changes that were never
saved), so recording the file's version would let the next import delete them. Save them into the file
instead; that save records the version (`record_saved_version`). A save after a partial import is refused,
so the version stays unknown until an import applies the whole file."""
from wrolpi.config_version import IMPORT, set_config_version
from wrolpi.db import get_db_session
if self.last_import_decision != IMPORT:
self.background_dump.activate_switch()
return
with get_db_session(commit=True) as session:
set_config_version(session, self.file_name, self.version or 0)
def db_version_for_dump(self, session) -> int:
"""The version `dump_config` writes: the database's, read in the same transaction as the rows it saves.
Before the database has one, the file's next version (`save` records it)."""
@ -997,8 +963,8 @@ class ConfigFile:
return db_version if db_version is not None else (self.version or 0) + 1
def record_saved_version(self, version: int):
"""The file was just written from the database. If the database's version was unknown (see
`record_import_version`), the two are now in step: record it, so changes from here on count against it."""
"""The file was just written from the database. If the database's version was unknown (an import without
deletes), the two are now in step: record it, so changes from here on count against it."""
from wrolpi.config_version import create_config_version
from wrolpi.db import get_db_session
with get_db_session(commit=True) as session:
@ -1466,6 +1432,38 @@ def get_all_configs() -> Dict[str, ConfigFile]:
return all_configs
async def import_file_configs():
"""Import the configs that do not need the database: wrolpi.yaml, then the downloader settings (not when
wrolpi.yaml failed, or while WROL mode is enabled). Errors are logged so the service starts; a config that failed
to import refuses to save.
This runs from a worker's after_server_start while the worker is already accepting requests, so no import may
block the event loop."""
from modules.archive.lib import get_archive_downloader_config
from modules.videos.lib import get_videos_downloader_config
from wrolpi.downloader import get_download_cache_config
wrolpi_config = get_wrolpi_config()
try:
await asyncio.to_thread(wrolpi_config.import_config)
logger.debug('wrolpi config imported')
except Exception as e:
logger.warning(f'Failed to import wrolpi config: {e}')
if not wrolpi_config.successful_import:
return
if wrolpi_config.wrol_mode:
logger.warning('Refusing to import other configs when WROL mode is enabled!')
return
for config in (get_videos_downloader_config(), get_archive_downloader_config(), get_download_cache_config()):
try:
await asyncio.to_thread(config.import_config)
logger.debug(f'{config.file_name} imported')
except Exception as e:
logger.warning(f'Failed to import {config.file_name}: {e}')
async def import_all_db_configs() -> dict[str, bool]:
"""
Import all configs that require the database.
@ -1670,10 +1668,8 @@ class WROLPiConfig(ConfigFile):
return default_config_path
def import_config(self, file: pathlib.Path = None, send_events=False):
super().import_config(file, send_events)
try:
# WROLPiConfig does not sync to database.
super().import_config(file)
# Destinations cannot be empty.
self._config['archive_destination'] = self.archive_destination or self.default_config['archive_destination']
self._config['map_destination'] = self.map_destination or self.default_config['map_destination']
@ -1699,8 +1695,6 @@ class WROLPiConfig(ConfigFile):
if repos_root not in ignored:
ignored.append(repos_root)
self._config['ignored_directories'] = ignored
self.successful_import = True
except Exception as e:
self.successful_import = False
message = f'Failed to import {self.file_name}'

14
wrolpi/config_api.py
View file

@ -1,3 +1,5 @@
import asyncio
from sanic import response, Blueprint
from sanic.request import Request
from sanic_ext.extensions.openapi import openapi
@ -46,11 +48,11 @@ def post_config_update(request: Request, body: config_schema.ConfigUpdateRequest
body=config_schema.ConfigsRequest,
validate=True,
)
def post_config_import(_: Request, body: config_schema.ConfigsRequest):
async def post_config_import(_: Request, body: config_schema.ConfigsRequest):
config = get_config_by_file_name(body.file_name)
try:
# The user chose to apply the file, whatever its version.
config.import_forced(send_events=True)
await asyncio.to_thread(config.import_forced, send_events=True)
except Exception as e:
logger.error(f'Failed to import config: {body.file_name}', exc_info=e)
raise InvalidConfig(f'Failed to import config {body.file_name}')
@ -64,10 +66,10 @@ def post_config_import(_: Request, body: config_schema.ConfigsRequest):
body=config_schema.ConfigsRequest,
validate=True,
)
def post_config_dump(_: Request, body: config_schema.ConfigsRequest):
async def post_config_dump(_: Request, body: config_schema.ConfigsRequest):
config = get_config_by_file_name(body.file_name)
try:
config.dump_config(send_events=True, overwrite=body.overwrite)
await asyncio.to_thread(config.dump_config, send_events=True, overwrite=body.overwrite)
except Exception as e:
logger.error(f'Failed to dump config: {body.file_name}', exc_info=e)
raise InvalidConfig(f'Failed to dump config {body.file_name}')
@ -114,7 +116,7 @@ def post_config_backup_preview(_: Request, body: config_schema.ConfigBackupPrevi
body=config_schema.ConfigBackupImportRequest,
validate=True,
)
def post_config_backup_import(_: Request, body: config_schema.ConfigBackupImportRequest):
async def post_config_backup_import(_: Request, body: config_schema.ConfigBackupImportRequest):
if body.mode not in ('merge', 'overwrite'):
raise InvalidConfig('mode must be "merge" or "overwrite"')
config = get_config_by_file_name(body.file_name)
@ -124,7 +126,7 @@ def post_config_backup_import(_: Request, body: config_schema.ConfigBackupImport
if not backup_file.is_file():
raise InvalidConfig(f'Backup file not found for date {body.backup_date}')
try:
config.import_backup(body.backup_date, body.mode, send_events=True)
await asyncio.to_thread(config.import_backup, body.backup_date, body.mode, send_events=True)
except Exception as e:
logger.error(f'Failed to import backup: {body.file_name} {body.backup_date}', exc_info=e)
raise InvalidConfig(f'Failed to import backup {body.file_name}')

6
wrolpi/config_version.py
View file

@ -5,7 +5,7 @@ A database config (tags.yaml, channels.yaml, ...) saves rows the database owns.
config's database version, in the same transaction (`_count_config_changes`). `dump_config` writes the database's
version into the file it saves, and an import records the file's version once it has applied it.
So an import can tell which side is newer by comparing two integers (see `ConfigFile.import_decision`):
So an import can tell which side is newer by comparing two integers (see `ConfigFile.version_decision`):
* file == database: in step. The file is current; import it, deleting what it no longer lists.
* file > database: the file is newer (a restored or wiped database, a copied config); import it the same way.
@ -153,10 +153,6 @@ def get_config_version(session: Session, name: str) -> Optional[int]:
return row[0] if row else None
def delete_config_version(session: Session, name: str):
session.connection().execute(text('DELETE FROM config_version WHERE name = :name'), {'name': name})
def create_config_version(session: Session, name: str, version: int):
"""Record `version` unless the database already has one."""
session.connection().execute(

8
wrolpi/downloader.py
View file

@ -2431,14 +2431,6 @@ class DownloadCacheConfig(ConfigFile):
)
validator = DownloadCacheConfigValidator
def import_config(self, file: pathlib.Path = None, send_events=False):
file = file or self.get_file()
if not file.is_file():
self.successful_import = True
return
super().import_config(file, send_events)
self.successful_import = True
@property
def video_durations(self) -> List[list]:
return self._config['video_durations'] or list()

175
wrolpi/test/test_file_config_import.py Normal file
View file

@ -0,0 +1,175 @@
"""A config that keeps no database rows imports by reading and validating its file, nothing more. The Import button
and the startup imports read files and may write the database, so they run off the event loop."""
import asyncio
from http import HTTPStatus
import pytest
from modules.archive.lib import ArchiveDownloaderConfig
from modules.map.pins import MapPinsConfig
from modules.videos.lib import VideoDownloaderConfig
from wrolpi.common import ConfigFile, WROLPiConfig
from wrolpi import common
from wrolpi.downloader import DownloadCacheConfig
from wrolpi.errors import InvalidConfig
from wrolpi.events import Events
@pytest.fixture
def pins_config(async_client):
config = MapPinsConfig()
config.get_file().parent.mkdir(parents=True, exist_ok=True)
return config
def on_event_loop() -> bool:
try:
asyncio.get_running_loop()
return True
except RuntimeError:
return False
@pytest.mark.asyncio
async def test_import(pins_config):
"""A valid file is applied, and the import succeeds without the subclass saying so."""
pins_config.get_file().write_text('version: 3\npins: [one, two]\n')
pins_config.import_config()
assert pins_config.successful_import is True
assert pins_config._config['pins'] == ['one', 'two']
@pytest.mark.asyncio
async def test_import_reads_once(pins_config, monkeypatch):
"""The file is parsed once, not once to validate and again to apply."""
pins_config.get_file().write_text('version: 3\npins: [one]\n')
reads = []
read = ConfigFile.read_config_file
monkeypatch.setattr(MapPinsConfig, 'read_config_file', lambda self, *a: reads.append(1) or read(self, *a))
pins_config.import_config()
assert len(reads) == 1
@pytest.mark.asyncio
async def test_import_missing(pins_config):
"""A missing file is imported as the defaults; the first save creates it."""
pins_config.import_config()
assert pins_config.successful_import is True
assert pins_config._config['pins'] == []
@pytest.mark.asyncio
async def test_import_empty(pins_config):
"""A 0-byte file is reported as empty, and never saved over."""
pins_config.get_file().write_text('')
with pytest.raises(InvalidConfig, match='empty'):
pins_config.import_config()
assert pins_config.successful_import is False
@pytest.mark.asyncio
async def test_import_invalid(pins_config, monkeypatch):
"""An invalid file raises, is never saved over, and the Import button reports it."""
pins_config.get_file().write_text('- not\n- a mapping\n')
sent = []
monkeypatch.setattr(Events, 'send_config_import_failed', lambda message: sent.append(message))
with pytest.raises(InvalidConfig):
pins_config.import_config(send_events=True)
assert pins_config.successful_import is False
assert sent == ['Failed to import config/map_pins.yaml']
with pytest.raises(RuntimeError, match='never successfully imported'):
pins_config.save()
@pytest.mark.asyncio
async def test_startup_imports_off_event_loop(test_wrolpi_config, monkeypatch):
"""wrolpi.yaml and the downloader settings are imported in a thread at startup."""
imported = []
def record(self, *args, **kwargs):
imported.append((self.file_name, on_event_loop()))
self.successful_import = True
for cls in (WROLPiConfig, VideoDownloaderConfig, ArchiveDownloaderConfig, DownloadCacheConfig):
monkeypatch.setattr(cls, 'import_config', record)
await common.import_file_configs()
assert imported == [
('wrolpi.yaml', False),
('videos_downloader.yaml', False),
('archives_downloader.yaml', False),
('download_cache.yaml', False),
]
@pytest.mark.asyncio
async def test_startup_imports_wrol_mode(test_wrolpi_config, monkeypatch):
"""No other config is imported when wrolpi.yaml failed, or while WROL mode is enabled."""
imported = []
def record(self, *args, **kwargs):
imported.append(self.file_name)
for cls in (VideoDownloaderConfig, ArchiveDownloaderConfig, DownloadCacheConfig):
monkeypatch.setattr(cls, 'import_config', record)
def fail(self, *args, **kwargs):
self.successful_import = False
raise InvalidConfig('invalid')
monkeypatch.setattr(WROLPiConfig, 'import_config', fail)
await common.import_file_configs()
assert imported == []
def wrol_mode(self, *args, **kwargs):
self.successful_import = True
self._config['wrol_mode'] = True
monkeypatch.setattr(WROLPiConfig, 'import_config', wrol_mode)
await common.import_file_configs()
assert imported == []
@pytest.mark.asyncio
@pytest.mark.parametrize('path,method,body', [
('/api/config/import', 'import_forced', dict(file_name='tags.yaml')),
('/api/config/dump', 'dump_config', dict(file_name='tags.yaml')),
])
async def test_api_off_event_loop(async_client, test_tags_config, monkeypatch, path, method, body):
"""The Import and Save buttons read the database and the file in a thread, not on the event loop."""
from wrolpi.tags import TagsConfig
calls = []
monkeypatch.setattr(TagsConfig, method, lambda self, *args, **kwargs: calls.append(on_event_loop()))
request, response = await async_client.post(path, json=body)
assert response.status_code == HTTPStatus.NO_CONTENT
assert calls == [False]
@pytest.mark.asyncio
async def test_backup_import_off_event_loop(async_client, test_tags_config, monkeypatch):
"""Importing a backup runs in a thread, not on the event loop."""
from wrolpi.tags import TagsConfig, get_tags_config
config = get_tags_config()
backup = config._get_backup_file('20260101')
backup.parent.mkdir(parents=True, exist_ok=True)
backup.write_text('version: 1\ntags: {}\n')
calls = []
monkeypatch.setattr(TagsConfig, 'import_backup', lambda self, *args, **kwargs: calls.append(on_event_loop()))
body = dict(file_name='tags.yaml', backup_date='20260101', mode='merge')
request, response = await async_client.post('/api/config/backup/import', json=body)
assert response.status_code == HTTPStatus.NO_CONTENT
assert calls == [False]