From bc58a5cc35dd651fb99294bb7ae5eb37a5505be7 Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Sun, 25 Oct 2026 20:32:12 -0600 Subject: [PATCH] Save download_manager.yaml after new Downloads are committed (#882) create_downloads activated save_downloads_config right after its flush, before the caller committed. The switch runs in the perpetual process, which cannot see uncommitted Downloads, so a save that ran first wrote the file without them and nothing saved them later. The switch is now activated when the session commits, and not at all when it rolls back. Co-authored-by: Claude Opus 5.5 --- wrolpi/downloader.py | 25 ++++++++++++++++++++++--- wrolpi/test/test_downloader.py | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index bf0ef65a..fdad945a 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -24,7 +24,7 @@ from zoneinfo import ZoneInfo import feedparser import pytz from feedparser import FeedParserDict -from sqlalchemy import Column, Integer, String, Text, ForeignKey, Index, JSON, insert, or_, text +from sqlalchemy import Column, Integer, String, Text, ForeignKey, Index, JSON, event, insert, or_, text from sqlalchemy.exc import OperationalError from sqlalchemy.orm import Mapped, Session, column_property, relationship @@ -1017,6 +1017,8 @@ class DownloadManager: logger.debug(f'Created {len(downloads)} new downloads') session.flush(downloads) + # Save the config once the new Downloads are committed (see `_save_downloads_config_after_commit`). + session.info[SAVE_DOWNLOADS_CONFIG_AFTER_COMMIT] = True try: from wrolpi import perpetual if perpetual.IN_PERPETUAL_PROCESS or PYTEST: @@ -1024,8 +1026,6 @@ class DownloadManager: # them for perpetual_download_worker's next cycle (a few seconds) rather than running a download # inside the API worker. background_task(self.dispatch_downloads()) - # Save the config now that new Downloads exist. - save_downloads_config.activate_switch() except RuntimeError: # Event loop isn't running. Probably testing? if not PYTEST: @@ -2504,6 +2504,25 @@ def save_downloads_config(): save_downloads_config: ActivateSwitchMethod +SAVE_DOWNLOADS_CONFIG_AFTER_COMMIT = 'save_downloads_config_after_commit' + + +@event.listens_for(Session, 'after_commit') +def _save_downloads_config_after_commit(session: Session): + """The switch runs in another process, which cannot see Downloads until they are committed.""" + if session.info.pop(SAVE_DOWNLOADS_CONFIG_AFTER_COMMIT, False): + try: + save_downloads_config.activate_switch() + except RuntimeError: + # Sanic's shared context does not exist. Probably testing? + if not PYTEST: + logger.error('Unable to save the downloads config after creating Downloads', exc_info=True) + + +@event.listens_for(Session, 'after_rollback') +def _forget_downloads_config_save(session: Session): + session.info.pop(SAVE_DOWNLOADS_CONFIG_AFTER_COMMIT, None) + async def import_downloads_config(): """Upsert all Downloads in the Download Manager Config into the DB. diff --git a/wrolpi/test/test_downloader.py b/wrolpi/test/test_downloader.py index cfce9d55..2f888223 100644 --- a/wrolpi/test/test_downloader.py +++ b/wrolpi/test/test_downloader.py @@ -2195,3 +2195,25 @@ async def test_create_downloads_does_not_dispatch_outside_the_perpetual_process( test_download_manager.create_downloads(test_session, ['https://example.com/perpetual'], test_downloader.name) await asyncio.sleep(0) dispatch.assert_called_once() + + +@pytest.mark.asyncio +async def test_create_downloads_saves_the_config_after_commit(test_session, test_download_manager, test_downloader, + monkeypatch): + """download_manager.yaml is saved once the new Downloads are committed: a save before the commit could not + see them (the switch runs in another process), and nothing would save them later.""" + from wrolpi import downloader + saves = [] + monkeypatch.setattr(downloader.save_downloads_config, 'activate_switch', lambda *a, **kw: saves.append(1)) + + test_download_manager.create_downloads(test_session, ['https://example.com/one'], test_downloader.name) + assert saves == [] + + test_session.commit() + assert saves == [1] + + # A rolled back Download is not saved. + test_download_manager.create_downloads(test_session, ['https://example.com/two'], test_downloader.name) + test_session.rollback() + test_session.commit() + assert saves == [1]