Undocumented --debug-storage -> undocumented READER_DEBUG_STORAGE. #323

src/reader/_app/cli.py

@@ -35,9 +35,6 @@ def serve(config, host, port, plugin, verbose):
     if plugin:
         config['app']['plugins'] = dict.fromkeys(plugin)
 
-    # TODO: remove this once we make debug_storage a storage_arg
-    config['default']['reader'].pop('debug_storage', None)
-
     app = create_app(config)
     app.wsgi_app = make_add_response_headers_middleware(
         app.wsgi_app,

src/reader/_cli.py

@@ -1,5 +1,4 @@
 import functools
-import json
 import logging
 import os.path
 import shutil
@@ -17,7 +16,6 @@
 from ._config import make_reader_from_config
 from ._plugins import Loader
 from ._plugins import LoaderError
-from ._storage._sqlite_utils import DebugConnection
 
 
 APP_NAME = reader.__name__
@@ -45,23 +43,7 @@ def abort(message, *args, **kwargs):
     raise click.ClickException(message.format(*args, **kwargs))
 
 
-def make_reader_with_plugins(*, debug_storage=False, **kwargs):
-    if debug_storage:
-        # TODO: the web app should be able to do this too
-
-        log_debug = logging.getLogger('reader._storage').debug
-        pid = os.getpid()
-
-        class Connection(DebugConnection):
-            _io_counters = True
-
-            @staticmethod
-            def _log_method(data):
-                data['pid'] = pid
-                log_debug(json.dumps(data))
-
-        kwargs['_storage_factory'] = Connection
-
+def make_reader_with_plugins(**kwargs):
     try:
         return make_reader_from_config(**kwargs)
     except StorageError as e:
@@ -170,7 +152,7 @@ def pass_reader(fn):
     @functools.wraps(fn)
     def wrapper(*args, **kwargs):
         ctx = click.get_current_context().find_root()
-        # TODO: replace with ctx.obj.make_reader('cli') when we get rid of debug_storage
+        # TODO: replace with ctx.obj.make_reader('cli')
         reader = make_reader_with_plugins(**ctx.obj.merged('cli').get('reader', {}))
         ctx.call_on_close(reader.close)
         return fn(reader, *args, **kwargs)
@@ -215,14 +197,9 @@ def wrapper(*args, **kwargs):
         "If not provided, don't open local feeds."
     ),
 )
-@click.option(
-    '--debug-storage/--no-debug-storage',
-    hidden=True,
-    help="NOT TESTED. With -vv, log storage database calls.",
-)
 @click.version_option(reader.__version__, message='%(prog)s %(version)s')
 @click.pass_obj
-def cli(config, db, plugin, cli_plugin, feed_root, debug_storage):
+def cli(config, db, plugin, cli_plugin, feed_root):
     # TODO: mention in docs that --db/--plugin/envvars ALWAYS override the config
     # (same for wsgi envvars)
     # NOTE: we can never use click defaults for --db/--plugin, because they would override the config always
@@ -247,10 +224,6 @@ def cli(config, db, plugin, cli_plugin, feed_root, debug_storage):
     if feed_root is not None:
         config['default']['reader']['feed_root'] = feed_root
 
-    # until we make debug_storage a proper make_reader argument,
-    # and we get rid of make_reader_with_plugins
-    config['default']['reader']['debug_storage'] = debug_storage
-
     try:
         loader = Loader()
         loader.init(config, config.merged('cli').get('plugins', {}))

src/reader/_storage/__init__.py

@@ -29,9 +29,8 @@ class Storage(FeedsMixin, EntriesMixin, TagsMixin, StorageBase):
 
     """
 
-    def __init__(self, *args: Any, **kwargs: Any):
-        # FIXME: types
-        super().__init__(*args, **kwargs)
+    def __init__(self, path: str, timeout: float | None = None):
+        super().__init__(path, timeout)
         self.changes: ChangeTrackerType = Changes(self)
 
     def make_search(self) -> SearchType:

src/reader/_storage/_base.py

@@ -1,6 +1,9 @@
 from __future__ import annotations
 
+import json
+import os
 import sqlite3
+import sys
 from collections.abc import Callable
 from collections.abc import Iterable
 from functools import partial
@@ -18,6 +21,24 @@
 _T = TypeVar('_T')
 
 
+# also used by tests
+CONNECTION_CLS = sqlite3.Connection
+
+debug = os.environ.get('READER_DEBUG_STORAGE', '')
+assert set(debug) <= {'m', 't', 'i'}, f"invalid READER_DEBUG_STORAGE={debug}"
+
+if debug:  # pragma: no cover
+
+    class CONNECTION_CLS(_sqlite_utils.DebugConnection):  # type: ignore # noqa: F811
+        _set_trace = 't' in debug
+        _io_counters = 'i' in debug
+        _pid = os.getpid()
+
+        def _log_method(self, data):  # type: ignore
+            data['pid'] = self._pid
+            print('STORAGE_DEBUG', json.dumps(data), file=sys.stderr)
+
+
 wrap_exceptions = partial(_sqlite_utils.wrap_exceptions, StorageError)
 
 
@@ -26,17 +47,10 @@ class StorageBase:
     chunk_size = 2**8
 
     @wrap_exceptions(message="while opening database")
-    def __init__(
-        self,
-        path: str,
-        timeout: float | None = None,
-        factory: type[sqlite3.Connection] | None = None,
-    ):
-        kwargs: dict[str, Any] = {}
+    def __init__(self, path: str, timeout: float | None = None):
+        kwargs: dict[str, Any] = {'factory': CONNECTION_CLS}
         if timeout is not None:
             kwargs['timeout'] = timeout
-        if factory:  # pragma: no cover
-            kwargs['factory'] = factory
 
         self.factory = _sqlite_utils.LocalConnectionFactory(path, **kwargs)
         db = self.factory()

src/reader/_storage/_sqlite_utils.py

@@ -636,17 +636,12 @@ def wrapper(self, *args):
             io_counters = self._io_counters
 
         if io_counters:
-            fields = ['read_count', 'write_count', 'read_bytes', 'write_bytes']
-            try:
-                import psutil  # type: ignore
+            import psutil  # type: ignore
 
-                process = psutil.Process()
-            except ImportError:
-                process = None
-            try:
-                start_io_counters = process.io_counters()
-            except AttributeError:
-                pass
+            fields = ['read_count', 'write_count', 'read_bytes', 'write_bytes']
+            process = psutil.Process()
+            # this will fail on MacOS, but that's OK
+            start_io_counters = process.io_counters()
 
         start = time.perf_counter()
         try:
@@ -660,14 +655,11 @@ def wrapper(self, *args):
             data['duration'] = end - start
 
             if io_counters:
-                try:
-                    end_io_counters = process.io_counters()
-                    data['io_counters'] = {
-                        f: getattr(end_io_counters, f) - getattr(start_io_counters, f)
-                        for f in fields
-                    }
-                except AttributeError:
-                    pass
+                end_io_counters = process.io_counters()
+                data['io_counters'] = {
+                    f: getattr(end_io_counters, f) - getattr(start_io_counters, f)
+                    for f in fields
+                }
 
             self._log(data)
 
@@ -680,6 +672,7 @@ def _make_debug_connection_cls():  # pragma: no cover
     # typing.no_type_check not supporting classes (yet);
     # https://github.com/python/mypy/issues/607
 
+    @no_type_check
     class DebugCursor(sqlite3.Cursor):
         def _log(self, data):
             # can't rely on id(self) as it's likely to be reused
@@ -691,6 +684,7 @@ def _log(self, data):
         close = _make_debug_method_wrapper(sqlite3.Cursor.close)
         __del__ = _make_debug_method_wrapper('__del__')
 
+    @no_type_check
     class DebugConnection(sqlite3.Connection):
 
         """sqlite3 connection subclass for debugging stuff.

src/reader/core.py

@@ -101,7 +101,6 @@ def make_reader(
     reserved_name_scheme: Mapping[str, str] = DEFAULT_RESERVED_NAME_SCHEME,
     search_enabled: bool | None | Literal['auto'] = 'auto',
     _storage: StorageType | None = None,
-    _storage_factory: Any = None,
 ) -> Reader:
     """Create a new :class:`Reader`.
 
@@ -258,7 +257,7 @@ 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, factory=_storage_factory)
+    storage: StorageType = _storage or Storage(url)
 
     try:
         # For now, we're using a storage-bound search provider.

tests/test_reader_context.py

@@ -47,10 +47,12 @@ def execute(self, sql, *args, **kwargs):
 
 @pytest.fixture
 def make_reader(make_reader, monkeypatch, tmp_path):
+    monkeypatch.setattr('reader._storage._base.CONNECTION_CLS', MyConnection)
+
     monkeypatch.chdir(tmp_path)
 
     def make_reader_with_data(path):
-        reader = make_reader(path, _storage_factory=MyConnection)
+        reader = make_reader(path)
         reader._parser = parser = Parser()
         feed = parser.feed(1)
         parser.entry(1, 1, title='entry')

tests/test_storage.py

@@ -130,7 +130,7 @@ def connect(*args, **kwargs):
     assert connect.expected_timeout == 19
 
 
-def test_close():
+def test_close(monkeypatch):
     close_called = False
 
     class Connection(sqlite3.Connection):
@@ -139,7 +139,9 @@ def close(self):
             nonlocal close_called
             close_called = True
 
-    storage = Storage(':memory:', factory=Connection)
+    monkeypatch.setattr('reader._storage._base.CONNECTION_CLS', Connection)
+
+    storage = Storage(':memory:')
     storage.get_db().execute('values (1)')
 
     storage.close()
@@ -149,14 +151,16 @@ def close(self):
     storage.close()
 
 
-def test_close_error():
+def test_close_error(monkeypatch):
     class Connection(sqlite3.Connection):
         pass
 
     def execute(*args):
         raise sqlite3.ProgrammingError('unexpected error')
 
-    storage = Storage(':memory:', factory=Connection)
+    monkeypatch.setattr('reader._storage._base.CONNECTION_CLS', Connection)
+
+    storage = Storage(':memory:')
     storage.get_db().execute = execute
 
     # should not be StorageError, because it's likely a bug