ffmpeg is wrapped in `nice -n19` so a multi-hour transcode never competes
with the API, downloads, or other background work.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A repr runs inside log calls. Reading a relationship or an expired column there runs a query, and
on an object whose session is gone it raises DetachedInstanceError instead of logging. Archive's
repr also stat'ed its SingleFile file on disk.
`wrolpi.common.loaded()` follows attributes through state that is already loaded (primary keys
from the identity key) and never runs SQL. Every model repr uses it; with everything loaded the
output is unchanged, except Video and Archive now show the FileGroup's primary path.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
`delete_duplicate_videos` kept whichever duplicate had the largest
FileGroup and deleted the rest. A leftover yt-dlp format file (video only,
or audio only) is often the largest, so it could be kept while every
playable copy was deleted. The deleted copies' `viewed`, `progress` and
`position` were also lost; only tags were carried over.
Rank copies by completeness first (has audio, has video, from the stored
ffprobe data; unknown counts as complete so no ffprobe runs inside the write
transaction), then by size, keeping the oldest of equals as before. The kept
copy takes the most recent view, and the resume position of the most
recently viewed duplicate when it has none of its own.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A merge that never finished leaves `<stem>.f313.webm` (video-only) next to
where the merged file belongs. A later download of the same video globs
every `<stem>.*` file into its FileGroup, but only `.part` files and format
files with the *same* extension as the merged video were deleted. A `.webm`
leftover beside a new `.mp4` survived, joined the new FileGroup, and could
become the Video's file.
Delete every `<stem>.fNNN.*` file in the group, whatever its extension, which
also covers WROLPi's poster/ffprobe sidecars of an indexed leftover and
yt-dlp's `.ytdl` state files.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
The total for an archive search joined `archive LEFT JOIN file_group` to the FTS table. A LEFT JOIN
fixes `archive` as the outer table, so SQLite scanned every Archive and ran the MATCH once per row:
about 20 seconds of CPU on a Pi with 128k Archives, every time the 60-second totals cache expired.
An inner join lets the planner start from the FTS matches (the same count in milliseconds). Every
Archive has a FileGroup, so the count is unchanged.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Viewing an archive or a doc marked it viewed on the request's read transaction. SQLite cannot
upgrade a read to a writer once another connection has committed (during a download or refresh),
and it does not wait on busy_timeout for that. The archive page returned a 500; the doc page
returned 200 but its view was lost when the request's commit failed.
Build the response on the read, then record the view in `request.ctx.db.write()`, as the video
page already does.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* File downloads: return plain values from upsert_file
Every file download failed after the file was saved, with DetachedInstanceError reading
`fg.location`. `upsert_file` returned a FileGroup from a session it had committed, and the
caller read it after the session was gone. SQLAlchemy 1.3 kept that session alive in a
reference cycle until the next garbage collection, so the read worked by accident; 2.0 frees it
as the `with` block ends. Failed downloads retry, re-downloading their files.
`upsert_file` now returns `UpsertedFile(file_group_id, location)`, read inside its session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Archive uploads: return plain values from singlefile_to_archive
Same failure as upsert_file: `singlefile_to_archive` returned an Archive from a committed session,
and the upload handler read `archive.file_group.title/url` and `archive.location` after the
session was freed, so every SingleFile upload raised DetachedInstanceError after the Archive was
saved. It now returns `UploadedArchive(file_group_id, name, url, location)`, read inside its
session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A RequestSession no longer expires loaded objects at commit, so a handler serializes what it
committed without a reload query. An AsyncSession cannot run that reload at all, so this is
groundwork for moving request handling to async. Background sessions (get_db_session) keep
SQLAlchemy's default: they change data outside the ORM (raw SQL, triggers) and rely on the reload.
Collection, CollectionItem and Channel dict() now run inside the serializer guard. Fixes it found:
- Channel.dict() no longer loads the deferred yt-dlp catalog (info_json), which the channel
handler loaded only to discard.
- get_channel eager-loads what Channel.dict() reads (populate_existing replaces a refresh query).
- create_channel / get_or_create_download link through relationships, so a loaded
Collection.downloads stays current.
- Setting a Repo to Never removes the deleted Download from the loaded collection; the response
reported the old frequency.
- The downloader no longer calls channel.dict() to preload a Channel; its callers read it inside
the session.
Tests run each request with expiry off, as production does, and turn it back on for the background
work that shares the test Session.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
GET /api/archive/<id> loaded each history Archive's FileGroup and Tags with
their own queries while serializing: two queries per history Archive. An
Archive with 51 history entries ran 107 statements per request. Both the
Archive and its history now use Archive.json_options(), so the count no
longer grows with the history (5 statements for the same Archive).
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
SQLAlchemy 1.3 has been end-of-life since 2021, and alembic was held at 1.10
because newer releases need SQLAlchemy 2. This is a mechanical port; the code
stays synchronous.
- SQLAlchemy[asyncio]==2.0.54, alembic==1.20.0.
- Model annotations use Mapped[...] (2.0 rejects bare annotations on mapped
attributes); declarative_base moves to sqlalchemy.orm.
- Plain SQL strings go through text() or exec_driver_sql; schema_ddl.execute_sql
accepts a SQLAlchemy Connection or a raw sqlite3 connection, so rebuild_fts
works from the baseline migration again.
- Session.get replaces Query.get; session.in_transaction() replaces
session.transaction; cache_ok on MediaPathType and FancyJSON.
- The tag-directory sync query joins through TagFile's relationships instead of
the legacy two-entity outerjoin.
- Section-hint rows use attribute access (2.0 rows are tuples).
- Migrations run on an AUTOCOMMIT connection, matching 1.3, where each
statement committed on its own. 2.0 autobegins, so the unique-stem
migration's read snapshot could not become a writer after its helpers wrote
on another connection ("database is locked"). The begin listener emits no
BEGIN for AUTOCOMMIT connections.
- Newer alembic compares column types: WrolpiKV now describes the TEXT table
its migration created.
- The test database installs its DDL inside engine.begin(); 2.0 connections
no longer autocommit, so the FTS tables were rolled back.
- Dependabot now holds SQLAlchemy at 2.0 instead of 1.3.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
CircleCI is gone, so the skip and its flag (CIRCLECI → IS_CI) are named for
CI in general, and the flag reads only GITHUB_ACTIONS.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
test_ownership.py's git() helper did not use TEST_GIT_ENV, so its commits
read the machine's git identity. CI runners have none and cannot guess one,
so seven tests failed there on every run.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Opening a doc on its page never set FileGroup.viewed, so docs were missing
from Recently Viewed. GET /api/docs/:id now marks the doc viewed, with the
same skip_viewed parameter as archives and videos.
The file preview marked a file viewed more than once: opening from a
?preview= URL sent two tracked requests, and every tag change re-fetched the
file with tracking on. The fetches now skip tracking, and the preview marks
each opening viewed once.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Count Channels with any of the Tags in the Other search estimate
The Other tab lists Channels with any of the selected Tags, but its estimate
only counted Channels with the first Tag, so the two disagreed when several
Tags were selected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Search Channels by name in the Other search tab
The Other tab only listed Channels with the selected Tags, so typing a
Channel's name found nothing. Channels now match the search text by name,
and the Tags when there are both. The list and its estimate share one query.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Add Domains to the Other search tab
Domains match the search text by name and/or any of the Tags, like Channels.
The list and its estimate share one query.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Add Playlists to the Other search tab
Domains and Playlists are Collections, so the Other tab searches both with
the existing /api/collections/search (by kind) instead of a Domain-only
endpoint. The search and the estimate share search_collections_query. The
search now loads each Collection's Downloads up front instead of once per
Collection.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Repos keeps local mirrors of git repositories, cloned over https and kept up to date on a schedule.
- A Repo is a Collection (kind='repo') plus a `repository` row. The new `git` downloader clones it, then updates it
(weekly by default, or Never: the clone is kept as it is and only updated on request).
- Repos are saved in `repos/<tag>/<name>` (the Repos Directory, ignored by default) or a directory the user chooses.
An existing clone can be imported where it is. repos.yaml is the source of truth.
- A GitHub-like page shows each repo's README, files, and history, with a ZIP download. READMEs are searchable
under the global search's Other tab.
- git is hardened against hostile repos and configs: https only, pinned addresses with no private addresses (for the
repo and its submodules), sanitized configs, no hooks or links, and only clones WROLPi made or imported are
updated.
- Tagging a Repo moves its clone as one directory. Repo changes wait for repos.yaml to be imported at startup.
- The file mover groups unindexed files with no primary file as one FileGroup, and never replaces a file at the
destination. A failed tag move restores the Collection's tag. A Collection's directory cannot leave the media
directory.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* collections: refuse playlist changes until playlists.yaml is imported
While WROLPi starts, the API accepts requests before every config is imported. A playlist created then could not be
saved, and the import which followed deleted it as "no longer in config" (and rebuilt every playlist's items from
the config, undoing any change). Playlist changes now wait for the import (503, "still starting").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* collections: refuse domain changes until domains.yaml is imported
A domain created, changed, tagged or deleted through the API before domains.yaml was imported (e.g. while WROLPi
starts) was undone by the import. These now wait for the import (503, "still starting").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* archive: importing domains.yaml keeps domains which have archives
Archives create their domain (e.g. one uploaded, or found by a refresh, while WROLPi starts, before domains.yaml is
imported). The import deleted every domain missing from the config, and the database deleted that domain's Archives
with it. A domain with Archives is now kept (and saved to the config with the next change).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* videos: refuse Channel changes until channels.yaml is imported
A Channel created through the API before channels.yaml was imported (e.g. while WROLPi starts) could not be saved,
and the import which followed deleted it, and its Downloads, as "not in the config". Creating, changing, tagging and
deleting a Channel (through the Channel or Collection API) now wait for the import (503, "still starting").
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* db: serializers read only loaded data
Serializing a model must not query. A lazy load there is a hidden query per row (search serialized 24 rows
with 55 statements), and would be a `MissingGreenlet` error under an async Session.
- `wrolpi.db.no_db_access(reason)` and `@serializer` (on every model `__json__`, and around `json_response`):
an engine `before_cursor_execute` listener raises `UnexpectedQuery` during tests, and in production logs once
per serializer and lets the query run. Tests record each trip as well, and the autouse
`no_unexpected_queries` fixture fails a test even when the code caught the error (Video's serializer was
swallowing its per-row loads of the deferred `ffprobe_json`).
- Relationships serializers read load with their parent: `selectin` for FileGroup.tag_files and
Collection.downloads; `joined` for TagFile.tag, Collection.tag, Channel.collection, Video.channel,
Archive.collection, ZimSubscription.download, CollectionItem.zim, and the file_group of Video, Archive, Doc,
Zim and CollectionItem.
- Search and the Video lookups the video page uses load the deferred `ffprobe_json` (`undefer`).
- `wrolpi/test/test_serializers.py`: every serialized model, freshly loaded, serializes without SQL.
Search: 55 -> 5 statements for 24 videos (31 -> 5 for archives), and a few ms faster locally.
Not in this change: guarding the `dict()` serializers and `expire_on_commit=False`, which go together (about 50
serialize-after-commit sites, and 40 tests that read values changed elsewhere after a commit).
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* db: serializer loads per query; the guard ignores transaction control
The query guard reported the engine's own BEGIN (emitted through the same event as the first statement on a
Session with no open transaction) instead of the lazy load after it, and in production that BEGIN used up the
one warning per serializer. Transaction-control statements are now ignored.
Mapper-level eager loads reached background code: every Video/Archive load in the modelers (one per file,
inside the write transaction) joined file_group/channel/collection/tag and selectin-loaded the Collection's
Download rows with their yt-dlp info_json. A second refresh of 100 videos took 0.40 s instead of 0.16 s.
Serializer needs are now declared per model (`json_options()`) and added only by the queries whose results
are serialized; Download.info_json is deferred there (no serializer reads it). Mapper defaults keep only
narrow joins nearly every use needs: TagFile.tag, Collection.tag and Channel.collection. A second refresh is
back to master's statement count and time; search still drops from 55 to 3 statements.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Deleting a Zim cascade-deletes its TagZimEntry rows, and the next tags
save writes them out of the config, so every outdated-Zim delete lost
its entry tags. The config-import migration only runs at startup, too
late to help. Move each outdated Zim's tags to the current Zim of the
same name before deleting, then save the tags config.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* Reset file worker status when a synchronous refresh is cancelled
refresh_sync runs inside request handlers. When Sanic cancelled one on
response timeout, the phase flag cleared as its `with` block unwound but
the shared status did not, so the worker reported 'modeling' forever while
nothing ran. Reset to idle on cancellation.
Deleting outdated Zims also ran the global modelers inside the request,
which is what timed out. Skip post-processing there and update the
outdated_zims flag directly, the only cleanup step it relied on.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* tests: observe the modeling flag in the cancelled refresh_sync test
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* db: request-path library functions use the request's connection
Library functions an API request reaches now take the request's Session (first argument) instead of
opening their own session or cursor, so a request needs one SQLite connection. Raw SQL runs through the
new `wrolpi.db.session_curs(session)`, a cursor on the Session's connection; `RequestDB.curs()` uses it.
- Search: files, videos, archives, flasher, and docs search share the request connection for the total,
the page of ids, and the page's models (3 connections -> 1).
- Zim search, including tag-filtered search and estimates (2 -> 1).
- Channel detail (`Channel.get_statistics` reads on the Channel's own Session) and the channel list.
- Video detail, edit, description, captions, comments and transcode. `get_video_for_app` and
`update_video` no longer commit; their handlers use `request.ctx.db.write()`, so editing a Video and
reading it back is one transaction.
- Tags (list, overlaps, recent), file statistics and estimates, other estimates, bulk tag preview, the
download list, retry, kill and the status summary, and docs/videos/archive statistics.
Not converted: file operations that refresh files inside the request (delete, rename, upload, channel
and collection tag/move) commit page by page in code shared with the file worker; the cached zim entry
lookup; config import.
The connection-count tests now assert one connection for every endpoint they cover (no xfails left), and
cover the videos and archive statistics endpoints, which had no API test.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* downloads: startup retries in a write session of its own
start_single_tasks still called retry_downloads() without a Session after it began requiring one, so any
start after a completed refresh raised TypeError there: incomplete downloads were not reset and the
download_on_startup enable after it never ran. Startup is not a request, so
DownloadManager.retry_downloads_on_startup() opens and commits its own short write transaction.
Tested through the method, not start_single_tasks: importing main registers its startup listeners on the
shared app, and every later test in that worker then runs them.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* db: one request-scoped database handle per API request
Every API request now gets `request.ctx.db` (`wrolpi.db.RequestDB`): a Session plus raw cursors on that
Session's connection, so a request needs at most one SQLite connection. Creating it does no I/O; requests
that never touch the database never connect. `request.ctx.session` remains the handle's Session.
A `RequestSession` binds itself to one lazily-opened Connection. An engine-bound Session releases its
connection at every commit, and NullPool closes it, so a request that read and then wrote reconnected.
Handlers write in `request.ctx.db.write()`: BEGIN IMMEDIATE, committed when the block exits, so the
write lock is not held for the rest of the handler or by background work it starts. This replaces the
handler-level `get_db_session(commit=True)` blocks (download create, collection item add/remove/reorder,
channel create). The zim search handlers use the request's Session in their worker thread.
Tests count the connections a request opens with production's per-call sessions
(`count_request_connections`). Endpoints whose library functions still open their own connection are
strict xfails, to be removed as those functions take the request's session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* tests: name the real extra connections in the request connection xfails
Channel detail: get_channel already uses the request session; the extra connection is
Channel.get_statistics opening a raw cursor (get_db_curs). Search: count_file_groups opens a
cursor for the total, then handle_file_group_search_results opens a cursor and a session.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
cachetools 6 removed cachetools.func.mru_cache; the Zim search estimate
cache was its only user, so it becomes an lru_cache and the pin moves
to 7.2.0 with the Dependabot ignore removed.
reportWebVitals still destructured the web-vitals 2.x names (getCLS,
getFID, ...) which no longer exist, so it silently did nothing. Use
the on* names, replace FID with INP, return the import promise, and
accept any callable rather than instanceof Function.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
The listing still read every channel row (for the summary columns) and every download
row of the listed Collections (for the minimum frequency). Both tables store yt-dlp
info_json, so rows are tens of KB and each sits on its own scattered page; selecting
fewer columns did not help because the seek per row is the cost. From a cold disk
cache that was 5 s per table, 10+ s total, on a large library.
Two covering indexes, channel (collection_id, video_count, total_size) and
download (collection_id, frequency), answer both queries without touching a row. The
listing no longer loads Download objects at all; it computes min_download_frequency
with one grouped query and drops the unused `downloads` array from listing rows (the
detail endpoints keep it; the list views only read min_download_frequency).
Adds a models-vs-migrations test (alembic check) so an index or column added to a
model without a migration, or vice versa, fails the suite.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
A video's audio stream can be dropped the same way its video stream could
already be: a new REMOVE_AUDIO audio target maps only the video stream with
-an, needs a video container, and is refused for audio files. Output
verification insists the result has no audio stream.
Removing both streams is refused by the validator, the API, resolve_output,
and the modal: choosing Remove for one stream resets the other to Keep and
withdraws the other's Remove option.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
ffmpeg wrote `<stem>.transcode.<container>` next to the source. A refresh
that overlapped the transcode discovered that file, probed the half-written
output (errors in the log), and created a FileGroup and Video for a path the
final rename removed moments later.
The temporary file is now `.<stem>.transcode.<container>`: every discovery
path already skips hidden files, and staying in the same directory keeps
the rename over the final path atomic. `transcode_tmp_path` is the single
place the name is built.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* files: shared modeler batch loop for video, doc, zim, archive
Paging, skip-ids, progress, and cancel yields live in
wrolpi.files.modeler.run_modeler_loop. Each module still owns select /
prepare / apply and its transaction style: video probes outside the write
lock, docs use one write per file with lock retries, zim and archive use
one write per page. Archive HTML that is not a SingleFile raises
SkipModeler so apply_indexers can claim it.
* files: quieter batch-modeler errors; drop unused imports
Break out of a batch page on InvalidRequestError after a flush failure
so the rest of the page does not each log the same poisoned session.
Re-raise PYTEST apply failures without a misleading commit-failure
message. Remove asyncio/List/OperationalError/PYTEST/Iterable leftovers
from docs, archive, and zim.
* api: run the perpetual loops in one Sanic-managed process
The singleton background loops (file worker, downloads, switches, jobs,
bulk tag, flag checks, update check) now run in a dedicated process that
Sanic's Worker Manager owns: `run_perpetual_process` is registered with
`app.manager.manage()` from `main_process_ready`. The manager guarantees
a single instance and SIGTERMs it on shutdown. No server worker runs the
loops, so the owner-pid / heartbeat / watchdog claim scheme is deleted
rather than hardened.
`perpetual_signal` keeps its name and is now a plain registrar. A new
`per_worker_task` registrar covers loops every server worker must run for
itself: syncing its log level (previously only the owner did, so the other
workers never picked up level changes) and a health check that asks the
manager to restart the perpetual process when it is FAILED (Sanic marks a
dead managed process but does not restart it).
Downloads are started as background tasks instead of Sanic signals; the
perpetual process has no server, so no finalized signal router. The
update check and the upgrade-check endpoint run git in a thread via
`refresh_update_status`. /api/status lists the perpetual process next
to the servers.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* perpetual: survive a predecessor's death, exit cleanly, coordinate restarts
Review fixes for the managed perpetual process.
Shutdown: cancel_background_tasks / cancel_refresh_tasks gather with
return_exceptions, so a cancelled child's CancelledError no longer aborts
the shutdown that called them (the process exited 1 with a download in
flight).
Restart after a crash: reconcile_after_predecessor heals shared locks a
dead process left held, fails RUNNING Jobs owned by a dead pid, clears
processing_domains, renews pending Downloads, and resets the file-worker
status before the loops start. N server workers issue one restart
request per cooldown through a shared timestamp in shared_ctx.
The log-level sync now also runs in the perpetual process. Manage() is
registered through register_perpetual_process so it is testable; the
entry point refuses to run with no loops registered (spawn).
Tests exercise the real entry point in a forked process with a live
background task under SIGINT and SIGTERM, Sanic's real WorkerManager
marking a crashed managed process FAILED and reviving it on restart,
Sanic's process naming and ProcessState enum, the listener registration,
and the reconcile step.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* perpetual: import the jobs helper inside its reconcile step
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* files: SQL FileGroup compare, stem column, drop unused discovery path
Refresh walked the tree twice (find, then stat every unchanged group),
and a two-directory refresh scanned the whole media directory. Compare
now diffs in SQL on a TEMP table that already has mtime, accepts multiple
roots, and groups by a stored stem that is versioned so a suffix-rule
change cannot delete-and-reinsert tagged files.
Deletes refresh_discover_paths and /api/files/refresh_progress, which
nothing in production called. Video/doc/zim modelers use the same
(no domain row OR indexed != True) gate.
* files: keep stem current on reorganize; NUL-safe find parse
Reorganize wrote the new primary_path but not stem, so the next refresh
treated the FileGroup as deleted+new and dropped tags. GNU find now
emits NUL-delimited records so a tab or newline in a filename cannot
abort a global refresh. Stem backfill skips the table scan once the
algorithm version matches, and runs off the event loop. An empty roots
list is a global refresh, not invalid SQL.
POST /api/videos/channels failed with "database is locked" on
`INSERT INTO collection` on the docker WROLPi while background workers
(config saves, refreshes) were writing. The handler wrote through the
deferred request session: `create_channel` reads (the URL/name/directory
conflict check) and then INSERTs, and SQLite refuses that mid-transaction
lock upgrade instantly when another connection wrote in between --
busy_timeout is skipped for upgrades.
Same shape and same fix as POST /api/download (daf2ba39): the handler
now uses `get_db_session(commit=True)`, which begins IMMEDIATE so the
concurrent writer waits its turn, and which commits before the refresh
is fired so the refresh does not wait on a lock the request still holds.
The regression test commits a write from a second connection between
the handler's conflict check and its INSERT. The existing
`write_lock_held_briefly` helper cannot reproduce this: the Sanic test
client takes over a second to start the app, so its lock is released
before the handler runs. Before the fix the test fails with the
production error verbatim.
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* Jobs queue and video transcode from the video page
Add an in-memory FIFO Job queue (wrolpi/jobs.py): functions register by
name, callers enqueue by name with JSON kwargs, and the perpetual-tasks
owner runs them one at a time. Logging emitted inside a Job and the
output of its subprocesses are captured into a per-Job log; Jobs report
progress and can be cancelled (a running Job's subprocess is killed).
Jobs are not persisted; an API restart forgets them.
Add a transcode Job and POST /api/videos/<id>/transcode. ffmpeg reports
progress via -progress pipe:1. The existing FileGroup is pointed at the
new file before the refresh so tags and history survive a container
change.
Replace the video page Refresh button with an Edit dropdown (Refresh,
Transcode). The Transcode modal pre-fills from the preferred codecs in
the Videos settings and shows the Job's progress, log tail and Cancel.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video page: move Delete into the Edit dropdown
The Edit dropdown now holds Refresh, Transcode and Delete, so the action
row is Download / Edit / Add to Playlist. Delete keeps its confirmation
(a Confirm dialog) and the tagged-files prompt, and is disabled in WROL
Mode like the other items.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video page: Edit > Edit to change a video's title
PUT /api/videos/<file_group_id> writes the title (and fulltitle) into the
Video's info json, creating the file when the video has none, then
validates the Video again so its title and search text follow. Files
stay the source of truth: the edit survives a refresh, and a re-download
of the metadata replaces it (the modal says so).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video page: edit the description in Edit > Edit
PUT /api/videos/<file_group_id> now takes optional title and description
and changes only the fields given; an empty description clears it. The
modal sends only what changed and refetches the description after saving.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode modal: allow a container-only remux
Keeping both streams is no longer refused: ffmpeg copies them into the
chosen container with fast start. The container select starts at the
file's own container, the hint explains what a remux does, and the button
reads Remux when nothing is re-encoded.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* WROL Mode: refuse video edits and transcodes past the API
The routes already returned 403, but a transcode Job queued before WROL
Mode was enabled still ran, update_video could be called from anywhere,
and the Edit modal's Save button stayed live. The Job now fails before
ffmpeg starts, update_video carries wrol_mode_check, and the modal
disables Save with a notice.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode: verify the output before replacing the original
After ffmpeg exits, the output is probed and compared with the source
(container, presence of video/audio streams, codec per stream, duration
within 1s or 1%) and its last 30 seconds are decoded to a null output.
Any failure removes the output and keeps the original.
The tail decode replaces the download-time completeness check, which
grabbed a single frame near the end: one ffmpeg_video_complete (sync for
the downloader, async for Jobs) now serves both, and is stricter, since
every frame of the tail plus the audio must decode.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Jobs: claim atomically, cap pending, own-session subprocesses; retarget before delete
Review fixes. The worker read a Job's status and wrote RUNNING as two
steps, so a cancel landing between them was overwritten and the Job ran;
_claim_job now does both under the lock. enqueue_job refuses past
JOB_PENDING_LIMIT pending Jobs. JobContext.run_command starts the
subprocess in its own session and raises CancelledError when the command
was killed, so a handler cannot mistake it for a normal exit.
The transcode Job now retargets the FileGroup (committed) while the
original file still exists, and deletes the original afterwards
(transcode_video_file gained keep_original). A crash in between leaves
both files with the database pointing at a real one.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video page: the Edit menu owns the transcode Job; polling survives Close and errors
Review fixes. The transcode Job's id and polling (useJob) move from the
modal to VideoEditMenu, so closing the modal no longer stops the page
from learning the Job finished (and refetching the video, whose path may
have changed). While a Job runs the menu shows Transcoding..., reopening
shows its progress instead of the form, and Delete waits. A failed poll
request is retried instead of ending the polling. getJobs checks the
response like its siblings. Design narration trimmed from comments.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode: audio files, and Remove video to turn a video into an audio file
video_codec 'none' (REMOVE_VIDEO) drops the video stream (-vn, audio
required); an audio-only source is audio-only output whatever the video
target says, and a video target on it is refused. Audio output gets an
audio container (m4a/ogg/mp3) checked against the codec it will hold, and
a video container asked for audio output is replaced by the codec's
natural one. faststart only for mp4/m4a. Verification learned the
audio-only shape and ffprobe's container names (mkv reports matroska).
The modal offers 'Remove video (audio only)', switches to audio
containers and follows the audio codec, reads Extract Audio, and opens
for audio files (no video codec select).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video page: yellow Edit button
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode: remove the temp output on cancel; tell the user when a closed-modal Job fails or is lost
Review fixes. The temp output was only removed on Exception, but a
cancelled Job's run_command raises CancelledError (a BaseException), so a
cancel left the partial file for a refresh to index. Closing the modal
during a Job that then failed showed nothing; it now toasts the error
(and a cancellation). Polling gives up on a 404 or five failures in a
row (the API restarted and forgot its Jobs), warning the user and
unlocking Transcode and Delete instead of reading Transcoding... forever.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Jobs: _claim_job docstring states the invariant, not the old race
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode: fragmented mp4 option for very long videos
A whole-file mp4 index grows with the frame count; for a many-hour video
it reaches tens of MB, which a phone must fetch and parse before the
first frame, and every seek walks it. `fragmented` writes the file with
-movflags +frag_keyframe+empty_moov+default_base_moof+global_sidx and
10s fragments: a ~1 KB moov plus a global seek index, so playback starts
and seeks quickly. mp4/m4a only; a remux, so it takes seconds per GB.
Measured on an 11-hour h264/aac file: moov 40 MB -> 1.4 KB, seek index
2 x 120 KB, 10k fragments, duration and codecs unchanged, remux in 25s.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Transcode modal: the Fragmented label says audio for audio output
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Video edits keep the original: custom_title/custom_description in the info json's wrolpi section
Editing a title or description no longer overwrites yt-dlp's keys. The
values go into the info json's `wrolpi` section (custom_title,
custom_description); the modeler prefers them when deriving the Video, so
a refresh keeps the edit while the original title and description stay in
the file. An empty custom description means none.
Both re-download paths carry the wrolpi section over: the metadata-only
refresh merges it into the fresh info json, and a full re-download
snapshots it before yt-dlp overwrites the file and restores it (then
re-validates) in finalize_download. FileGroup.get_wrolpi_json reads it.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* docs: browse walks the file_group date index instead of probing every doc
The unfiltered /api/docs/search drove its join from `doc`, so SQLite probed
every doc's file_group row for the sort key before returning the first page.
With 20k docs scattered through a 10 GB database on a spinning disk, the first
request took about a minute cold (0.07s warm).
Use the same CROSS JOIN shape as the archive browse: file_group is the outer
table, SQLite walks file_group_published_datetime_idx and probes doc's covering
unique index on file_group_id. Both sides stay index-only and the scan stops at
the page limit. Measured cold: ~60s -> 0.6s, identical rows.
Filtered searches and the size/title orderings keep the ORM query.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* docs: trim browse comments, stabler plan assertion
Review feedback: drop the benchmark narrative from docstrings, state only the
non-obvious constraint at the call site, and assert the doc covering-index probe
by shape rather than by autoindex name.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
* zim: recompute the outdated-Zims flag after startup and on Delete
The Dashboard shows the "Outdated Zim Files" banner from the persisted
`outdated_zims` flag, but the modal lists files from a live scan. The two
disagreed: the flag was restored from the DB on startup and never
recomputed once the outdated file had been removed, so the banner showed
with nothing to delete, and the modal displayed an endless placeholder.
- The startup recompute now runs after the DB is up and configs are
imported, so it overrides the value `init_flags` restored from the DB.
Previously it ran before the DB was up, and the restored value won.
- DELETE /api/zim/outdated always recomputes the flag, even when nothing
was deleted, so clicking Delete clears a stale banner.
- The modal re-scans when opened and says when nothing was found instead
of showing a placeholder.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* zim: enforce restore-then-scan ordering; no Kiwix restart on empty delete
- Startup calls flags.init_flags() then flag_outdated_zim_files() right
after db_up, before the DB config import, so the disk scan always
overwrites the DB-restored value in one local sequence.
- DELETE /api/zim/outdated recomputes the flag in a finally.
- remove_outdated_zim_files skips refresh and the Kiwix restart when no
file was deleted.
- Trim narrative comments to the constraint they enforce.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
COUNT(*) OVER() forced SQLite to visit every matching file_group
before LIMIT, which is a full table scan of the (large) file_group
table — ~22s cold on a Pi for videos ordered by download date.
Count separately and cache for 60s by filters so indexed orders can
stop after one page. Past-the-end pages now report the true total.
Videos/archives/docs pagers keep totalPages across page changes.
* videos: codec download preferences with transcode and strict-fail options
Add video_codecs/audio_codecs ordered preference lists to the video,
channel, and RSS downloaders, alongside the existing resolution
selector. Codec preference is codec-major: a preferred codec at a
lower resolution beats another codec at a higher resolution.
When 'transcode' is enabled, a downloaded file whose streams do not
match the preferences is re-encoded with ffmpeg after download (only
the mismatching stream; targets are extensible maps, currently h264
and aac/opus/mp3). When 'strict_codecs' is enabled and transcode is
not, a download with no matching format fails permanently instead of
falling back.
Empty codec lists produce a byte-identical yt-dlp command to before.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* videos: honor strict_codecs when transcode cannot produce the preference
Review fixes:
- strict_codecs is now suppressed only when transcoding can produce a
preferred codec for every non-empty preference list (new
transcode_can_satisfy_codecs); av1-only preferences with transcode
enabled fail under strict instead of silently keeping the file.
- No partial transcode: an unfixable video mismatch blocks an
audio-only conversion (and vice versa); the mismatch falls through
to strict/keep.
- fsync the transcode output before rename, and the directory after,
so power loss on a Pi cannot leave a truncated final file.
- mp3 yt-dlp pattern matches mp4a.6B as well as mp4a.6b.
- ffmpeg maps use 0:V:0 (excludes attached pictures) and 0🅰️0?
(tolerates missing audio).
- Strict toggle is no longer disabled by transcode in the UI; popup
wording corrected.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* videos: serialize transcodes machine-wide via shared_ctx lock
Sanic runs multiple worker processes, so an asyncio primitive cannot
serialize transcodes across download workers. A multiprocessing.Lock
in shared_ctx now guarantees only one ffmpeg transcode runs at a time
on the machine; waiting workers poll the lock asynchronously so the
event loop stays responsive while a transcode holds it for hours.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* videos: transcode-lock review fixes
- Document that a crashed lock holder requires an API restart; this is
accepted rather than adding owner-death recovery.
- Move the 'Transcoding' user notification to after the lock is
acquired, so it fires when work begins rather than before a
potentially hours-long wait.
- Soften the lock test docstring to what it measures (in-process
wait/release); trim the acquire docstring to the polling rationale.
- Give transcode_lock its own section header in contexts.py.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
test_search_videos_does_not_block_event_loop asserted a wall-clock
threshold (echo < 0.4s), which fails on loaded CI machines from
scheduler noise alone. Replace the timing with ordering: the mocked
search parks its thread on an Event, the echo is sent only once the
handler is provably inside the query, and the test asserts the echo
completes while the query is still held open. Events are only waited
on off the event loop, so a regression fails the ordering assertion
instead of deadlocking.
Verified both directions: passes repeatedly against the real endpoint,
and fails when the endpoint is changed to run the search on the loop.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* Fix API stalls: non-blocking switch worker, threaded searches, indexed Videos page
Three response-time fixes found by benchmarking the read-only API endpoints
hot and cold on real libraries:
* `switch_worker` runs on the Sanic worker's event loop every 0.1s and called
the blocking `multiprocessing.Event.wait(timeout=1)`. With no switch pending
(the normal state) this froze the loop for a second at a time; about a third
of all requests, including trivial ones, took ~1,000 ms. It now polls
`is_set()` and returns immediately.
* The search handlers (videos, files, archives, docs, zims) and `/api/statistics`
ran synchronous SQLite queries on the event loop, so one slow search stalled
every other request on that worker. They now run via `asyncio.to_thread`.
* The unfiltered Videos page (`POST /api/videos/search` with only an order)
used `mimetype LIKE 'video/%' OR ...` plus `COUNT(*) OVER()`, which visits
every video's index entry before LIMIT applies: seconds cold on a large
library on a spinning disk, and ~10 s hot for a deep page. It now seeks each
exact mimetype on the (mimetype, effective_datetime) index for `limit + offset`
rows and merges them, with the total from a separate count cached for 60 s.
Results are identical to the generic query.
Also adds `scripts/api_perf.py` (times every read-only endpoint against a host,
optionally restarting the API over SSH for cold numbers) and
`scripts/api_perf_compare.py` (merges per-host results). Host profiles live in a
gitignored JSON file; an example is included.
Measured on an 8-core docker host with a 10 GB library: median hot request
1,007 ms -> 22 ms; Videos page 1 hot 287 -> 108 ms, page 50 1,345 -> 120 ms.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Serve the video description from its own endpoint instead of Video.__json__
`Video.__json__` computed `description` as `c_text or get_video_description()`,
where the fallback opens and parses the video's info json from disk. The list
endpoints serialize every result with `__json__`, so any page containing videos
without an indexed description (common: many videos have none) read their info
json files on every request, cold, for a field the list never displays. The
description is not part of search either: search runs against the FTS copy of
`c_text`, so the fallback never fed the index.
Drop `description` from `__json__` and serve it from
`GET /api/videos/<id>/description`, like comments and captions. The endpoint
returns the indexed `c_text`, falling back to the info json only there, for a
row indexed before descriptions were stored. The player page fetches it through
`useVideoExtras` alongside the comments.
Tests assert the list and detail responses never touch the info json, and cover
the endpoint's indexed, fallback, and not-found cases.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Address review: thread-local zim sessions, fast-path equivalence test, comments
* Zim search, single-zim search, and search estimates run libzim off the event
loop, each opening its own session inside the worker thread rather than
passing the request's SQLAlchemy Session across threads.
* The Videos fast-path test now runs the same requests through the generic
query and asserts identical rows and paging for both date orders. The one
intentional difference is documented: past the end, the fast path still
reports the true total.
* switch_worker returns before its try block so the empty path does not clear
the event; its comment states the invariant only. The copied "synchronous
SQLite" comments are reduced to one line in one place.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* videos: a collaboration video's channel directory conflict killed the download
YouTube collaboration videos appear in every collaborator's feed but their
metadata names only the primary uploader's channel. When that channel's
computed directory already belonged to an existing Channel that no longer
matched by source_id/url/name, channel creation raised
ChannelDirectoryConflict and the download failed unrecoverably. Now the
channel that owns the directory is used instead of creating a conflicting one.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: trim comments that restated the code
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
NAME_PARSER's suffix whitelist had no audio suffixes (and no mkv/m4v),
so a correctly-named audio file got no published date and sorted to the
very end of its Channel -- which reads as missing. The whitelist now
carries the media suffixes a Video FileGroup can lead with.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* videos: an uploaded audio file never appeared in its Channel
The upload flow asks Video.can_model, which only accepted video/*, so an
uploaded .flac was indexed with no Video row -- and the batch modeler's
`indexed != True` gate then excluded it on every later refresh, leaving
the file permanently invisible in its Channel.
can_model now accepts audio/* like the batch modeler always has, and the
modeler gates on the Video row being missing (mirroring the doc_modeler
fix for the same class of bug) so already-stuck files heal on the next
refresh. Audio playlists (.m3u detects as audio/x-mpegurl) are excluded
from both paths and unmodeled by video_cleanup.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* review: widen the playlist denylist, prove modeler termination
The denylist gains audio/x-scpls and audio/scpls -- .pls detection
varies by libmagic version, and any audio/* playlist type must be
refused wherever it comes back. Modeling one file is extracted as
_model_video so a test can force persistent failure and prove the loop
terminates; failed_ids is recorded before the PYTEST re-raise so no
failure path can re-select an id forever. Playlist mimetypes are bound
through json_each like the rest of video_cleanup, and the comments
state the invariants instead of the history.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test: the playlist test must not depend on libmagic's m3u opinion
CI's libmagic detects .m3u as text/plain where a Mac's says
audio/x-mpegurl. The invariant is that a playlist mimetype is never
modeled by either path, so the test forces the mimetype like the other
denylist tests instead of asserting what detection returned.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
The channel downloader refreshes a Channel's files before downloading,
already passing send_events=False, but the parameter was never plumbed
into the file worker, so every pre-download refresh notified the user.
FileTask now carries send_events and handle_refresh skips the
Refreshing/Refreshed events when it is False. User-requested refreshes
and global refreshes still send Events.
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* ui: a preview map that waited for a container it could never see
Both map preview modals -- the per-region eye button and "View All Regions"
-- opened to an empty box. Each renders a div into a Modal and hands it to
MapLibre from an effect gated on `open`, reading the node out of a `useRef`:
if (!open || !mapContainer.current) return;
Mantine's Modal mounts its body one commit after `opened` flips true, so on
the render that opens the modal the ref is still null. The effect returns,
nothing it depends on changes again, and it never runs a second time. No
console error, no rejected promise, no canvas -- just an empty div.
Semantic UI's Portal mounted the body in the same commit that opened it, so
the ref was attached by the time effects ran. The effect bodies are
byte-identical across that migration; the modal library is the only variable,
confirmed against a device still running the older build where both previews
render.
A callback ref instead, so the effect re-runs when the node arrives and this
stops depending on any modal's mounting schedule. The async source lookup
also gets a cancelled flag: it could resolve after the cleanup ran, building
a map nothing would ever remove and leaking a WebGL context per open.
The tests drive the real page and assert a map was built against a node that
is in the document -- the outcome, rather than the mechanism that quietly
changed underneath it. Seven of the eight fail on the code before this.
* build: docker had no world underneath its map regions
Caddy serves /blobs/map-overview.pmtiles out of /opt/wrolpi-blobs, and that
blob is the world at zoom 0-6 -- what a regional extract sits on top of.
Without it a user who subscribes to a few regions gets those regions floating
on an empty background everywhere their data does not reach. Only a full
planet file, at 134GB, makes it unnecessary.
Raspberry Pi and Debian installs get the blob from pi-gen, the live-build
config, and upgrade.sh. Docker got nothing: the compose file bind-mounted a
git-ignored host directory over /opt/wrolpi-blobs, and a bind mount replaces
the image's directory rather than merging with it, so the path existed and was
empty on every Docker WROLPi. Verified against a running deployment, where
that URL returned 404 while the same URL on hardware returned 206.
So the web image fetches the blob at build time, as the other three builders
do, and the mount that would hide it is gone. `curl -f` so an HTTP error
fails the layer -- an image that builds without the blob is this same bug,
except silent at build time too.
The tests read the deployment files rather than a built container: building
to check costs a CDN download and several minutes, and both things that broke
-- nothing fetches it, a mount hides it -- are visible in the source.
* ui: cancel the load handler too, and make the cancel test falsifiable
Review follow-ups on the preview modals.
The `cancelled` flag was checked before constructing the map but not inside
the `load` handler it installs, so cleanup landing between the two left that
body drawing into a map `remove()` had already torn down. Guarded, mirroring
the `destroyed` pattern in MapViewer.js. The source promise also gets a
`.catch`: the helper cannot reject today, but nothing says it never will, and
an unhandled rejection here would be as quiet as the bug this all started as.
Two test corrections, both cases of an assertion claiming more than it read:
The region-drawing test spied on `addSource` AFTER construction and then
re-fired `load` by hand, so its count came entirely from that manual call and
said nothing about the production path. FakeMap now records the sources it is
given, and the assertion reads what the component's own `load` drew -- each
region once, in order, and none for the entry with no bbox.
The new cancel test passed with the guard deleted, which is worse than not
having it: Mantine had not mounted the modal body when it closed, so the
effect had never run and there was nothing to cancel. It now waits for the
effect to reach the source lookup before closing, and ships beside a control
that releases the same held promise WITHOUT closing and requires a map. The
control is what makes the zero meaningful; verified that removing the guard
fails the cancel test while the control still passes.
* test: do not leave videos_destination as an invalid template
test_format_videos_destination deliberately sets %(channel)s to prove
unknown placeholders fail, then never restored the global WROLPi config.
On xdist workers that later ran channel_factory without async_client
re-init, the polluted template caused KeyError/FileNotFoundError in
test_format_video_filename_uses_channel_name_over_info_json.
* test: stop search_suggestions from flaking empty domains on CI
A different failure than videos_destination pollution: under xdist load,
search_suggestions sometimes returned no domains for a valid query while
channels still matched. Harden the path by:
- running channel/domain/author/subject searches sequentially on the shared
Session (asyncio.gather is not safe with one SQLAlchemy session)
- giving archive_factory a URL when only a domain is set so validate() does
not clear collection_id via apply_domain()
- rewriting test_search_suggestions without hardcoded Collection ids and with
unique names that will not collide with other tests
All three bugs confirmed; the two suggestions taken.
[bug] `video_modeler` awaited ffprobe -- a subprocess per video, up to 20
per batch -- inside its `commit=True` session. With IMMEDIATE as the
default that holds the write lock for the whole batch, which is the same
starvation this branch is curing in `video_cleanup`. (It was already
partly true before: the batch's first `session.flush([video])` took the
lock ahead of the remaining ffprobes.) The batch is now read, ffprobed
with no transaction open, then written. `get_or_create_ffprobe_json` in
modules/videos/common.py is the path-only entry point that makes that
possible; `Video.get_ffprobe_json` delegates to it and keeps appending
the cache file to its FileGroup. `validate()` -- and the poster
generation it can trigger -- is still inside the write transaction, and
now says so.
[bug] `test_download_dispatched_outside_immediate_transaction` probed
`getattr(_immediate_txn, 'active', False)`, which a ContextVar never has,
so it passed unconditionally. It now probes the actual write lock from a
competing connection under `production_like_sessions`. Mutation-checked:
moving the dispatch back inside the claim transaction fails it.
[bug] `asyncio.create_task` snapshots the current Context, so a flag held
for a write session's lifetime was inherited by every task spawned inside
it -- and the parent's `reset()` cannot reach a child's copy, leaving
that task issuing BEGIN IMMEDIATE for its *read* sessions.
`create_downloads` spawns `dispatch_downloads` exactly that way, so every
user-created download hit it. The flag now lives only across the
session's opening `connection()` call, which also means the write lock is
taken when the session opens rather than at the caller's first statement.
[suggestion] `renew_recurring_downloads` scanned every recurring Download
under the write lock, including the usual cycle where nothing is due. It
reads first now and takes the lock only for the rows that change,
re-checking status since the dispatcher may have claimed one in between.
[nit] Parenthesized the directory OR in the claim join.
`probe_write_lock_is_held` moved to conftest alongside
`write_lock_held_briefly`; three suites use it now.
Suite: 1712 passed, 5 skipped.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three "database is locked" failures on the docker WROLPi, one cause each:
* `INSERT INTO download` from POST /api/download -- a user creating a
video download while a refresh ran (2026-08-06 14:06:46);
* six `INSERT INTO file_group` from `upsert_file`, uploads landing in
the same three-minute window;
* `UPDATE download SET status` from `renew_recurring_downloads`.
All three are the deferred read-then-write trap: a transaction that
begins as a reader and upgrades at its first write is refused
*instantly* when another connection wrote in between, because
busy_timeout is skipped for lock upgrades. `get_immediate_db_session`
existed to avoid it, but an opt-in helper only works if every author
remembers, and read-then-write is the ordinary shape of a write. So it
is gone: `get_db_session(commit=True)` -- the existing declaration of
write intent -- now begins IMMEDIATE. Rails 7.1 and Django 5.1 made the
same call, defaulting SQLite transactions to IMMEDIATE rather than
asking application code to opt in.
That default is only safe if write transactions stay short, and one did
not. `video_cleanup` ran its whole Channel-claiming join --
`UPDATE video ... FROM channel ... LEFT JOIN file_group`, O(channels x
file_groups) -- inside one write transaction: 110 to 130 seconds each of
the eight times it appears in the log, on a library with ~1500 channels.
While it ran, *every* writer on the box failed, including ones that
began IMMEDIATE, because the hold far exceeds the 30s busy_timeout. The
14:04-14:06 failures above sit inside its 130-second window. The join
is now a read -- WAL readers never block writers -- and only the
resulting UPDATEs take the lock, 500 rows per transaction.
`claim_videos_for_channels` had the same join and the same fix; channel
import calls it with every channel in channels.yaml.
Also here:
* `renew_recurring_downloads` was a read session with an inner commit;
it is a write session now. Its computed `next_download` values now
persist even when nothing was renewed -- that is the point of
computing them.
* POST /api/download no longer writes through `request.ctx.session`.
That session is committed by the response middleware, so a writer
would hold the lock across the whole request; a write session
commits before the handler returns, ahead of the background dispatch
it fires.
* `_immediate_txn` is a ContextVar, not a threading.local. The
writers that set it include coroutines, and one asyncio thread runs
many of them interleaved.
* `write_lock_held_briefly` moved from the move tests into conftest,
since two suites now need it.
Both regression tests were verified by mutation: reverting either site
alone fails its own test, and the API one fails with the production
error verbatim.
Sources: fractaledmind.com/2023/12/11/sqlite-on-rails-improving-concurrency,
tenthousandmeters.com/blog/sqlite-concurrent-writes-and-database-is-locked-errors,
berthub.eu/articles/posts/a-brief-post-on-sqlite3-database-locked-despite-timeout
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Move the flush to immediately before FileGroup.move so it covers every
deletion queued in this function, not only the one found by
get_by_path. The earlier missing-file deletions were already reaching
the DB -- the get_by_path query autoflushes them -- but that is an
incidental property of the query sitting between them and the rename,
not something the rename should depend on.
Also assert the kept Video's old path is gone, so the test reads a move
rather than a copy.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`delete_duplicate_videos` deletes the smaller copy sitting at the
destination path, then renames the copy it keeps onto that path.
`FileGroup.delete` unlinks the files right away but only queues the row
deletion on the session, and SQLAlchemy emits UPDATEs before DELETEs
within a flush -- so the rename's UPDATE of `primary_path` collided with
the row that was about to be deleted:
sqlite3.IntegrityError: UNIQUE constraint failed: file_group.primary_path
The whole download transaction rolled back, so a re-downloaded video
lost its DB record while its files stayed on disk (and the previous
copy's files were already unlinked).
Flush the delete before claiming the path. The earlier missing-file
deletions in the same function never hit this because the `get_by_path`
query between them autoflushes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The yt-dlp `-f` selector was built by joining every selector of every
requested resolution with a comma. In yt-dlp a comma means "download all
of these formats"; `/` is the fallback operator. The resolution map is
ordered best-to-worst and its entries already use `/` internally, so the
comma was never the intent.
With the default resolutions this handed yt-dlp ~45 comma-separated
selectors and asked it to download every one that matched. A 720p video
matched 8 of them (including duplicates, which yt-dlp does not dedupe), so
the whole per-download pipeline ran 8 times against the same output path:
subtitles downloaded, thumbnail downloaded, info.json written, subtitles
converted, thumbnail converted, existing subtitles deleted, repeat. The
duplicate work was invisible because each pass overwrites the last.
Join with `/` so the resolution list behaves as the preference chain it
reads as.
modules/inventory/common.py: _normalize_items now strips leading/trailing whitespace from any string-valued item field before persisting an inventory (name, category, subcategory, location, etc.). Non-string values (numbers, None) are left untouched.
Closes#52
Triage of Greptile findings on the default-Zim / configurable-directory PR:
- Stale config on restart (P1): settings update() only queues an async
background_save, and the restart switch has no ordering guarantee (the switch
worker pops LIFO), so start_kiwix_serve.sh could read the old zims_destination.
Save the config synchronously before activating the restart switch.
- YAML null (P1): read_config_value.sh now treats an unquoted null/~ as unset
(returns the default), matching PyYAML/the Python config layer. A quoted
"null" remains the literal string.
- Path traversal (P1/security): the settings API now rejects a zims_destination
containing "..", and start_kiwix_serve.sh defends in depth by falling back to
`zims` for any absolute/traversal/empty value, so Kiwix can never scan or
rewrite library.xml outside the media root.
- Reproducible build (P2): pin the zim-tools image to 3.7.0 instead of :latest.
Tests added for null handling, traversal rejection (API + script).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two related gaps in how WROLPi serves Kiwix content:
1. A fresh WROLPi (or one whose zims are all corrupt/absent) showed a blank
Kiwix. Every image now ships a small built-in default Zim that Kiwix falls
back to, telling the user to download real Zim files.
2. The startup scripts hardcoded the `/zims` directory, ignoring wrolpi.yaml's
`zims_destination` (which the Python side already honors).
Changes:
- New wrolpi/scripts/read_config_value.sh: a zero-dependency (awk) reader so
non-Python scripts can reliably read top-level wrolpi.yaml scalars. Also a
config_value() wrapper in wrolpi/scripts/lib.sh.
- New wrolpi/scripts/default_zim/: HTML source + a reproducible builder
(ghcr.io/openzim/zim-tools) that outputs the committed wrolpi/blobs/default.zim.
- New wrolpi/blobs/: any committed blob here is copied to /opt/wrolpi-blobs by
the pi-gen and debian-live builds; the Docker zim image bakes it in.
- start_kiwix_serve.sh honors zims_destination and falls back to the default
Zim when no functioning zim is found.
- docker/zim/entrypoint.sh gains the same default-Zim fallback. It needs no
config read: it already scans the whole mounted media dir, and
zims_destination is always a subdirectory of that mount.
- Changing zims_destination via the settings API now restarts Kiwix so it
serves the new directory without a reboot.
Tests: new test_read_config_value.py, extended test_start_kiwix_serve.py
(configured dir + default-Zim fallback), and a root_api test for the restart.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
