channels.yaml: moves, invalid Downloads, and saves after the write

- A moved Channel's Download diff is retargeted to its new directory:
  one its new entry lists is the same Download, one it does not is
  deleted in the same import (the old directory was treated as kept).
- An unreadable Download is counted as skipped, and its Channel keeps
  its Downloads (it was dropped, and the Channel's Download deleted).
- A new Download without a frequency is dropped before the write and
  counted (it re-opened a write on every import).
- No source id is fetched while entries are skipped (its save would be
  refused, leaving the database newer than the file); a fetched one is
  saved with dump_config, which records the database's version.
- Download.delete(commit=False) neither changes the skip list nor starts
  config saves, which would read the database before the import commits
  (and write a removed Download back into channels.yaml).  The import
  saves download_manager.yaml after its write.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-25 12:29:14 -06:00
parent c54892ffac
commit 5045e37a71
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
3 changed files with 124 additions and 13 deletions

View file

@ -24,7 +24,7 @@ from wrolpi.config_import import ConfigSection, chunks, media_relative
from wrolpi.config_version import register_ownership
from wrolpi.dates import Seconds, from_timestamp
from wrolpi.db import get_db_curs, get_db_session, session_curs
from wrolpi.downloader import Download, download_manager
from wrolpi.downloader import Download, download_manager, save_downloads_config
from wrolpi.errors import UnknownDirectory
from wrolpi.events import Events
from wrolpi.files.lib import split_path_stem_and_suffix
@ -388,18 +388,20 @@ class ChannelDownloadsSection(ConfigSection):
name = 'downloads'
def file_rows(self, config: dict) -> Tuple[Dict[tuple, dict], Set[tuple]]:
rows = dict()
rows, invalid = dict(), set()
for data in config.get('channels') or []:
try:
key = _channel_key(data)
except Exception:
continue
for download in data.get('downloads') or []:
for position, download in enumerate(data.get('downloads') or []):
if isinstance(download, dict) and download.get('url'):
rows[(key, download['url'])] = dict(frequency=download.get('frequency'))
else:
channel_import_logger.error(f'Skipping invalid Download of Channel {key!r}: {download!r}')
return rows, set()
# Counted as skipped; the Channel keeps its Downloads (see `ChannelsConfig.prepare`).
invalid.add((key, download if isinstance(download, str) else f'\0{position}'))
return rows, invalid
def present(self, config: dict, file_rows: dict, invalid_keys: set) -> bool:
# A Channel listed with an empty list of Downloads has none (see `ChannelsConfig.prepare` for entries without
@ -491,22 +493,44 @@ class ChannelsConfig(ConfigFile):
removed_by_name = dict()
for key in channels.removed:
removed_by_name.setdefault(db_rows[key]['name'], []).append(key)
moves = dict()
for key, row in list(channels.added.items()):
if len(old := removed_by_name.get(row['name']) or []) == 1:
channels.removed.discard(old[0])
del channels.added[key]
channels.changed[old[0]] = dict(row, directory=key)
moves[old[0]] = key
# An invalid entry keeps its Channel (found by the entry's name when its directory is invalid), and the
# Channel's Downloads; so does an entry without a list of Downloads (an old config).
# Channel's Downloads; so does an entry without a list of Downloads (an old config), or with an unreadable
# Download.
entries = [i for i in self._config.get('channels') or [] if isinstance(i, dict)]
invalid_names = {i.get('name') for i in entries if i.get('name')} - {i['name'] for i in self._channel_rows()}
for key in [i for i in channels.removed if db_rows[i]['name'] in invalid_names]:
channels.removed.discard(key)
kept = set(db_rows) - set(self._channel_rows_by_key()) - channels.removed
kept = set(db_rows) - set(self._channel_rows_by_key()) - channels.removed - set(moves)
kept |= {key for key, data in self._channel_entries() if 'downloads' not in data}
kept |= {directory for directory, _ in ChannelDownloadsSection().file_rows(self._config)[1]}
# A moved Channel's Downloads are deleted after the move, under its new directory; one its new entry lists
# is the same Download.
for directory, url in [i for i in downloads.removed if i[0] in moves]:
downloads.removed.discard((directory, url))
if (moves[directory], url) in downloads.added:
del downloads.added[(moves[directory], url)]
else:
downloads.removed.add((moves[directory], url))
downloads.removed = {i for i in downloads.removed if i[0] not in kept}
# A new Download needs a frequency.
if missing := {url for (_, url), row in downloads.added.items() if not row['frequency']}:
with get_db_session() as session:
missing -= {i for i, in session.query(Download.url).filter(Download.url.in_(list(missing)))}
for key in [i for i in downloads.added if i[1] in missing]:
channel_import_logger.error(f'Refusing to create Download for Channel without frequency: {key[1]}')
del downloads.added[key]
self.import_skipped += 1
def _channel_entries(self) -> List[Tuple[str, dict]]:
"""The valid entries of the file, keyed by directory."""
entries = list()
@ -535,7 +559,8 @@ class ChannelsConfig(ConfigFile):
.filter(Collection.directory.in_([str(media_directory / i) for i in changed])).all()
from modules.videos import claim_videos_for_channels
claim_videos_for_channels([i.id for i in updated])
for id_, url, source_id, download_missing_data in updated:
# A fetched source id changes channels.yaml, which cannot be saved while it has skipped entries.
for id_, url, source_id, download_missing_data in [] if self.import_skipped else updated:
if not source_id and url and download_missing_data and flags.have_internet.is_set() \
and download_manager.can_download:
channel_import_logger.info(f'Fetching channel source id for Channel id={id_}')
@ -555,6 +580,9 @@ class ChannelsConfig(ConfigFile):
if self._config_outdated:
self.background_dump.activate_switch()
if diffs['downloads']:
# The import's Downloads changed download_manager.yaml; saved after the import's write commits.
save_downloads_config.activate_switch()
def dump_config(self, file: pathlib.Path = None, send_events=False, overwrite=False):
"""Dump all channels from database to config file."""
@ -1041,7 +1069,9 @@ async def fetch_channel_source_id(channel_id: int):
with get_db_session(commit=True) as session:
Channel.find_by_id(session, channel_id).source_id = source_id
save_channels_config.activate_switch()
# Dumped with the database's version (see wrolpi.config_version).
if config := get_channels_config():
config.background_dump.activate_switch()
except Exception as e:
logger.error(f'Failed to get Channel source id of id={channel_id}', exc_info=e)

View file

@ -204,3 +204,83 @@ async def test_no_links_while_entries_are_skipped(test_session, saved_channels,
test_session.expire_all()
assert Download.get_by_url(test_session, 'https://example.com/two').collection_id is None
assert get_config_version(test_session, 'channels.yaml') == yaml.safe_load(saved_channels.read_text())['version']
@pytest.mark.asyncio
async def test_moved_channel_downloads(test_session, saved_channels):
"""A moved Channel keeps the Downloads its new entry lists, and loses the ones it does not, in the same import;
the next import writes nothing."""
edit_channel(saved_channels, 'one', directory='videos/moved/one', downloads=[])
edit_channel(saved_channels, 'two', directory='videos/moved/two',
downloads=[dict(url='https://example.com/two/feed', frequency=86400)])
config = get_channels_config()
config.import_config()
test_session.expire_all()
assert {i.url for i in test_session.query(Download)} == {'https://example.com/two/feed'}
assert directory_of(test_session, 'one') == 'videos/moved/one'
with statements() as collected:
config.import_config()
assert writes(collected) == []
@pytest.mark.asyncio
async def test_invalid_download_keeps_the_channel_downloads(test_session, saved_channels):
"""An unreadable Download of a Channel is counted as skipped, and the Channel's Downloads are kept."""
edit_channel(saved_channels, 'one', downloads=[dict(uri='https://example.com/one')])
config = get_channels_config()
config.import_config()
assert config.import_skipped == 1
test_session.expire_all()
assert {i.url for i in test_session.query(Download)} == {'https://example.com/one'}
@pytest.mark.asyncio
async def test_new_download_without_frequency_is_skipped(test_session, saved_channels):
"""A new Download without a frequency cannot be created: it is skipped (counted), and the next import writes
nothing."""
edit_channel(saved_channels, 'two', downloads=[dict(url='https://example.com/two/feed')])
config = get_channels_config()
with statements() as collected:
config.import_config()
assert config.import_skipped == 1
assert writes(collected) == []
@pytest.mark.asyncio
async def test_no_source_id_fetch_while_entries_are_skipped(test_session, saved_channels, monkeypatch):
"""Fetching a Channel's source id changes channels.yaml; while an entry is skipped it is not fetched."""
from wrolpi import flags
scheduled = []
monkeypatch.setattr(lib, 'background_task', lambda coro: scheduled.append(coro.close()))
monkeypatch.setattr(flags.have_internet, 'is_set', lambda: True)
monkeypatch.setattr(type(lib.download_manager), 'can_download', property(lambda self: True))
edit_config(saved_channels, lambda data: data['channels'].extend([
dict(name='three', directory='videos/three', url='https://example.com/three', download_missing_data=True),
dict(directory='videos/nameless'),
]))
get_channels_config().import_config()
assert set(channels(test_session)) == {'one', 'two', 'three'}
assert scheduled == []
@pytest.mark.asyncio
async def test_import_does_not_save_during_its_write(test_session, saved_channels, test_download_manager_config,
monkeypatch):
"""Deleting a Download inside the import's write neither skips its URL nor starts a config save (which would read
the database before the import commits)."""
from wrolpi.downloader import get_download_manager_config
saves = []
monkeypatch.setattr(lib.save_channels_config, 'activate_switch', lambda *a, **kw: saves.append('channels'))
edit_channel(saved_channels, 'one', downloads=[])
get_channels_config().import_config()
assert saves == []
assert 'https://example.com/one' not in (get_download_manager_config().skip_urls or [])

View file

@ -378,9 +378,10 @@ class Download(ModelHelper, Base): # noqa
download_manager.add_to_skip_list(self.url)
def delete(self, add_to_skip_list: bool = True, commit: bool = True):
"""`commit`: False only flushes (the caller's write commits)."""
"""`commit`: False only flushes; the caller's write commits, and saves the configs afterward (a save now
would read the database before that commit), and does not change the skip list."""
# Do not download this automatically again. This saves the config.
if add_to_skip_list:
if add_to_skip_list and commit:
self.add_to_skip_list()
session = Session.object_session(self)
@ -390,10 +391,10 @@ class Download(ModelHelper, Base): # noqa
collection_kind = collection.kind if collection else None
session.delete(self)
if commit:
session.commit()
else:
if not commit:
session.flush([self])
return
session.commit()
# Save appropriate config based on collection kind. Channels/domains own their configs;
# other kinds have no generic collections config to save.