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]