rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-25 18:40:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Sync Zim subscriptions directly during the downloads import

Browse source 
A row now compares its subscription's (name, language), derived from the
URL, instead of whether one exists.  The import moves a subscription of
that name onto the written Download (a changed language or URL), removes
one the file no longer implies, and leaves the Download's own columns as
the file has them; change_download renamed the old Download's URL into
the one just inserted, and reset attempts and the downloaders.  A Kiwix
URL that names no known subscription is a plain Download instead of
failing the whole write.

prepare runs only when there are differences, and its docstring states
that it must be safe to repeat and to abandon.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Roland 2026-10-24 17:37:58 -06:00
parent 949d8b4923
commit 9b331e7a84
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
3 changed files with 157 additions and 20 deletions
Download patch file Download diff file Expand all files Collapse all files

11
wrolpi/common.py
View file

@ -846,7 +846,8 @@ class ConfigFile:
db_rows = {i.name: i.db_rows(session) for i in self.sections}
diffs = {i.name: diff_section(*file_rows[i.name], db_rows[i.name], deletes=decision == IMPORT)
for i in self.sections}
self.prepare(diffs)
if any(diffs.values()):
self.prepare(diffs)
file_version = self.version or 0
if any(diffs.values()) or (decision == IMPORT and db_version != file_version):
@ -885,8 +886,12 @@ class ConfigFile:
return True
def prepare(self, diffs: dict):
"""Slow work the write needs (files, network), run with no transaction open. `diffs` maps each section's
name to its `SectionDiff`."""
"""Slow work the write needs (files, network), run with no transaction open, and only when there are
differences. `diffs` maps each section's name to its `SectionDiff`.
It must be safe to repeat and to abandon: when the database changes before the write, the import decides
again (up to three passes), and the diffs it was given may never be written (the write fails, or the next
pass saves the database instead)."""
def after_import(self, diffs: dict):
"""Run after the write: activate switches, send events, start background tasks."""

72
wrolpi/downloader.py
View file

@ -2062,19 +2062,28 @@ def _utc(value) -> Optional[datetime]:
return (value if value.tzinfo else value.replace(tzinfo=pytz.utc)).astimezone(pytz.utc)
def is_zim_subscription_download(url: str, frequency: Optional[int]) -> bool:
"""A recurring Kiwix Download belongs to a ZimSubscription."""
return bool(frequency) and url.startswith('https://download.kiwix.org/')
def zim_subscription_for(url: str, frequency: Optional[int]) -> Optional[Tuple[str, str]]:
"""The (name, language) of the ZimSubscription a recurring Kiwix Download belongs to. None for any other
Download, including a Kiwix URL that names no known subscription."""
from modules.zim.lib import zim_download_url_to_name
if not frequency or not url.startswith('https://download.kiwix.org/'):
return None
try:
return zim_download_url_to_name(url)
except Exception as e:
logger.warning(f'Download {url} is not a known Zim subscription: {e}')
return None
class DownloadsSection(ConfigSection):
"""download_manager.yaml's Downloads, keyed by URL. A recurring Kiwix Download's ZimSubscription is derived
from it: `subscribed` compares whether the subscription exists, so a missing one is restored."""
from it: `subscription` compares the subscription's (name, language), so a missing, stale or unwanted one is
corrected."""
name = 'downloads'
@staticmethod
def _row(url, destination, downloader, frequency, last_successful_download, next_download, settings, status,
sub_downloader, tag_names, subscribed, defaults: dict) -> dict:
sub_downloader, tag_names, subscription, defaults: dict) -> dict:
return dict(
destination=str(destination) if destination else None,
downloader=downloader,
@ -2086,7 +2095,7 @@ class DownloadsSection(ConfigSection):
status=status or DownloadStatus.new,
sub_downloader=sub_downloader,
tag_names=list(tag_names) if tag_names else None,
subscribed=subscribed,
subscription=tuple(subscription) if subscription else None,
)
def file_rows(self, config: dict) -> Tuple[Dict[str, dict], Set[str]]:
@ -2106,11 +2115,19 @@ class DownloadsSection(ConfigSection):
rows[url] = self._row(
url, destination, data['downloader'], data.get('frequency'),
data.get('last_successful_download'), data.get('next_download'), settings, data.get('status'),
data.get('sub_downloader'), tag_names, is_zim_subscription_download(url, data.get('frequency')),
data.get('sub_downloader'), tag_names, zim_subscription_for(url, data.get('frequency')),
defaults)
except Exception as e:
logger.error(f'Skipping invalid Download in config: {data}', exc_info=e)
invalid.add(url)
# A subscription's name is unique: the first Download listed for it gets it.
names = set()
for url, row in rows.items():
if row['subscription'] and row['subscription'][0] in names:
logger.warning(f'Download {url} is a second Download for Zim subscription {row["subscription"][0]}')
row['subscription'] = None
elif row['subscription']:
names.add(row['subscription'][0])
return rows, invalid
def db_rows(self, session: Session) -> Dict[str, dict]:
@ -2119,15 +2136,13 @@ class DownloadsSection(ConfigSection):
query = session.query(
Download.url, Download.destination, Download.downloader, Download.frequency,
Download.last_successful_download, Download.next_download, Download.settings, Download.status,
Download.sub_downloader, Download.tag_names, ZimSubscription.id,
Download.sub_downloader, Download.tag_names, ZimSubscription.name, ZimSubscription.language,
).outerjoin(ZimSubscription, ZimSubscription.download_id == Download.id) \
.filter(*Download.saved_by_config_filter())
return {url: self._row(url, *values, subscribed=subscription_id is not None, defaults=defaults)
for url, *values, subscription_id in query}
return {url: self._row(url, *values, subscription=(name, language) if name else None, defaults=defaults)
for url, *values, name, language in query}
def upsert(self, session: Session, rows: Dict[str, dict]):
from modules.zim.lib import zim_download_url_to_name
from modules.zim.models import ZimSubscription
existing = dict()
for urls in chunks(rows):
existing.update({i.url: i for i in session.query(Download).filter(Download.url.in_(urls))})
@ -2150,14 +2165,37 @@ class DownloadsSection(ConfigSection):
for name, value in values[url].items():
setattr(download, name, value)
session.flush()
self._sync_subscriptions(session, rows)
@staticmethod
def _sync_subscriptions(session: Session, rows: Dict[str, dict]):
"""Give each written Download the ZimSubscription its row names, and no other. A subscription of that name
on another Download (the URL changed) is moved to this one. The Download's own columns are the file's."""
from modules.zim.models import ZimSubscription
ids = dict()
for urls in chunks(rows):
ids.update(session.query(Download.url, Download.id).filter(Download.url.in_(urls)))
by_download = dict()
for download_ids in chunks(ids.values()):
by_download.update({i.download_id: i for i in
session.query(ZimSubscription).filter(ZimSubscription.download_id.in_(download_ids))})
for url, row in rows.items():
if row['subscribed']:
name, language = zim_download_url_to_name(url)
subscription = session.query(ZimSubscription).filter_by(name=name).one_or_none() \
or ZimSubscription(name=name, language=language)
subscription.change_download(session, url, row['frequency'])
download_id = ids[url]
current = by_download.get(download_id)
if current is not None and current.download_id != download_id:
current = None # Moved to an earlier row's Download.
if current is not None and (not row['subscription'] or current.name != row['subscription'][0]):
session.delete(current)
session.flush()
current = None
if row['subscription']:
name, language = row['subscription']
subscription = current or session.query(ZimSubscription).filter_by(name=name).one_or_none() \
or ZimSubscription(name=name)
subscription.language = language
subscription.download_id = download_id
session.add(subscription)
session.flush()
def delete(self, session: Session, keys: Set[str]):
for urls in chunks(keys):

94
wrolpi/test/test_config_import.py
View file

@ -290,3 +290,97 @@ async def test_relative_destination(test_session, test_directory, saved_download
with statements() as collected:
config.import_config()
assert writes(collected) == []
def edit_kiwix(path, **values):
def change(data):
for download in data['downloads']:
if download['url'] == KIWIX_URL:
download.update(values)
edit_config(path, change)
@pytest.mark.asyncio
async def test_zim_subscription_follows_a_language_change(test_session, saved_downloads):
"""A Kiwix URL changed to another language moves the existing subscription (its name is unique) to the new
Download, with the new language; the old Download is removed."""
french = KIWIX_URL.replace('_en_', '_fr_')
edit_kiwix(saved_downloads, url=french)
config = get_download_manager_config()
config.import_config()
assert config.successful_import is True
subscription = test_session.query(ZimSubscription).one()
assert (subscription.name, subscription.language, subscription.download.url) == ('Wikisource', 'fr', french)
assert KIWIX_URL not in urls(test_session)
@pytest.mark.asyncio
async def test_zim_subscription_download_keeps_its_state(test_session, saved_downloads):
"""Changing a subscribed Download in the file writes only the file's values: its attempts are kept, and the
next import is in step."""
test_session.query(Download).filter_by(url=KIWIX_URL).update(dict(attempts=3))
test_session.commit()
edit_kiwix(saved_downloads, frequency=604800)
config = get_download_manager_config()
config.import_config()
test_session.expire_all()
download = Download.get_by_url(test_session, KIWIX_URL)
assert (download.frequency, download.attempts) == (604800, 3)
with statements() as collected:
config.import_config()
assert writes(collected) == []
@pytest.mark.asyncio
async def test_unsubscribed_download_drops_its_subscription(test_session, saved_downloads):
"""A Kiwix Download the file keeps, but no longer as recurring, loses its subscription; the next import is in
step."""
edit_kiwix(saved_downloads, frequency=None)
config = get_download_manager_config()
config.import_config()
assert test_session.query(ZimSubscription).count() == 0
assert KIWIX_URL in urls(test_session)
with statements() as collected:
config.import_config()
assert writes(collected) == []
@pytest.mark.asyncio
async def test_unknown_kiwix_url_is_imported(test_session, saved_downloads):
"""A recurring Kiwix URL that names no known subscription is imported as a plain Download; it does not fail the
other Downloads."""
unknown = 'https://download.kiwix.org/zim/other/unknown.zim'
edit_config(saved_downloads, lambda data: data['downloads'].extend([
dict(url=unknown, downloader='kiwix_zim', frequency=604800, status='new', last_successful_download=None,
next_download=None),
dict(url='https://example.com/added', downloader='archive', status='new', last_successful_download=None,
next_download=None),
]))
config = get_download_manager_config()
config.import_config()
assert config.successful_import is True
assert {unknown, 'https://example.com/added'} <= urls(test_session)
assert test_session.query(ZimSubscription).count() == 1
with statements() as collected:
config.import_config()
assert writes(collected) == []
@pytest.mark.asyncio
async def test_nothing_to_write_skips_prepare(test_session, saved_downloads, monkeypatch):
"""An import with no differences does not start any preparation."""
prepared = []
monkeypatch.setattr(DownloadManagerConfig, 'prepare', lambda self, diffs: prepared.append(diffs))
get_download_manager_config().import_config()
assert prepared == []