Tasks don't inherit a declared write; tags import change backed out

From review of #864:

- background_task's task copied its creator's Context, including
  _write_intent when started inside get_db_session(commit=True) or
  RequestDB.write(), so the guard ignored the task's own writes for its
  lifetime.  The task now starts with no declared write.  That exposes
  fetch_channel_source_id (started by the channels config import): it read
  the Channel, asked the remote site for its source_id, then committed in
  the same deferred transaction.  It now reads, fetches with no
  transaction open, then writes in a short get_db_session(commit=True).
- tags.import_config: commit=True held the write lock through its directory
  walk, and its exit commit saved changes the need_commit gate used to
  drop.  Reverted to master; splitting its reads from a short write is its
  own change.  The two tests that run it through a request are marked
  allow_deferred_request_writes until then.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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)
 

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:

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)}

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."""

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()

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