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 <noreply@anthropic.com>

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.

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]