diff --git a/modules/videos/lib.py b/modules/videos/lib.py index c5dd210f..952f6458 100644 --- a/modules/videos/lib.py +++ b/modules/videos/lib.py @@ -909,15 +909,20 @@ async def fetch_channel_source_id(channel_id: int): # If we can download from a channel, we must have its source_id. try: with get_db_session() as session: - from modules.videos.channel.lib import get_channel channel = Channel.find_by_id(session, channel_id) + source_id, url = channel.source_id, channel.url + if source_id: + return - channel.source_id = channel.source_id or get_channel_source_id(channel.url) - if not channel.source_id: - channel_import_logger.warning(f'Unable to fetch source_id for {channel.url}') - else: - session.commit() - save_channels_config.activate_switch() + # Fetched outside any transaction: this asks the remote site. + source_id = get_channel_source_id(url) + if not source_id: + channel_import_logger.warning(f'Unable to fetch source_id for {url}') + return + + with get_db_session(commit=True) as session: + Channel.find_by_id(session, channel_id).source_id = source_id + save_channels_config.activate_switch() except Exception as e: logger.error(f'Failed to get Channel source id of id={channel_id}', exc_info=e) diff --git a/wrolpi/common.py b/wrolpi/common.py index feb4917f..c18a7736 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -2680,6 +2680,10 @@ def background_task(coro) -> Task: coro_name = getattr(coro, '__qualname__', None) or getattr(coro, '__name__', None) or str(coro) async def error_logger(): + # The task copied its creator's Context. A write the creator declared (`get_db_session(commit=True)`, + # `RequestDB.write()`) is not the task's: the task's own writes must declare themselves. + from wrolpi.db import _write_intent # wrolpi.db imports wrolpi.common. + _write_intent.set(False) try: await coro except Exception as e: diff --git a/wrolpi/tags.py b/wrolpi/tags.py index 8b0c6914..62469b09 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -406,8 +406,7 @@ class TagsConfig(ConfigFile): try: need_commit = False - # Reads, then writes: begin as a writer (see `get_db_session`). - with get_db_session(commit=True) as session: + with get_db_session() as session: if config.tags: # Tags have been saved to config, import them tags_by_name: Dict[str, Tag] = {i.name: i for i in session.query(Tag)} diff --git a/wrolpi/test/test_config_api.py b/wrolpi/test/test_config_api.py index ccb0b6a0..4a947069 100644 --- a/wrolpi/test/test_config_api.py +++ b/wrolpi/test/test_config_api.py @@ -7,6 +7,9 @@ from wrolpi.tags import Tag @pytest.mark.asyncio +# tags.import_config reads, then writes, in a deferred session; splitting its reads (and directory walk) from a +# short write is its own change. +@pytest.mark.allow_deferred_request_writes async def test_configs_crud(async_client, test_session, test_tags_config, tag_factory, test_wrolpi_config, test_channels_config, await_switches): """Configs can be imported or saved on demand.""" diff --git a/wrolpi/test/test_config_backup_import.py b/wrolpi/test/test_config_backup_import.py index 09ef6dc2..8cbf73b4 100644 --- a/wrolpi/test/test_config_backup_import.py +++ b/wrolpi/test/test_config_backup_import.py @@ -603,6 +603,9 @@ class TestBackupAPIEndpoints: assert 'preview' in response.json assert len(response.json['preview']['add']) == 1 + # tags.import_config reads, then writes, in a deferred session; splitting its reads (and directory walk) from a + # short write is its own change. + @pytest.mark.allow_deferred_request_writes async def test_import_endpoint(self, async_client, test_directory, test_tags_config, test_session): from wrolpi.tags import get_tags_config config = get_tags_config() diff --git a/wrolpi/test/test_db.py b/wrolpi/test/test_db.py index 6a64dfc9..2d071f87 100644 --- a/wrolpi/test/test_db.py +++ b/wrolpi/test/test_db.py @@ -368,3 +368,28 @@ def test_deferred_request_writes_are_recorded(test_session): test_session.add(Tag(name='outside', color='#000000')) test_session.flush() assert db_module.DEFERRED_REQUEST_WRITES == [] + + +@pytest.mark.asyncio +async def test_task_started_in_a_write_does_not_inherit_its_intent(test_session): + """A task started inside `RequestDB.write()` copies the request's Context, but not its declared write: the + task's own undeclared writes are recorded.""" + from wrolpi import db as db_module + from wrolpi.common import background_task + + async def undeclared_write(): + with get_db_session() as session: + session.add(Tag(name='from a task', color='#000000')) + session.flush() + + test_session.commit() + with production_like_sessions(test_session): + db = RequestDB() + with db.write(): + task = background_task(undeclared_write()) + await task + db.finish(ok=True) + + statements = [statement for statement, _ in db_module.DEFERRED_REQUEST_WRITES] + db_module.DEFERRED_REQUEST_WRITES.clear() + assert len(statements) == 1 and statements[0].startswith('INSERT INTO tag'), statements