Merge pull request #290 from alessio-locatelli/forbid_closed_cache_reuse

Show a warning on a cache access when a `CachedSession` context manager has been exited

HISTORY.md

@@ -1,5 +1,9 @@
 # History
 
+## (unreleased)
+
+- Now a warning is raised when a cache backend is accessed after disconnecting (after exiting the `CachedSession` context manager). (#241)
+
 ## 0.12.4 (2024-10-30)
 
 - Fixed a bug that allowed users to use `save_response()` and `from_client_response()` with an incorrect `expires` argument without throwing any warnings or errors.

aiohttp_client_cache/backends/base.py

@@ -296,6 +296,7 @@ def __init__(
     ):
         super().__init__()
         self._serializer = serializer or self._get_serializer(secret_key, salt)
+        self._closed = False
 
     def serialize(self, item: ResponseOrKey = None) -> bytes | None:
         """Serialize a URL or response into bytes"""

aiohttp_client_cache/backends/sqlite.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+import functools
+import warnings
 import asyncio
 import sqlite3
 from contextlib import asynccontextmanager
@@ -24,6 +26,15 @@
 logger = getLogger(__name__)
 
 
+closed_session_warning = functools.partial(
+    warnings.warn,
+    'Cache access after closing the `Cachedsession` context manager '
+    + 'is discouraged and can be forbidden in the future to prevent '
+    + 'errors related to a closed database connection.',
+    stacklevel=2,
+)
+
+
 class SQLiteBackend(CacheBackend):
     """Async cache backend for `SQLite <https://www.sqlite.org>`_
 
@@ -99,7 +110,11 @@ async def get_connection(self, commit: bool = False) -> AsyncIterator[aiosqlite.
             if self._connection is None:
                 self._connection = await aiosqlite.connect(self.filename, **self.connection_kwargs)
                 await self._init_db()
+
         yield self._connection
+
+        if self._closed:
+            closed_session_warning()
         if commit and not bulk_commit_var.get():
             await self._connection.commit()
 
@@ -154,6 +169,7 @@ async def clear(self):
 
     async def close(self):
         """Close any open connections"""
+        self._closed = True
         async with self._lock:
             if self._connection is not None:
                 await self._connection.close()
@@ -187,6 +203,8 @@ async def keys(self) -> AsyncIterable[str]:
 
     async def read(self, key: str) -> ResponseOrKey:
         async with self.get_connection() as db:
+            if self._closed:
+                closed_session_warning()
             cursor = await db.execute(f'SELECT value FROM `{self.table_name}` WHERE key=?', (key,))
             row = await cursor.fetchone()
             return row[0] if row else None

docs/backends.md

@@ -108,3 +108,47 @@ You can then use your custom backend in a {py:class}`.CachedSession` with the `c
 ```python
 >>> session = CachedSession(cache=CustomCache())
 ```
+
+## Can I reuse a cache backend instance across multiple `CachedSession` instances?
+
+First of all, read the following warning in the [`aiohttp` documentation](https://docs.aiohttp.org/en/stable/client_quickstart.html#make-a-request) to make sure you need multiple `CachedSession` or `Session`:
+
+> Don’t create a session per request. Most likely you need a session per application which performs all requests together.
+>
+> More complex cases may require a session per site, e.g. one for Github and other one for Facebook APIs. Anyway making a session for every request is a very bad idea.
+>
+> A session contains a connection pool inside. Connection reusage and keep-alive (both are on by default) may speed up total performance.
+
+It depends on your application design, but you have at least three options:
+
+- Create a cache instance per `CachedSession`:
+
+  ```py
+  github_api = CachedSession(SQLiteBackend())
+  gitlab_api = CachedSession(SQLiteBackend())
+  ```
+
+- Create a single cache instance, but keep all `CachedSession` open:
+
+  ```py
+  cache_backend = CacheBackend()
+  sessions_pool = [...]  # Manage multiple `Cachedsession` with a single cached backend.
+
+  # Make requests...
+
+  for s in sessions:
+      await s.close()
+  ```
+
+- Override the `close` method and close the cache backed manually:
+
+  ```py
+  class CustomSQLiteBackend(SQLiteBackend):
+    def close(self): pass  # Override to prevent disconnecting.
+
+  cache = CustomSQLiteBackend()
+  async with CachedSession(cache): ...
+
+  # It is up to you to close the connection when you exit the application.
+  await cache._connection.close()
+  ```