From bf804758ae4826ad1c2f70c6d043d3c06bef097f Mon Sep 17 00:00:00 2001 From: Divyanshi Purohit Date: Sun, 30 Aug 2026 23:50:11 +0530 Subject: [PATCH 1/2] Add make_reader migrate flag and matching CLI option --- src/reader/_cli.py | 9 ++++++++- src/reader/_storage/__init__.py | 8 ++++++-- src/reader/_storage/_base.py | 27 ++++++++++++++++++++++----- src/reader/core.py | 13 ++++++++++++- tests/test_reader_lifecycle.py | 33 +++++++++++++++++++++++++++++++++ 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/reader/_cli.py b/src/reader/_cli.py index b7ebd33f..b9d5967a 100644 --- a/src/reader/_cli.py +++ b/src/reader/_cli.py @@ -169,6 +169,11 @@ def convert(self, value, param, ctx): ), ) @click.option('--read-only/--no-read-only', help="Do not modify storage.") +@click.option( + '--migrate/--no-migrate', + default=True, + help="Enable or disable automatic schema migrations (default: enabled).", +) @click.option( '--plugin', 'plugins', @@ -202,7 +207,9 @@ def convert(self, value, param, ctx): ) @click.version_option(reader.__version__, message='%(prog)s %(version)s') @click.pass_context -def cli(ctx, url, feed_root, read_only, plugins, cli_plugins, reserved_name_scheme): +def cli( + ctx, url, feed_root, read_only, migrate, plugins, cli_plugins, reserved_name_scheme +): """reader command-line interface. Option defaults can be set via environment variables; diff --git a/src/reader/_storage/__init__.py b/src/reader/_storage/__init__.py index 56696b3e..86723e15 100644 --- a/src/reader/_storage/__init__.py +++ b/src/reader/_storage/__init__.py @@ -27,9 +27,13 @@ class Storage(FeedsMixin, EntriesMixin, TagsMixin, StorageBase): """ def __init__( - self, path: str, read_only: bool = False, timeout: float | None = None + self, + path: str, + read_only: bool = False, + timeout: float | None = None, + migrate: bool = True, ): - super().__init__(path, read_only, timeout) + super().__init__(path, read_only, timeout, migrate) self.changes: ChangeTrackerType = Changes(self) def make_search(self) -> SearchType: diff --git a/src/reader/_storage/_base.py b/src/reader/_storage/_base.py index 6b335fb0..4f585fcd 100644 --- a/src/reader/_storage/_base.py +++ b/src/reader/_storage/_base.py @@ -7,6 +7,7 @@ import sys from collections.abc import Callable from collections.abc import Iterable +from dataclasses import replace from functools import partial from typing import Any from typing import Self @@ -52,7 +53,13 @@ class StorageBase: chunk_size = 2**8 @wrap_exceptions(message="while opening database") - def __init__(self, path: str, read_only: bool, timeout: float | None = None): + def __init__( + self, + path: str, + read_only: bool, + timeout: float | None = None, + migrate: bool = True, + ): kwargs: dict[str, Any] = {'factory': CONNECTION_CLS} if timeout is not None: kwargs['timeout'] = timeout @@ -61,23 +68,33 @@ def __init__(self, path: str, read_only: bool, timeout: float | None = None): # has to run for every connection (in every thread), # since it's not persisted across connections self.factory = _sqlite_utils.LocalConnectionFactory( - path, self.setup_db, read_only, **kwargs + path, partial(self.setup_db, migrate=migrate), read_only, **kwargs ) def get_db(self) -> sqlite3.Connection: return self.factory() @staticmethod - def setup_db(db: sqlite3.Connection) -> None: + def setup_db(db: sqlite3.Connection, migrate: bool = True) -> None: # Private API, used by tests. - from . import MINIMUM_SQLITE_VERSION from . import REQUIRED_SQLITE_FUNCTIONS from ._schema import MIGRATION + migration = MIGRATION + if not migrate: + migration = replace( + MIGRATION, + migrations={}, + missing_suffix=( + "; reader created with migrate=False, " + "so migrations cannot run automatically;" + " pass migrate=True to allow it" + ), + ) return _sqlite_utils.setup_db( db, - migration=MIGRATION, + migration=migration, id=APPLICATION_ID, minimum_sqlite_version=MINIMUM_SQLITE_VERSION, required_sqlite_functions=REQUIRED_SQLITE_FUNCTIONS, diff --git a/src/reader/core.py b/src/reader/core.py index adca3d08..f9ba66a1 100644 --- a/src/reader/core.py +++ b/src/reader/core.py @@ -101,6 +101,7 @@ def make_reader( *, feed_root: str | None = None, read_only: bool = False, + migrate: bool = True, plugins: Iterable[PluginInput[Reader]] = DEFAULT_PLUGINS, session_timeout: TimeoutType = DEFAULT_TIMEOUT, reserved_name_scheme: Mapping[str, str] = DEFAULT_RESERVED_NAME_SCHEME, @@ -167,6 +168,11 @@ def make_reader( read_only (bool): Allow only read storage operations. + migrate (bool): + Allow storage to migrate to a newer version automatically, + if needed. If :const:`False` and a migration is needed, + :exc:`StorageError` is raised. + plugins (iterable(str or callable(Reader)) or None): An iterable of built-in plugin names (``.``), :func:`~pkgutil.resolve_name` import paths @@ -247,6 +253,9 @@ def make_reader( Built-in plugins starting with ``reader.``; use ``.`` instead. + .. versionadded:: 3.27 + The ``migrate`` keyword argument. + """ # Do as much work as possible before creating the storage. @@ -272,7 +281,9 @@ def make_reader( # See this comment for details on how it should evolve: # https://github.com/lemon24/reader/issues/168#issuecomment-642002049 - storage: StorageType = _storage or Storage(url, read_only=read_only) + storage: StorageType = _storage or Storage( + url, read_only=read_only, migrate=migrate + ) try: # For now, we're using a storage-bound search provider. diff --git a/tests/test_reader_lifecycle.py b/tests/test_reader_lifecycle.py index 304251d7..0649ffba 100644 --- a/tests/test_reader_lifecycle.py +++ b/tests/test_reader_lifecycle.py @@ -10,6 +10,7 @@ import asyncio import concurrent.futures import functools +import pathlib import sqlite3 import sys import threading @@ -19,6 +20,7 @@ from reader import SearchError from reader import StorageError +from reader._storage._sqlite_utils import HeavyMigration from utils import rename_argument pytestmark = pytest.mark.noscheduled @@ -385,3 +387,34 @@ def test_paths_read_only_private(make_reader, path): # can open, but can't create tables etc. because read only with pytest.raises(StorageError): make_reader(path, read_only=True) + + +def test_migrate(make_reader, db_path, monkeypatch): + # By default, migrations should happen automatically. + reader = make_reader(db_path) + reader.close() + + # Build a database stuck at version 1 + old_db_path = str(pathlib.Path(db_path).parent / 'old.sqlite') + db = sqlite3.connect(old_db_path) + try: + HeavyMigration(create=lambda db: None, version=1, migrations={}).migrate(db) + finally: + db.close() + + # Define a new schema (version 2) with trivial migrations from version 1 to 2 + new_migration = HeavyMigration( + create=lambda db: None, + version=2, + migrations={1: lambda db: None}, + ) + monkeypatch.setattr('reader._storage._schema.MIGRATION', new_migration) + + # migrate=False, refuse and raise StorageError. + with pytest.raises(StorageError) as excinfo: + make_reader(old_db_path, migrate=False) + assert 'migrate=False' in str(excinfo.value) + + # migrate=True (the default), migrate successfully. + reader = make_reader(old_db_path, migrate=True) + reader.close() From 1af4485ccb392d25f38379440cad148a77675e4e Mon Sep 17 00:00:00 2001 From: Divyanshi Purohit Date: Thu, 3 Sep 2026 23:50:41 +0530 Subject: [PATCH 2/2] Fix: reorder parameters and simplify test_migrate --- src/reader/_storage/__init__.py | 4 ++-- src/reader/_storage/_base.py | 2 +- tests/test_reader_lifecycle.py | 39 ++++++++++++++------------------- 3 files changed, 20 insertions(+), 25 deletions(-) diff --git a/src/reader/_storage/__init__.py b/src/reader/_storage/__init__.py index 86723e15..bf99671e 100644 --- a/src/reader/_storage/__init__.py +++ b/src/reader/_storage/__init__.py @@ -30,10 +30,10 @@ def __init__( self, path: str, read_only: bool = False, - timeout: float | None = None, migrate: bool = True, + timeout: float | None = None, ): - super().__init__(path, read_only, timeout, migrate) + super().__init__(path, read_only, migrate, timeout) self.changes: ChangeTrackerType = Changes(self) def make_search(self) -> SearchType: diff --git a/src/reader/_storage/_base.py b/src/reader/_storage/_base.py index 4f585fcd..3ac57a79 100644 --- a/src/reader/_storage/_base.py +++ b/src/reader/_storage/_base.py @@ -57,8 +57,8 @@ def __init__( self, path: str, read_only: bool, - timeout: float | None = None, migrate: bool = True, + timeout: float | None = None, ): kwargs: dict[str, Any] = {'factory': CONNECTION_CLS} if timeout is not None: diff --git a/tests/test_reader_lifecycle.py b/tests/test_reader_lifecycle.py index 0649ffba..2de5ddf0 100644 --- a/tests/test_reader_lifecycle.py +++ b/tests/test_reader_lifecycle.py @@ -390,31 +390,26 @@ def test_paths_read_only_private(make_reader, path): def test_migrate(make_reader, db_path, monkeypatch): - # By default, migrations should happen automatically. - reader = make_reader(db_path) - reader.close() - - # Build a database stuck at version 1 - old_db_path = str(pathlib.Path(db_path).parent / 'old.sqlite') - db = sqlite3.connect(old_db_path) - try: - HeavyMigration(create=lambda db: None, version=1, migrations={}).migrate(db) - finally: - db.close() - - # Define a new schema (version 2) with trivial migrations from version 1 to 2 - new_migration = HeavyMigration( + migration = HeavyMigration( create=lambda db: None, - version=2, - migrations={1: lambda db: None}, + version=1, + migrations={}, ) - monkeypatch.setattr('reader._storage._schema.MIGRATION', new_migration) + monkeypatch.setattr('reader._storage._schema.MIGRATION', migration) - # migrate=False, refuse and raise StorageError. + # Test empty case + with make_reader(db_path) as reader: + assert migration.get_version(reader._storage.get_db()) == 1 + + # Update migration definition to version 2 + migration.version = 2 + migration.migrations[1] = lambda _: None + + # Test make_reader(db_path, migrate=False) with pytest.raises(StorageError) as excinfo: - make_reader(old_db_path, migrate=False) + make_reader(db_path, migrate=False) assert 'migrate=False' in str(excinfo.value) - # migrate=True (the default), migrate successfully. - reader = make_reader(old_db_path, migrate=True) - reader.close() + # Test make_reader(db_path) + with make_reader(db_path) as reader: + assert migration.get_version(reader._storage.get_db()) == 2