From dab333124e16353621818a0abbfe3b513d5d9aba Mon Sep 17 00:00:00 2001 From: lrnselfreliance <33263708+lrnselfreliance@users.noreply.github.com> Date: Fri, 9 Oct 2026 15:08:32 -0600 Subject: [PATCH] Upgrade to SQLAlchemy 2.0 (synchronous) (#803) 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 --- .github/dependabot.yml | 9 +- alembic/env.py | 5 +- modules/archive/__init__.py | 3 +- modules/archive/models.py | 6 +- .../archive/test/test_collection_id_bug.py | 2 +- modules/docs/lib.py | 13 +- modules/docs/models.py | 4 +- modules/videos/downloader.py | 2 +- modules/videos/models.py | 7 +- modules/zim/models.py | 24 ++-- requirements.txt | 4 +- wrolpi/collections/models.py | 11 +- wrolpi/collections/test/test_reorganize.py | 134 +++++++++--------- wrolpi/common.py | 2 +- wrolpi/conftest.py | 8 +- wrolpi/db.py | 14 +- wrolpi/downloader.py | 4 +- wrolpi/files/modeler.py | 2 +- wrolpi/files/models.py | 23 +-- wrolpi/files/test/test_lib.py | 7 +- wrolpi/fts.py | 12 +- wrolpi/media_path.py | 1 + wrolpi/schema_ddl.py | 14 +- wrolpi/tags.py | 10 +- wrolpi/test/test_db.py | 9 +- 25 files changed, 175 insertions(+), 155 deletions(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 785d2aee..29480843 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -8,13 +8,10 @@ updates: target-branch: "master" open-pull-requests-limit: 5 ignore: - # SQLAlchemy 1.4 is the 2.0 transition release and is not a drop-in - # change for this codebase; alembic >=1.11 requires SQLAlchemy 2. - # Hold both until the SQLAlchemy 2 migration lands. + # SQLAlchemy 2.1 is a minor release with its own deprecations; upgrade it + # deliberately rather than by an automatic bump. - dependency-name: "SQLAlchemy" - versions: [">=1.4"] - - dependency-name: "alembic" - versions: [">=1.11"] + versions: [">=2.1"] groups: python-dependencies: patterns: diff --git a/alembic/env.py b/alembic/env.py index eb679467..1fc7bd9c 100644 --- a/alembic/env.py +++ b/alembic/env.py @@ -98,7 +98,10 @@ def run_migrations_online(): connectable = create_wrolpi_engine(get_url()) - with connectable.connect() as connection: + # Autocommit: each migration statement is its own transaction. A migration may call WROLPi + # functions which write on their own connection; a migration connection holding an open read + # transaction could not then upgrade to a writer ("database is locked"). + with connectable.connect().execution_options(isolation_level='AUTOCOMMIT') as connection: context.configure( connection=connection, target_metadata=target_metadata, diff --git a/modules/archive/__init__.py b/modules/archive/__init__.py index 85af6412..3f820560 100644 --- a/modules/archive/__init__.py +++ b/modules/archive/__init__.py @@ -6,6 +6,7 @@ from abc import ABC from dataclasses import dataclass from typing import Callable, List, Optional, Tuple +from sqlalchemy import text from sqlalchemy.orm import Session from wrolpi.cmd import SINGLE_FILE_BIN, CHROMIUM, FIREFOX, DENO_BIN, SINGLE_FILE_DENO_SCRIPT @@ -405,7 +406,7 @@ def archive_cleanup(): with get_db_session(commit=True) as session: # Remove any domain Collections without any Archives. # Get all collection_ids that have archives - collection_ids = [i[0] for i in session.execute('SELECT DISTINCT collection_id FROM archive') if i[0]] + collection_ids = [i[0] for i in session.execute(text('SELECT DISTINCT collection_id FROM archive')) if i[0]] # Find domain collections that have no archives for collection in session.query(Collection).filter_by(kind='domain'): if collection.id not in collection_ids: diff --git a/modules/archive/models.py b/modules/archive/models.py index a6604918..baa75164 100644 --- a/modules/archive/models.py +++ b/modules/archive/models.py @@ -5,7 +5,7 @@ from typing import Iterable, List, Optional import pytz from sqlalchemy import Column, Integer, ForeignKey, BigInteger, Index -from sqlalchemy.orm import relationship, Session, joinedload +from sqlalchemy.orm import Mapped, relationship, Session, joinedload from wrolpi import dates from wrolpi.collections import Collection @@ -36,9 +36,9 @@ class Archive(Base, ModelHelper): id = Column(Integer, primary_key=True) collection_id = Column(Integer, ForeignKey('collection.id', ondelete='CASCADE')) - collection: Collection = relationship('Collection', primaryjoin='Archive.collection_id==Collection.id') + collection: Mapped[Collection] = relationship('Collection', primaryjoin='Archive.collection_id==Collection.id') file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), unique=True, nullable=False) - file_group: FileGroup = relationship('FileGroup') + file_group: Mapped[FileGroup] = relationship('FileGroup') @staticmethod def json_options() -> tuple: diff --git a/modules/archive/test/test_collection_id_bug.py b/modules/archive/test/test_collection_id_bug.py index 02cfae64..89b503bd 100644 --- a/modules/archive/test/test_collection_id_bug.py +++ b/modules/archive/test/test_collection_id_bug.py @@ -135,7 +135,7 @@ async def test_model_archive_extracts_url_from_singlefile(async_client, test_ses "collection_id should be set after URL extraction" # Verify collection was created with correct domain - collection = test_session.query(Collection).get(archive.collection_id) + collection = test_session.get(Collection, archive.collection_id) assert collection.name == domain assert collection.kind == 'domain' diff --git a/modules/docs/lib.py b/modules/docs/lib.py index a24317a0..3cb67253 100644 --- a/modules/docs/lib.py +++ b/modules/docs/lib.py @@ -404,8 +404,7 @@ def _search_docs(session: Session, search_str=None, author=None, subject=None, l from wrolpi.tags import Tag, TagFile tagged_fg_ids = session.query(TagFile.file_group_id) \ .join(Tag, Tag.id == TagFile.tag_id) \ - .filter(Tag.name.in_(tag_names)) \ - .subquery() + .filter(Tag.name.in_(tag_names)) query = query.filter(FileGroup.id.in_(tagged_fg_ids)) unfiltered = not (search_str or author or subject or language or mimetype or tag_names) @@ -497,10 +496,10 @@ def _fetch_section_hints(session, file_group_ids, search_str): rows = session.execute(sql, {'q': match, 'ids': list(file_group_ids)}).fetchall() hints = {} for row in rows: - hints[row['fg_id']] = { - 'kind': row['kind'], - 'ordinal': row['ordinal'], - 'label': row['label'], - 'snippet': row['snippet'], + hints[row.fg_id] = { + 'kind': row.kind, + 'ordinal': row.ordinal, + 'label': row.label, + 'snippet': row.snippet, } return hints diff --git a/modules/docs/models.py b/modules/docs/models.py index ba2e0f62..84cdc562 100644 --- a/modules/docs/models.py +++ b/modules/docs/models.py @@ -2,7 +2,7 @@ import pathlib from typing import Optional from sqlalchemy import Column, Integer, BigInteger, ForeignKey, String, Text, Index, or_ -from sqlalchemy.orm import relationship, Session, joinedload +from sqlalchemy.orm import Mapped, relationship, Session, joinedload from wrolpi.common import ModelHelper, Base from wrolpi.db import get_db_session, serializer @@ -56,7 +56,7 @@ class Doc(ModelHelper, Base): description = Column(Text) file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), nullable=False, unique=True) - file_group: FileGroup = relationship('FileGroup') + file_group: Mapped[FileGroup] = relationship('FileGroup') sections = relationship('DocSection', back_populates='doc', cascade='all, delete-orphan', passive_deletes=True) diff --git a/modules/videos/downloader.py b/modules/videos/downloader.py index 789febb7..fe4f36ca 100755 --- a/modules/videos/downloader.py +++ b/modules/videos/downloader.py @@ -1259,7 +1259,7 @@ class VideoDownloader(Downloader, ABC): # Update channel's collection's file_format to track the format used. if executed.channel_id: from modules.videos.lib import save_channels_config - channel_obj = session.query(Channel).get(executed.channel_id) + channel_obj = session.get(Channel, executed.channel_id) if channel_obj and channel_obj.collection and not channel_obj.collection.file_format: video_config = get_videos_downloader_config() channel_obj.collection.file_format = video_config.file_name_format diff --git a/modules/videos/models.py b/modules/videos/models.py index b8591ef4..20130ade 100644 --- a/modules/videos/models.py +++ b/modules/videos/models.py @@ -3,8 +3,7 @@ import pathlib from typing import Optional, Dict, List from sqlalchemy import Column, Integer, String, Boolean, Date, ForeignKey, BigInteger, Index, text, JSON -from sqlalchemy.orm import relationship, Session, deferred, undefer, joinedload, selectinload -from sqlalchemy.orm.collections import InstrumentedList +from sqlalchemy.orm import Mapped, relationship, Session, deferred, undefer, joinedload, selectinload from modules.videos.errors import UnknownVideo, UnknownChannel from wrolpi.captions import read_captions, read_captions_with_timestamps @@ -49,7 +48,7 @@ class Video(ModelHelper, Base): channel_id = Column(Integer, ForeignKey('channel.id')) channel = relationship('Channel', primaryjoin='Video.channel_id==Channel.id', back_populates='videos') file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), unique=True, nullable=False) - file_group: FileGroup = relationship('FileGroup') + file_group: Mapped[FileGroup] = relationship('FileGroup') def __repr__(self): v = None @@ -628,7 +627,7 @@ class Channel(ModelHelper, Base): info_json = deferred(Column(JSON)) info_date = Column(Date) - videos: InstrumentedList = relationship('Video', primaryjoin='Channel.id==Video.channel_id') + videos: Mapped[List['Video']] = relationship('Video', primaryjoin='Channel.id==Video.channel_id') collection_id = Column(Integer, ForeignKey('collection.id', ondelete='CASCADE')) # Joined: a Channel's name, directory and Tag live on its Collection. collection = relationship('Collection', foreign_keys=[collection_id], lazy='joined') diff --git a/modules/zim/models.py b/modules/zim/models.py index 7de30ce3..9489fdd9 100644 --- a/modules/zim/models.py +++ b/modules/zim/models.py @@ -7,7 +7,7 @@ from typing import List, Tuple, OrderedDict as OrderedDictType, Dict, Optional, from libzim import Archive, Searcher, Query, Entry, SuggestionSearcher from sqlalchemy import Column, Integer, BigInteger, ForeignKey, Text, tuple_, Boolean, UniqueConstraint -from sqlalchemy.orm import relationship, Session, joinedload +from sqlalchemy.orm import Mapped, relationship, Session, joinedload from sqlalchemy.orm.exc import NoResultFound # noqa from modules.zim.errors import UnknownZimEntry, UnknownZimTagEntry, UnknownZim @@ -71,10 +71,10 @@ class Zim(Base, ModelHelper): UniqueConstraint('path', name='zim_path_key'), ) id = Column(Integer, primary_key=True) - path: pathlib.Path = Column(MediaPathType, nullable=False) + path: Mapped[pathlib.Path] = Column(MediaPathType, nullable=False) file_group_id = Column(BigInteger, ForeignKey('file_group.id', ondelete='CASCADE'), nullable=False) - file_group: FileGroup = relationship('FileGroup') + file_group: Mapped[FileGroup] = relationship('FileGroup') auto_search = Column(Boolean, default=True) def __repr__(self): @@ -359,11 +359,11 @@ class TagZimEntry(Base): ) tag_id = Column(Integer, ForeignKey('tag.id'), primary_key=True) - tag: Tag = relationship('Tag') + tag: Mapped[Tag] = relationship('Tag') zim_id = Column(Integer, ForeignKey('zim.id', ondelete='CASCADE'), primary_key=True) - zim: Zim = relationship('Zim') - zim_entry: str = Column(Text, nullable=False, primary_key=True) - created_at: datetime = Column(TZDateTime, default=dates.now) + zim: Mapped[Zim] = relationship('Zim') + zim_entry: Mapped[str] = Column(Text, nullable=False, primary_key=True) + created_at: Mapped[datetime] = Column(TZDateTime, default=dates.now) def __repr__(self): tag = self.tag_id @@ -384,11 +384,11 @@ class TagZimEntry(Base): class ZimSubscription(Base): __tablename__ = 'zim_subscription' - id: int = Column(Integer, primary_key=True) - name: str = Column(Text, unique=True, nullable=False) - language: str = Column(Text, nullable=False) - download_id: int = Column(Integer, ForeignKey('download.id', ondelete='CASCADE'), nullable=False) - download: Download = relationship('Download', primaryjoin='ZimSubscription.download_id==Download.id') + id: Mapped[int] = Column(Integer, primary_key=True) + name: Mapped[str] = Column(Text, unique=True, nullable=False) + language: Mapped[str] = Column(Text, nullable=False) + download_id: Mapped[int] = Column(Integer, ForeignKey('download.id', ondelete='CASCADE'), nullable=False) + download: Mapped[Download] = relationship('Download', primaryjoin='ZimSubscription.download_id==Download.id') def __repr__(self): name = self.name diff --git a/requirements.txt b/requirements.txt index f7b838b4..f3f2c36f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,8 @@ Pillow==12.3.0 PyYAML==6.0.3 -SQLAlchemy[asyncio]==1.3.24 +SQLAlchemy[asyncio]==2.0.54 aiohttp==3.14.3 -alembic==1.10.4 +alembic==1.20.0 beautifulsoup4==4.15.0 cachetools==7.2.0 cryptography==50.0.2 diff --git a/wrolpi/collections/models.py b/wrolpi/collections/models.py index f146f830..a423e4f0 100644 --- a/wrolpi/collections/models.py +++ b/wrolpi/collections/models.py @@ -7,8 +7,7 @@ from typing import Optional, List from sqlalchemy import Column, Integer, String, ForeignKey, Text, DateTime, Index, UniqueConstraint, func, BigInteger, \ or_, CheckConstraint -from sqlalchemy.orm import relationship, Session, joinedload, selectinload -from sqlalchemy.orm.collections import InstrumentedList +from sqlalchemy.orm import Mapped, relationship, Session, joinedload, selectinload from wrolpi import flags from wrolpi.common import Base, ModelHelper, logger, get_media_directory, get_relative_to_media_directory, \ @@ -108,7 +107,7 @@ class Collection(ModelHelper, Base): kind = Column(String, nullable=False, server_default='channel', default='channel') # Optional directory restriction - if set, only files in this directory tree can be added - directory: Optional[pathlib.Path] = Column(MediaPathType, nullable=True) + directory: Mapped[Optional[pathlib.Path]] = Column(MediaPathType, nullable=True) # Optional tag relationship (similar to Channel) tag_id = Column(Integer, ForeignKey('tag.id')) @@ -126,7 +125,7 @@ class Collection(ModelHelper, Base): total_size = Column(BigInteger, default=0, nullable=False) # Relationship to items (ordered) - items: InstrumentedList = relationship( + items: Mapped[List['CollectionItem']] = relationship( 'CollectionItem', primaryjoin='Collection.id==CollectionItem.collection_id', back_populates='collection', @@ -135,7 +134,7 @@ class Collection(ModelHelper, Base): ) # Relationship to downloads - downloads: InstrumentedList = relationship( + downloads: Mapped[List['Download']] = relationship( 'Download', primaryjoin='Download.collection_id==Collection.id', back_populates='collection', @@ -1239,7 +1238,7 @@ class CollectionItem(ModelHelper, Base): # Relationships collection = relationship('Collection', back_populates='items') - file_group: FileGroup = relationship('FileGroup') + file_group: Mapped[FileGroup] = relationship('FileGroup') zim = relationship('Zim') # Indexes for performance diff --git a/wrolpi/collections/test/test_reorganize.py b/wrolpi/collections/test/test_reorganize.py index a916fafd..6460c8b3 100644 --- a/wrolpi/collections/test/test_reorganize.py +++ b/wrolpi/collections/test/test_reorganize.py @@ -225,7 +225,7 @@ async def test_handle_reorganize_marks_job_failed_on_exception( video = video_factory(channel_id=channel_id, title='will_fail', with_video_file=True) with get_db_session() as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) old_path = video_obj.file_group.primary_path dest = channel_dir / 'renamed_will_fail.mp4' @@ -337,7 +337,7 @@ async def test_reorganize_updates_filegroup_data_fields(async_client, test_direc with get_db_session(commit=True) as session: # Link archive to our collection - archive_obj = session.query(Archive).get(archive.id) + archive_obj = session.get(Archive, archive.id) archive_obj.collection_id = collection_id session.commit() @@ -378,7 +378,7 @@ async def test_reorganize_updates_filegroup_data_fields(async_client, test_direc # Verify results with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) # Verify files were physically renamed assert new_primary_path.exists(), "New primary file should exist" @@ -751,7 +751,7 @@ async def test_reorganize_preserves_tag_association(async_client, test_directory video = video_factory(channel_id=channel_id, title='test_video', with_video_file=True) with get_db_session(commit=True) as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) video_obj.file_group.add_tag(session, tag.id) fg_id = video_obj.file_group.id old_primary_path = video_obj.file_group.primary_path @@ -759,7 +759,7 @@ async def test_reorganize_preserves_tag_association(async_client, test_directory # Verify tag was added with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert fg.tag_names == ['mytag'], "Tag should be applied before reorganization" # Create move mapping: rename files to a new pattern @@ -779,7 +779,7 @@ async def test_reorganize_preserves_tag_association(async_client, test_directory # Verify tags preserved after file move with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert fg.tag_names == ['mytag'], "Tag should be preserved after reorganization" assert fg.primary_path == new_primary_path, "File should have moved" @@ -810,7 +810,7 @@ async def test_reorganize_updates_playlists_config(async_client, test_directory, video = video_factory(channel_id=channel_id, title='test_video', with_video_file=True) with get_db_session(commit=True) as session: - file_group = session.query(Video).get(video.id).file_group + file_group = session.get(Video, video.id).file_group old_primary_path = file_group.primary_path playlist = Collection(name='Watch', kind='playlist') session.add(playlist) @@ -869,7 +869,7 @@ async def test_reorganize_then_refresh_preserves_filegroup_id_and_tags( video = video_factory(channel_id=channel_id, title='old title', with_video_file=True) with get_db_session(commit=True) as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) video_obj.file_group.add_tag(session, tag.id) fg_id = video_obj.file_group.id old_primary_path = video_obj.file_group.primary_path @@ -887,7 +887,7 @@ async def test_reorganize_then_refresh_preserves_filegroup_id_and_tags( await refresh_files([channel_dir]) with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert fg is not None, 'FileGroup was dropped by refresh after reorganize' assert fg.primary_path == new_primary_path assert fg.stem == '2024-01-01 New Title' @@ -929,7 +929,7 @@ async def test_reorganize_preserves_multiple_tags(async_client, test_directory, video = video_factory(channel_id=channel_id, title='multi_tagged_video', with_video_file=True) with get_db_session(commit=True) as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) video_obj.file_group.add_tag(session, tag1.id) video_obj.file_group.add_tag(session, tag2.id) video_obj.file_group.add_tag(session, tag3.id) @@ -939,7 +939,7 @@ async def test_reorganize_preserves_multiple_tags(async_client, test_directory, # Verify all tags were added with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert sorted(fg.tag_names) == ['alpha', 'beta', 'gamma'], "All tags should be applied" # Create move mapping @@ -958,7 +958,7 @@ async def test_reorganize_preserves_multiple_tags(async_client, test_directory, # Verify all tags preserved with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert sorted(fg.tag_names) == ['alpha', 'beta', 'gamma'], \ "All tags should be preserved after reorganization" assert fg.primary_path == new_primary_path, "File should have moved" @@ -1002,7 +1002,7 @@ async def test_reorganize_updates_tag_directory_links(async_client, test_directo video = video_factory(channel_id=channel_id, title='link_test_video', with_video_file=True) with get_db_session(commit=True) as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) video_obj.file_group.add_tag(session, tag.id) fg_id = video_obj.file_group.id old_primary_path = video_obj.file_group.primary_path @@ -1099,11 +1099,11 @@ async def test_batch_reorganize_preserves_tags(async_client, test_directory, vid video2 = video_factory(channel_id=channel2_id, title='batch_video_2', with_video_file=True) with get_db_session(commit=True) as session: - video1_obj = session.query(Video).get(video1.id) + video1_obj = session.get(Video, video1.id) video1_obj.file_group.add_tag(session, tag_a.id) fg1_id = video1_obj.file_group.id - video2_obj = session.query(Video).get(video2.id) + video2_obj = session.get(Video, video2.id) video2_obj.file_group.add_tag(session, tag_b.id) fg2_id = video2_obj.file_group.id @@ -1111,8 +1111,8 @@ async def test_batch_reorganize_preserves_tags(async_client, test_directory, vid # Verify tags before batch reorganization with get_db_session() as session: - fg1 = session.query(FileGroup).get(fg1_id) - fg2 = session.query(FileGroup).get(fg2_id) + fg1 = session.get(FileGroup, fg1_id) + fg2 = session.get(FileGroup, fg2_id) assert fg1.tag_names == ['batch_tag_a'] assert fg2.tag_names == ['batch_tag_b'] @@ -1125,8 +1125,8 @@ async def test_batch_reorganize_preserves_tags(async_client, test_directory, vid # Verify tags preserved after batch reorganization with get_db_session() as session: - fg1 = session.query(FileGroup).get(fg1_id) - fg2 = session.query(FileGroup).get(fg2_id) + fg1 = session.get(FileGroup, fg1_id) + fg2 = session.get(FileGroup, fg2_id) assert fg1.tag_names == ['batch_tag_a'], \ "Tag on first channel's video should be preserved after batch reorganization" assert fg2.tag_names == ['batch_tag_b'], \ @@ -1172,7 +1172,7 @@ async def test_reorganize_file_format_updated_after_completion(async_client, tes video = video_factory(channel_id=channel_id, title='format_test_video', with_video_file=True) with get_db_session() as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) old_primary_path = video_obj.file_group.primary_path # Get the current config format (what it should become after reorganization) @@ -1195,7 +1195,7 @@ async def test_reorganize_file_format_updated_after_completion(async_client, tes # Before completion, file_format should still be old with get_db_session() as session: - collection = session.query(Collection).get(collection_id) + collection = session.get(Collection, collection_id) assert collection.file_format == old_format, \ "file_format should NOT be updated before reorganization completes" @@ -1204,7 +1204,7 @@ async def test_reorganize_file_format_updated_after_completion(async_client, tes # After completion, file_format should now be updated with get_db_session() as session: - collection = session.query(Collection).get(collection_id) + collection = session.get(Collection, collection_id) assert collection.file_format == new_format, \ "file_format should be updated after reorganization completes" assert collection.needs_reorganization is False, \ @@ -1248,9 +1248,9 @@ async def test_reorganize_can_retry_after_partial_failure(async_client, test_dir video3 = video_factory(channel_id=channel_id, title='partial_video_3', with_video_file=True) with get_db_session() as session: - video1_obj = session.query(Video).get(video1.id) - video2_obj = session.query(Video).get(video2.id) - video3_obj = session.query(Video).get(video3.id) + video1_obj = session.get(Video, video1.id) + video2_obj = session.get(Video, video2.id) + video3_obj = session.get(Video, video3.id) path1 = video1_obj.file_group.primary_path path2 = video2_obj.file_group.primary_path @@ -1265,13 +1265,13 @@ async def test_reorganize_can_retry_after_partial_failure(async_client, test_dir assert not path1.exists(), "Video 1 should not be at source" with get_db_session() as session: - video1_obj = session.query(Video).get(video1.id) + video1_obj = session.get(Video, video1.id) assert video1_obj.file_group.primary_path == path1, \ "DB should still point to old location (simulating partial failure before DB update)" # Verify needs_reorganization is still True (file_format not updated) with get_db_session() as session: - collection = session.query(Collection).get(collection_id) + collection = session.get(Collection, collection_id) assert collection.file_format == old_format, \ "file_format should still be old after partial failure" assert collection.needs_reorganization is True, \ @@ -1315,7 +1315,7 @@ async def test_reorganize_handles_already_moved_files(async_client, test_directo video = video_factory(channel_id=channel_id, title='recovery_video', with_video_file=True) with get_db_session() as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) old_primary_path = video_obj.file_group.primary_path fg_id = video_obj.file_group.id @@ -1331,7 +1331,7 @@ async def test_reorganize_handles_already_moved_files(async_client, test_directo assert dest_path.exists(), "Destination should exist (manually moved)" with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert fg.primary_path == old_primary_path, "DB should still point to source" # Now run reorganization - it should recover by updating DB @@ -1352,7 +1352,7 @@ async def test_reorganize_handles_already_moved_files(async_client, test_directo # Verify recovery: DB should now point to destination with get_db_session() as session: - fg = session.query(FileGroup).get(fg_id) + fg = session.get(FileGroup, fg_id) assert fg.primary_path == dest_path, \ f"DB should be updated to destination path after recovery. Got {fg.primary_path}" assert fg.directory == dest_path.parent, \ @@ -1404,9 +1404,9 @@ async def test_reorganize_handles_mixed_state(async_client, test_directory, vide video3 = video_factory(channel_id=channel_id, title='mixed_video_3', with_video_file=True) with get_db_session() as session: - v1_obj = session.query(Video).get(video1.id) - v2_obj = session.query(Video).get(video2.id) - v3_obj = session.query(Video).get(video3.id) + v1_obj = session.get(Video, video1.id) + v2_obj = session.get(Video, video2.id) + v3_obj = session.get(Video, video3.id) path1 = v1_obj.file_group.primary_path path2 = v2_obj.file_group.primary_path @@ -1452,19 +1452,19 @@ async def test_reorganize_handles_mixed_state(async_client, test_directory, vide with get_db_session() as session: # Video 1: Should have DB updated to dest (recovery) - fg1 = session.query(FileGroup).get(fg1_id) + fg1 = session.get(FileGroup, fg1_id) assert fg1.primary_path == dest1, \ f"Video 1 should have DB updated to dest path (recovery). Got {fg1.primary_path}" # Video 2: Should be moved and DB updated (normal) - fg2 = session.query(FileGroup).get(fg2_id) + fg2 = session.get(FileGroup, fg2_id) assert fg2.primary_path == dest2, \ f"Video 2 should be moved to dest. Got {fg2.primary_path}" assert dest2.exists(), "Video 2 file should exist at destination" assert not path2.exists(), "Video 2 file should not exist at source" # Video 3: Should be skipped (both exist - conflict) - fg3 = session.query(FileGroup).get(fg3_id) + fg3 = session.get(FileGroup, fg3_id) assert fg3.primary_path == path3, \ f"Video 3 should remain at source (conflict skipped). Got {fg3.primary_path}" assert path3.exists(), "Video 3 should still exist at source" @@ -1526,8 +1526,8 @@ async def test_batch_reorganize_can_resume_after_collection_failure(async_client # Verify both collections need reorganization with get_db_session() as session: - c1 = session.query(Collection).get(collection1_id) - c2 = session.query(Collection).get(collection2_id) + c1 = session.get(Collection, collection1_id) + c2 = session.get(Collection, collection2_id) assert c1.needs_reorganization is True assert c2.needs_reorganization is True @@ -1542,8 +1542,8 @@ async def test_batch_reorganize_can_resume_after_collection_failure(async_client # Verify first collection completed with get_db_session() as session: - c1 = session.query(Collection).get(collection1_id) - c2 = session.query(Collection).get(collection2_id) + c1 = session.get(Collection, collection1_id) + c2 = session.get(Collection, collection2_id) # First collection should be done (file_format updated) # Note: This tests current behavior - may need adjustment based on implementation @@ -1609,9 +1609,9 @@ async def test_reorganize_skips_deleted_files_gracefully(async_client, test_dire video3 = video_factory(channel_id=channel_id, title='existing_video_3', with_video_file=True) with get_db_session() as session: - v1_obj = session.query(Video).get(video1.id) - v2_obj = session.query(Video).get(video2.id) - v3_obj = session.query(Video).get(video3.id) + v1_obj = session.get(Video, video1.id) + v2_obj = session.get(Video, video2.id) + v3_obj = session.get(Video, video3.id) path1 = v1_obj.file_group.primary_path path2 = v2_obj.file_group.primary_path @@ -1626,8 +1626,8 @@ async def test_reorganize_skips_deleted_files_gracefully(async_client, test_dire # (The video_modeler would otherwise create a new Video and try to ffprobe it) os.remove(str(path2)) with get_db_session(commit=True) as session: - video_to_delete = session.query(Video).get(video2.id) - fg_to_delete = session.query(FileGroup).get(fg2_id) + video_to_delete = session.get(Video, video2.id) + fg_to_delete = session.get(FileGroup, fg2_id) session.delete(video_to_delete) session.delete(fg_to_delete) @@ -1663,7 +1663,7 @@ async def test_reorganize_skips_deleted_files_gracefully(async_client, test_dire # Verify results with get_db_session() as session: # Video 1: Should be moved and DB updated - fg1 = session.query(FileGroup).get(fg1_id) + fg1 = session.get(FileGroup, fg1_id) assert fg1.primary_path == dest1, f"Video 1 should be moved to dest. Got {fg1.primary_path}" assert dest1.exists(), "Video 1 file should exist at destination" assert not path1.exists(), "Video 1 file should not exist at source" @@ -1674,13 +1674,13 @@ async def test_reorganize_skips_deleted_files_gracefully(async_client, test_dire assert not dest2.exists(), "Video 2 file should not exist at destination (was deleted)" # Video 3: Should be moved and DB updated - fg3 = session.query(FileGroup).get(fg3_id) + fg3 = session.get(FileGroup, fg3_id) assert fg3.primary_path == dest3, f"Video 3 should be moved to dest. Got {fg3.primary_path}" assert dest3.exists(), "Video 3 file should exist at destination" assert not path3.exists(), "Video 3 file should not exist at source" # Collection file_format should be updated (reorganization completed successfully) - collection = session.query(Collection).get(collection_id) + collection = session.get(Collection, collection_id) assert collection.file_format == config.file_name_format, \ "Collection file_format should be updated after successful reorganization" @@ -1750,7 +1750,7 @@ def test_reorganize_skips_videos_missing_required_metadata( with_info_json=info_json if info_json else None, ) - video_obj = test_session.query(Video).get(video.id) + video_obj = test_session.get(Video, video.id) # Set source_id directly on the Video model (separate from info_json) video_obj.source_id = 'abc123' if has_source_id else None @@ -1823,7 +1823,7 @@ def test_reorganize_skips_archives_missing_required_metadata( title='Test Article', ) - archive_obj = test_session.query(Archive).get(archive.id) + archive_obj = test_session.get(Archive, archive.id) # Clear URL if testing without it if not has_url: @@ -1892,12 +1892,12 @@ async def test_domain_reorganization_moves_files_from_root_to_year_subdirectory( archive = archive_factory(domain='test.com', url='https://test.com/article', title='Test Article') with get_db_session(commit=True) as session: - archive_obj = session.query(Archive).get(archive.id) + archive_obj = session.get(Archive, archive.id) archive_obj.collection_id = collection_id session.commit() # Verify: collection appears organized but files are in wrong place - collection = session.query(Collection).get(collection_id) + collection = session.get(Collection, collection_id) assert collection.file_format == year_format, "Collection format should match config" assert collection.needs_reorganization is False, \ "Collection should appear organized (format matches config)" @@ -1923,7 +1923,7 @@ async def test_domain_reorganization_moves_files_from_root_to_year_subdirectory( # Verify: files moved to year subdirectory with get_db_session() as session: - archive_obj = session.query(Archive).get(archive.id) + archive_obj = session.get(Archive, archive.id) new_path = archive_obj.file_group.primary_path assert '/2000/' in str(new_path), f"File should be in year subdirectory. Got: {new_path}" assert new_path.exists(), "New file should exist" @@ -1985,7 +1985,7 @@ async def test_channel_reorganization_moves_files_from_root_to_year_subdirectory ) with get_db_session(commit=True) as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) old_path = video_obj.file_group.primary_path assert old_path.parent == channel_dir, \ @@ -2008,7 +2008,7 @@ async def test_channel_reorganization_moves_files_from_root_to_year_subdirectory # Verify: files moved to year subdirectory with get_db_session() as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) new_path = video_obj.file_group.primary_path assert '/2024/' in str(new_path), f"File should be in year subdirectory. Got: {new_path}" assert new_path.exists(), "New file should exist" @@ -2081,8 +2081,8 @@ async def test_channel_reorganization_preserves_root_level_files( # Verify initial state: videos in root, root files exist with get_db_session() as session: - v1 = session.query(Video).get(video1.id) - v2 = session.query(Video).get(video2.id) + v1 = session.get(Video, video1.id) + v2 = session.get(Video, video2.id) assert v1.file_group.primary_path.parent == channel_dir assert v2.file_group.primary_path.parent == channel_dir assert root_json.exists(), "Root JSON should exist before reorganization" @@ -2099,8 +2099,8 @@ async def test_channel_reorganization_preserves_root_level_files( # Verify: videos moved to year subdirectory with get_db_session() as session: - v1 = session.query(Video).get(video1.id) - v2 = session.query(Video).get(video2.id) + v1 = session.get(Video, video1.id) + v2 = session.get(Video, video2.id) assert '/2024/' in str(v1.file_group.primary_path), "Video 1 should be in year subdirectory" assert '/2024/' in str(v2.file_group.primary_path), "Video 2 should be in year subdirectory" assert v1.file_group.primary_path.exists() @@ -2154,7 +2154,7 @@ async def test_preview_computes_actual_moves(async_client, test_session, test_di archive = archive_factory(domain='exactcount.com', url='https://exactcount.com/article', title='Test') with get_db_session(commit=True) as session: - archive_obj = session.query(Archive).get(archive.id) + archive_obj = session.get(Archive, archive.id) archive_obj.collection_id = collection_id session.commit() @@ -2233,8 +2233,8 @@ async def test_reorganize_fails_on_filename_conflicts(async_client, test_directo # Update both videos to have the same title and source_id # This makes them produce the same destination filename with get_db_session(commit=True) as session: - v1 = session.query(Video).get(video1.id) - v2 = session.query(Video).get(video2.id) + v1 = session.get(Video, video1.id) + v2 = session.get(Video, video2.id) v1.file_group.title = 'Duplicate Title' v2.file_group.title = 'Duplicate Title' v1.source_id = same_source_id @@ -2296,8 +2296,8 @@ async def test_reorganize_preview_returns_conflict_details(async_client, test_di # Update both videos to have the same title and source_id with get_db_session(commit=True) as session: - v1 = session.query(Video).get(video1.id) - v2 = session.query(Video).get(video2.id) + v1 = session.get(Video, video1.id) + v2 = session.get(Video, video2.id) v1.file_group.title = 'Duplicate Preview Title' v2.file_group.title = 'Duplicate Preview Title' v1.source_id = same_source_id @@ -2428,8 +2428,8 @@ async def test_conflict_preview_includes_quality_rank(async_client, test_directo # Make both videos conflict (same title, same source_id → same destination) with get_db_session(commit=True) as session: - v1 = session.query(Video).get(video1.id) - v2 = session.query(Video).get(video2.id) + v1 = session.get(Video, video1.id) + v2 = session.get(Video, video2.id) v1.file_group.title = 'Same Title' v2.file_group.title = 'Same Title' v1.source_id = same_source_id @@ -2762,7 +2762,7 @@ def test_preview_excludes_files_already_in_correct_location(test_session, test_d # Verify the video is at the expected location with get_db_session() as session: - video_obj = session.query(Video).get(video.id) + video_obj = session.get(Video, video.id) current_path = video_obj.file_group.primary_path # The format_video_filename should produce the same filename diff --git a/wrolpi/common.py b/wrolpi/common.py index 7a6aa83b..9d6f1b2e 100644 --- a/wrolpi/common.py +++ b/wrolpi/common.py @@ -36,7 +36,7 @@ from aiohttp import ClientResponse, ClientSession from bs4 import BeautifulSoup from selenium import webdriver from sqlalchemy import types -from sqlalchemy.ext.declarative import declarative_base +from sqlalchemy.orm import declarative_base from sqlalchemy.orm import Session from wrolpi.dates import now, from_timestamp diff --git a/wrolpi/conftest.py b/wrolpi/conftest.py index c5cb5a61..d3990bb5 100644 --- a/wrolpi/conftest.py +++ b/wrolpi/conftest.py @@ -75,7 +75,7 @@ def test_db() -> Tuple[Engine, Session]: # Create all tables. No need to check if they exist because this is a test DB. Base.metadata.create_all(test_engine, checkfirst=False) # Triggers and FTS5 tables, exactly as the alembic baseline installs them. - with test_engine.connect() as conn: + with test_engine.begin() as conn: install_raw_ddl(conn) session = sessionmaker(bind=test_engine)() return test_engine, session @@ -1124,9 +1124,9 @@ def insert_file_group(test_session, test_directory): files=json.dumps(files), stem=stem, ) - test_session.execute('INSERT INTO file_group ' - '(indexed, directory, primary_path, files, stem) VALUES ' - '(true, :directory, :primary_path, :files, :stem)', params) + test_session.execute(sqlalchemy.text('INSERT INTO file_group ' + '(indexed, directory, primary_path, files, stem) VALUES ' + '(true, :directory, :primary_path, :files, :stem)'), params) return _ diff --git a/wrolpi/db.py b/wrolpi/db.py index cad659bc..e9a95e7b 100644 --- a/wrolpi/db.py +++ b/wrolpi/db.py @@ -191,12 +191,15 @@ def create_wrolpi_engine(target: Union[str, pathlib.Path]) -> sqlalchemy.engine. @event.listens_for(engine, 'begin') def _sqlite_do_begin(conn): + if conn.get_execution_options().get('isolation_level') == 'AUTOCOMMIT': + # Every statement is its own transaction (e.g. Alembic migrations). + return # Write sessions (`commit=True`) take the write lock up front; read sessions stay deferred # so readers never block (in WAL; see `_immediate_txn`). if _immediate_txn.get(): - conn.execute('BEGIN IMMEDIATE') + conn.exec_driver_sql('BEGIN IMMEDIATE') else: - conn.execute('BEGIN') + conn.exec_driver_sql('BEGIN') return engine @@ -325,7 +328,7 @@ def get_db_session(commit: bool = False) -> Generator[Session, Any, None]: # Rollback only if a transaction hasn't been committed. # In tests, the test_session fixture manages the session lifecycle, # so we should not rollback here - that would undo other test operations. - if not PYTEST and session.transaction.is_active: + if not PYTEST and session.in_transaction(): session.rollback() @@ -394,8 +397,9 @@ def session_curs(session: Session) -> Generator[sqlite3.Cursor, Any, None]: def _session_has_connection(session: Session) -> bool: """True if the Session's transaction has already connected (and so already emitted BEGIN).""" - # SQLAlchemy 1.3 has no public API for this; 2.0 replaces it with `session.in_transaction()`. - transaction = session.transaction + # `in_transaction()` is also true for a transaction that has autobegun but not yet connected; + # SQLAlchemy has no public API for "connected". + transaction = session.get_transaction() return transaction is not None and bool(transaction._connections) diff --git a/wrolpi/downloader.py b/wrolpi/downloader.py index 9eae4baa..256d18b1 100644 --- a/wrolpi/downloader.py +++ b/wrolpi/downloader.py @@ -26,7 +26,7 @@ import pytz from feedparser import FeedParserDict from sqlalchemy import Column, Integer, String, Text, ForeignKey, Index, JSON from sqlalchemy.exc import OperationalError -from sqlalchemy.orm import Session, relationship +from sqlalchemy.orm import Mapped, Session, relationship from wrolpi import flags from wrolpi.api_utils import api_app, perpetual_signal @@ -253,7 +253,7 @@ class Download(ModelHelper, Base): # noqa url = Column(String, nullable=False, unique=True) attempts = Column(Integer, default=0) - destination: pathlib.Path = Column(MediaPathType) # '/media/wrolpi/videos/WROLPi' + destination: Mapped[pathlib.Path] = Column(MediaPathType) # '/media/wrolpi/videos/WROLPi' downloader = Column(Text) # 'videos', 'archive', 'kiwix_zim', etc. sub_downloader = Column(Text) # The downloader any returned downloads should be sent. error = Column(Text) # traceback from an error during downloading. diff --git a/wrolpi/files/modeler.py b/wrolpi/files/modeler.py index ac9b1cc9..28b07d20 100644 --- a/wrolpi/files/modeler.py +++ b/wrolpi/files/modeler.py @@ -138,7 +138,7 @@ async def _apply_per_item( for attempt in range(lock_retries): try: with get_db_session(commit=True) as session: - file_group = session.query(FileGroup).get(fg_id) + file_group = session.get(FileGroup, fg_id) if file_group is None: break apply(session, file_group, prepared) diff --git a/wrolpi/files/models.py b/wrolpi/files/models.py index 35eb7133..d056ec8e 100644 --- a/wrolpi/files/models.py +++ b/wrolpi/files/models.py @@ -6,9 +6,9 @@ import urllib.parse from datetime import datetime from typing import List, Type, Optional, Iterable -from sqlalchemy import Column, String, BigInteger, Boolean, event, Float, Index, Integer, JSON, or_ +from sqlalchemy import Column, String, BigInteger, Boolean, event, Float, Index, Integer, JSON, or_, Text from sqlalchemy import types -from sqlalchemy.orm import deferred, relationship, Session, selectinload +from sqlalchemy.orm import Mapped, deferred, relationship, Session, selectinload from wrolpi.common import Base, ModelHelper, logger, recursive_map, get_media_directory, \ get_relative_to_media_directory, unique_by_predicate, replace_file @@ -51,6 +51,7 @@ class FancyJSON(types.TypeDecorator): Converts datetime to ISO strings when moving into DB, and vice versa. """ impl = JSON + cache_ok = True def process_bind_param(self, value, dialect): if value: @@ -89,11 +90,11 @@ class FileGroup(ModelHelper, Base): Index('file_group_directory_stem_idx', 'directory', 'stem', unique=True), ) # SQLite requires exactly "INTEGER PRIMARY KEY" for the rowid alias (FTS5 content_rowid). - id: int = Column(BigInteger().with_variant(Integer, 'sqlite'), primary_key=True) + id: Mapped[int] = Column(BigInteger().with_variant(Integer, 'sqlite'), primary_key=True) # Directory containing all files in this group (absolute path). # All paths in `files` and `data` are relative to this directory. - directory: pathlib.Path = Column(MediaPathType, nullable=False) + directory: Mapped[pathlib.Path] = Column(MediaPathType, nullable=False) author = Column(String) # name of the author, maybe even a URL censored = Column(Boolean) # the file is no longer available for download @@ -109,7 +110,7 @@ class FileGroup(ModelHelper, Base): mimetype = Column(String) # wrolpi.files.lib.get_mimetype model = Column(String) # "video", "archive", "doc", etc. modification_datetime = Column(TZDateTime) # the modification date of the file on disk - primary_path: pathlib.Path = Column(MediaPathType, nullable=False, unique=True) + primary_path: Mapped[pathlib.Path] = Column(MediaPathType, nullable=False, unique=True) published_datetime = Column(TZDateTime) # the date the creator published this file published_modified_datetime = Column(TZDateTime) # the date the publisher modified this file size = Column(BigInteger, default=lambda: 0) @@ -125,7 +126,7 @@ class FileGroup(ModelHelper, Base): # `file_group_effective_datetime_trigger` and `update_effective_datetime` event handler effective_datetime = Column(TZDateTime) # Equivalent to COALESCE(published_datetime, download_datetime) - tag_files: Iterable[TagFile] = relationship('TagFile', cascade='all') + tag_files: Mapped[List[TagFile]] = relationship('TagFile', cascade='all') # Full-text search over these columns is provided by the external-content FTS5 table # `file_group_fts` (see `wrolpi.fts`): a=title, b=author-ish, c=description, d=body/captions. @@ -863,8 +864,8 @@ class Directory(ModelHelper, Base): Index('directory_name_idx', 'name'), ) - path: pathlib.Path = Column(MediaPathType, primary_key=True) - name: str = Column(String, nullable=False) + path: Mapped[pathlib.Path] = Column(MediaPathType, primary_key=True) + name: Mapped[str] = Column(String, nullable=False) idempotency = Column(TZDateTime, default=lambda: now()) @serializer @@ -883,5 +884,7 @@ class WrolpiKV(Base): """Internal key/value store. Not user config. Holds schema-adjacent stamps such as STEM_ALGORITHM_VERSION so compare can refuse to run on a stale stem cache.""" __tablename__ = 'wrolpi_kv' - key = Column(String, primary_key=True) - value = Column(String, nullable=False) + # Matches the table the migration created (`key TEXT PRIMARY KEY`); SQLite lets a non-INTEGER + # primary key hold NULL unless it is declared NOT NULL. + key = Column(Text, primary_key=True, nullable=True) + value = Column(Text, nullable=False) diff --git a/wrolpi/files/test/test_lib.py b/wrolpi/files/test/test_lib.py index fe4265ed..193ff7b8 100644 --- a/wrolpi/files/test/test_lib.py +++ b/wrolpi/files/test/test_lib.py @@ -15,6 +15,7 @@ import wrolpi.common from modules.videos import Video from wrolpi.common import timer, get_wrolpi_config from wrolpi.conftest import await_switches +import sqlalchemy from sqlalchemy.exc import IntegrityError from wrolpi.errors import InvalidFile, UnknownDirectory, FileGroupIsTagged, NoPrimaryFile @@ -1429,9 +1430,9 @@ async def test_apply_indexers_concurrent_writer(test_session, make_files_structu # Another connection commits a write mid-batch, like a modeler or download worker. other = maker() try: - other.execute("INSERT INTO directory (path, name, idempotency) " - "VALUES ('/tmp/concurrent', 'concurrent', CURRENT_TIMESTAMP) " - "ON CONFLICT (path) DO UPDATE SET name = 'concurrent'") + other.execute(sqlalchemy.text("INSERT INTO directory (path, name, idempotency) " + "VALUES ('/tmp/concurrent', 'concurrent', CURRENT_TIMESTAMP) " + "ON CONFLICT (path) DO UPDATE SET name = 'concurrent'")) other.commit() finally: other.close() diff --git a/wrolpi/fts.py b/wrolpi/fts.py index a97bbf4b..cb7b2435 100644 --- a/wrolpi/fts.py +++ b/wrolpi/fts.py @@ -22,6 +22,8 @@ import re import sqlite3 from typing import List, Optional, Tuple +from wrolpi.schema_ddl import execute_sql + FILE_GROUP_FTS_COLUMNS = ('a_text', 'b_text', 'c_text', 'd_text') # The fast (non-deep) path searches these columns only. ABC_COLUMNS = ('a_text', 'b_text', 'c_text') @@ -330,10 +332,12 @@ def headline_texts(entries: List[Optional[str]], search_str: str, return results -def rebuild_fts(curs): - """Rebuild the FTS5 indexes from their content tables (e.g. after a bulk import).""" - curs.execute("INSERT INTO file_group_fts(file_group_fts) VALUES('rebuild')") - curs.execute("INSERT INTO doc_section_fts(doc_section_fts) VALUES('rebuild')") +def rebuild_fts(conn): + """Rebuild the FTS5 indexes from their content tables (e.g. after a bulk import). + + `conn` may be a SQLAlchemy Connection, a raw `sqlite3.Connection`, or a cursor.""" + execute_sql(conn, "INSERT INTO file_group_fts(file_group_fts) VALUES('rebuild')") + execute_sql(conn, "INSERT INTO doc_section_fts(doc_section_fts) VALUES('rebuild')") def optimize_fts(curs): diff --git a/wrolpi/media_path.py b/wrolpi/media_path.py index 651a1e3c..3c569755 100644 --- a/wrolpi/media_path.py +++ b/wrolpi/media_path.py @@ -8,6 +8,7 @@ PATH_TYPE = Union[str, pathlib.Path] class MediaPathType(types.TypeDecorator): # noqa impl = types.String + cache_ok = True def process_bind_param(self, value, dialect): """Convert paths into what the DB expects. (pathlib.Path -> str)""" diff --git a/wrolpi/schema_ddl.py b/wrolpi/schema_ddl.py index 5ba48385..0f53f179 100644 --- a/wrolpi/schema_ddl.py +++ b/wrolpi/schema_ddl.py @@ -185,13 +185,21 @@ def install_raw_ddl(conn): from wrolpi import fts for statement in [*TRIGGER_DDL, *fts.FTS_DDL]: - conn.execute(statement) + execute_sql(conn, statement) # DDL for tables created after the baseline migration (which also calls this function). if _table_exists(conn, 'repository'): for statement in fts.REPOSITORY_FTS_DDL: - conn.execute(statement) + execute_sql(conn, statement) + + +def execute_sql(conn, statement: str): + """Execute a plain SQL string on a SQLAlchemy Connection or a raw `sqlite3.Connection`.""" + if hasattr(conn, 'exec_driver_sql'): + return conn.exec_driver_sql(statement) + return conn.execute(statement) def _table_exists(conn, name: str) -> bool: - return conn.execute(f"SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = '{name}'").fetchone() is not None + sql = f"SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = '{name}'" + return execute_sql(conn, sql).fetchone() is not None diff --git a/wrolpi/tags.py b/wrolpi/tags.py index cf1c0fdd..63c6173e 100644 --- a/wrolpi/tags.py +++ b/wrolpi/tags.py @@ -8,7 +8,7 @@ import cachetools from cachetools.keys import hashkey from sqlalchemy import Column, Integer, String, ForeignKey, BigInteger, event, UniqueConstraint from sqlalchemy.exc import IntegrityError -from sqlalchemy.orm import relationship, Session +from sqlalchemy.orm import Mapped, relationship, Session from wrolpi import dates, flags from wrolpi.common import ModelHelper, Base, logger, ConfigFile, get_media_directory, background_task, \ @@ -33,7 +33,7 @@ class TagFile(ModelHelper, Base): __table_args__ = ( UniqueConstraint('tag_id', 'file_group_id', name='tag_file_tag_id_file_group_id_key'), ) - created_at: datetime = Column(TZDateTime, default=dates.now) + created_at: Mapped[datetime] = Column(TZDateTime, default=dates.now) tag_id = Column(Integer, ForeignKey('tag.id'), primary_key=True) # Joined: a TagFile is loaded for its Tag's name (serializers, the tags config). @@ -77,8 +77,8 @@ class Tag(ModelHelper, Base): name = Column(String, unique=True, nullable=False) color = Column(String) - tag_files: List[TagFile] = relationship('TagFile', back_populates='tag', cascade='all') - tag_zim_entries: List = relationship('TagZimEntry', back_populates='tag', cascade='all') + tag_files: Mapped[List[TagFile]] = relationship('TagFile', back_populates='tag', cascade='all') + tag_zim_entries: Mapped[List['TagZimEntry']] = relationship('TagZimEntry', back_populates='tag', cascade='all') # Note: Channel relationship removed - Channels now access Tags through Collection @@ -722,7 +722,7 @@ def _sync_tags_directory_tag_files(tags_directory: pathlib.Path, session: Sessio """Create all links that should be in the Tags Directory. Return all links that should exist.""" from wrolpi.files.models import FileGroup tag_files: List[Tuple[Tag, TagFile, FileGroup]] = session.query(Tag, TagFile, FileGroup) \ - .outerjoin(Tag, FileGroup).all() + .select_from(TagFile).join(TagFile.tag).join(TagFile.file_group).all() links = list() for tag, tag_file, file_group in tag_files: paths_map = file_group.get_tag_directory_paths_map() diff --git a/wrolpi/test/test_db.py b/wrolpi/test/test_db.py index 2d8bb1f1..c6ac2edf 100644 --- a/wrolpi/test/test_db.py +++ b/wrolpi/test/test_db.py @@ -4,6 +4,7 @@ import sqlite3 from unittest import mock import pytest +import sqlalchemy from wrolpi.conftest import production_like_sessions, probe_write_lock_is_held, count_connections from wrolpi.db import get_db_session, _configure_sqlite_connection, RequestDB, RequestSession, no_db_access, \ @@ -121,7 +122,7 @@ def test_write_session_takes_write_lock_up_front(test_session): with get_db_session(commit=True) as session: # Trigger the transaction's BEGIN with a trivial read (as the download dispatcher # does before writing). This session already holds the write lock. - session.execute('SELECT 1') + session.execute(sqlalchemy.text('SELECT 1')) assert probe_write_lock_is_held(db_file), \ 'immediate session did not hold the write lock after BEGIN' @@ -132,7 +133,7 @@ def test_read_session_does_not_take_write_lock(test_session): with production_like_sessions(test_session): with get_db_session() as session: - session.execute('SELECT 1') + session.execute(sqlalchemy.text('SELECT 1')) assert not probe_write_lock_is_held(db_file), \ 'read-only session unexpectedly holds the write lock' @@ -152,12 +153,12 @@ async def test_background_task_does_not_inherit_write_intent(test_session): async def child(): with get_db_session() as session: # A read session: it must stay deferred. - session.execute('SELECT 1') + session.execute(sqlalchemy.text('SELECT 1')) held_during_child_read['held'] = probe_write_lock_is_held(db_file) with production_like_sessions(test_session): with get_db_session(commit=True) as session: - session.execute('SELECT 1') + session.execute(sqlalchemy.text('SELECT 1')) task = asyncio.create_task(child()) # The write session has committed; only the child's own session can hold the lock now. await task