Merge pull request #397 from JWCook/stale-if-error

Alias/rename old_data_on_error to stale_if_error for consistency with Cache-Control: stale-if-error (backwards-compatibile)

9 files changed, 62 insertions, 48 deletions

diff --git a/HISTORY.md b/HISTORY.md
index a20d657..969afbd 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -28,31 +28,35 @@
 
 **Other features:**
 * Add `BaseCache.update()` method as a shortcut for exporting to a different cache instance
-* Allow `BaseCache.has_url()` and `delete_url()` to optionally take arguments for `requests.Request` instead of just a URL
-* Allow `create_key()` to optionally accept arguments for `requests.Request` instead of a request object
+* Allow `BaseCache.has_url()` and `delete_url()` to optionally take parameters for `requests.Request` instead of just a URL
+* Allow `create_key()` to optionally accept parameters for `requests.Request` instead of a request object
 * Allow `match_headers` to optionally accept a list of specific headers to match
 * Add support for custom cache key callbacks with `key_fn` parameter
 * By default use blake2 instead of sha256 for generating cache keys
 * Slightly reduce size of serialized responses
 
+**Backwards-compatible API changes:**
+
+The following changes are meant to make certain behaviors more obvious for new users, without breaking existing usage:
+* For consistency with `Cache-Control: stale-if-error`, rename `old_data_on_error` to `stale_if_error`
+  * Going forward, any new options based on a standard HTTP caching feature will be named after that feature
+* For clarity about matching behavior, rename `include_get_headers` to `match_headers`
+  * References in the docs to cache keys and related behavior are now described as 'request matching'
+* For consistency with other backends, rename SQLite backend classes: `backends.sqlite.Db*` -> `SQLiteCache`, `SQLiteDict`, `SQLitePickleDict`
+* Add aliases for all previous parameter/class names for backwards-compatibility
+
 **Depedencies:**
 * Add `appdirs` as a dependency for easier cross-platform usage of user cache directories
 * Update `cattrs` from optional to required dependency
 * Update `itsdangerous` from required to optional (but recommended) dependency
-* Require requests 2.22+ and urllib3 1.25.5+
+* Require `requests` 2.22+ and `urllib3` 1.25.5+
 
 **Deprecations & removals:**
 * Drop support for python 3.6
-    * Note: python 3.6 support in 0.7.x will continue to be maintained at least until it reaches EOL (2021-12-23)
+    * **Note:** python 3.6 support in 0.7.x will continue to be maintained at least until it reaches EOL (2021-12-23)
     * Any bugfixes for 0.8 that also apply to 0.7 will be backported
 * Remove deprecated `core` module
-* Remove deprecated `BaseCache.remove_old_entries()` method (use `remove_expired_responses()` instead)
-* For clarity about matching behavior, rename `include_get_headers` to `match_headers`
-  * References in the docs to 'cache key' related behavior is now described as 'request matching' to avoid confusion
-  * Add alias for previous parameter for backwards-compatibility
-* For consistency with other backends, rename SQLite backend classes:
-  * `backends.sqlite.Db*` -> `SQLiteCache`, `SQLiteDict`, `SQLitePickleDict`
-  * Add aliases for previous names for backwards-compatibility
+* Remove deprecated `BaseCache.remove_old_entries()` method
 
 -----
 ### 0.7.5 (2021-TBD)
@@ -226,6 +230,7 @@ Thanks to [Code Shelter](https://www.codeshelter.co) and [contributors](https://
 * Deprecate `core` module; all imports should be made from top-level package instead
     * e.g.: `from requests_cache import CachedSession`
     * Imports `from requests_cache.core` will raise a `DeprecationWarning`, and will be removed in a future release
+* Rename `BaseCache.remove_old_entries()` to `remove_expired_responses()`, to match its wrapper method `CachedSession.remove_expired_responses()`
 
 **Docs & Tests:**
 * Add type annotations to main functions/methods in public API, and include in documentation on [readthedocs](https://requests-cache.readthedocs.io/en/stable/)
@@ -252,6 +257,7 @@ Project is now added to [Code Shelter](https://www.codeshelter.co)
 * Fix bulk_commit #78
 * Fix remove_expired_responses missed in __init__.py #93
 * Fix deprecation warnings #122, thanks to mbarkhau
+* Drop support for python 2.6
 
 -----
 ### 0.4.13 (2016-12-23)
diff --git a/README.md b/README.md
index 3f8c5ff..d95a7e4 100644
--- a/README.md
+++ b/README.md
@@ -102,9 +102,9 @@ session = CachedSession(
     expire_after=timedelta(days=1),    # Otherwise expire responses after one day
     allowable_methods=['GET', 'POST']  # Cache POST requests to avoid sending the same data twice
     allowable_codes=[200, 400]         # Cache 400 responses as a solemn reminder of your failures
-    match_headers=True,                # Match all request headers
     ignored_parameters=['api_key'],    # Don't match this param or save it in the cache
-    old_data_on_error=True,            # In case of request errors, use stale cache data if possible
+    match_headers=True,                # Match all request headers
+    stale_if_error=True,               # In case of request errors, use stale cache data if possible
 )
 ```
 
diff --git a/docs/user_guide/expiration.md b/docs/user_guide/expiration.md
index 0f9c62a..6ef3dbf 100644
--- a/docs/user_guide/expiration.md
+++ b/docs/user_guide/expiration.md
@@ -71,12 +71,12 @@ frequently, another that changes infrequently, and another that never changes. E
 ## Expiration and Error Handling
 In some cases, you might cache a response, have it expire, but then encounter an error when
 retrieving a new response. If you would like to use expired response data in these cases, use the
-`old_data_on_error` option.
+`stale_if_error` option.
 
 For example:
 ```python
 >>> # Cache a test response that will expire immediately
->>> session = CachedSession(old_data_on_error=True)
+>>> session = CachedSession(stale_if_error=True)
 >>> session.get('https://httpbin.org/get', expire_after=0.0001)
 >>> time.sleep(0.0001)
 ```
@@ -89,7 +89,7 @@ you get a 500. You will then get the expired cache data instead:
 True, True
 ```
 
-In addition to HTTP error codes, `old_data_on_error` also applies to python exceptions (typically a
+In addition to HTTP error codes, `stale_if_error` also applies to python exceptions (typically a
 {py:exc}`~requests.RequestException`). See `requests` documentation on
 [Errors and Exceptions](https://2.python-requests.org/en/master/user/quickstart/#errors-and-exceptions)
 for more details on request errors in general.
diff --git a/docs/user_guide/troubleshooting.md b/docs/user_guide/troubleshooting.md
index 25d4d3f..a075f5f 100644
--- a/docs/user_guide/troubleshooting.md
+++ b/docs/user_guide/troubleshooting.md
@@ -70,7 +70,7 @@ Here are some error messages you may see either in the logs or (more rarely) in
     either {py:meth}`~.BaseCache.clear` the cache or {py:meth}`~.BaseCache.remove_expired_responses`
     to get rid of the invalid responses.
 * **`Request for URL {url} failed; using cached response`:** This is just a notification that the
-  {ref}`old_data_on_error <request-errors>` option is working as intended
+  {ref}`stale_if_error <request-errors>` option is working as intended
 * **{py:exc}`~requests.RequestException`:** These are general request errors not specific to
   requests-cache. See `requests` documentation on
   [Errors and Exceptions](https://2.python-requests.org/en/master/user/quickstart/#errors-and-exceptions)
diff --git a/noxfile.py b/noxfile.py
index 941b945..de970b7 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -77,5 +77,7 @@ def livedocs(session):
 @session(python=False)
 def lint(session):
     """Run linters and code formatters via pre-commit"""
-    cmd = 'pre-commit run --all-files'
-    session.run(*cmd.split(' '))
+    cmd_1 = 'pre-commit run --all-files'
+    cmd_2 = 'mypy --install-types --non-interactive requests_cache'
+    session.run(*cmd_1.split(' '))
+    session.run(*cmd_2.split(' '))
diff --git a/requests_cache/backends/__init__.py b/requests_cache/backends/__init__.py
index 2162551..8c3b413 100644
--- a/requests_cache/backends/__init__.py
+++ b/requests_cache/backends/__init__.py
@@ -2,7 +2,7 @@
 # flake8: noqa: F401
 from inspect import signature
 from logging import getLogger
-from typing import Callable, Dict, Iterable, Type, Union
+from typing import Callable, Dict, Iterable, Optional, Type, Union
 
 from .. import get_placeholder_class, get_valid_kwargs
 from .base import BaseCache, BaseStorage
@@ -72,21 +72,21 @@ BACKEND_CLASSES = {
 }
 
 
-def init_backend(backend: BackendSpecifier = None, *args, **kwargs) -> BaseCache:
+def init_backend(cache_name: str, backend: Optional[BackendSpecifier], **kwargs) -> BaseCache:
     """Initialize a backend from a name, class, or instance"""
-    logger.debug(f'Initializing backend: {backend}')
+    logger.debug(f'Initializing backend: {backend} {cache_name}')
 
-    # Omit 'cache_name' positional arg if an equivalent backend-specific kwarg is specified
-    # TODO: The difference in parameter names here can be problematic. A better solution for this
-    #       would be nice, if it can be done without breaking backwards-compatibility.
-    if any([k in kwargs for k in CACHE_NAME_KWARGS]):
-        args = tuple()
+    # The 'cache_name' arg has a different purpose depending on the backend. If an equivalent
+    # backend-specific keyword arg is specified, handle that here to avoid conflicts. A consistent
+    # positional-only or keyword-only arg would be better, but probably not worth a breaking change.
+    cache_name_kwargs = [kwargs.pop(k) for k in CACHE_NAME_KWARGS if k in kwargs]
+    cache_name = cache_name or cache_name_kwargs[0]
 
     # Determine backend class
     if isinstance(backend, BaseCache):
         return backend
     elif isinstance(backend, type):
-        return backend(*args, **kwargs)
+        return backend(cache_name, **kwargs)
     elif not backend:
         backend = 'sqlite' if BACKEND_CLASSES['sqlite'] else 'memory'
 
@@ -94,4 +94,4 @@ def init_backend(backend: BackendSpecifier = None, *args, **kwargs) -> BaseCache
     if backend not in BACKEND_CLASSES:
         raise ValueError(f'Invalid backend: {backend}. Choose from: {BACKEND_CLASSES.keys()}')
 
-    return BACKEND_CLASSES[backend](*args, **kwargs)
+    return BACKEND_CLASSES[backend](cache_name, **kwargs)
diff --git a/requests_cache/patcher.py b/requests_cache/patcher.py
index 85f8908..14301cf 100644
--- a/requests_cache/patcher.py
+++ b/requests_cache/patcher.py
@@ -28,7 +28,7 @@ def install_cache(
     allowable_codes: Iterable[int] = (200,),
     allowable_methods: Iterable['str'] = ('GET', 'HEAD'),
     filter_fn: Callable = None,
-    old_data_on_error: bool = False,
+    stale_if_error: bool = False,
     session_factory: Type[OriginalSession] = CachedSession,
     **kwargs,
 ):
@@ -56,7 +56,7 @@ def install_cache(
                 allowable_codes=allowable_codes,
                 allowable_methods=allowable_methods,
                 filter_fn=filter_fn,
-                old_data_on_error=old_data_on_error,
+                stale_if_error=stale_if_error,
                 **kwargs,
             )
 
diff --git a/requests_cache/session.py b/requests_cache/session.py
index 7c22c03..4c6c1b2 100644
--- a/requests_cache/session.py
+++ b/requests_cache/session.py
@@ -50,21 +50,21 @@ class CacheMixin(MIXIN_BASE):
         backend: BackendSpecifier = None,
         expire_after: ExpirationTime = -1,
         urls_expire_after: Dict[str, ExpirationTime] = None,
+        cache_control: bool = False,
         allowable_codes: Iterable[int] = (200,),
         allowable_methods: Iterable[str] = ('GET', 'HEAD'),
         filter_fn: FILTER_FN = None,
-        old_data_on_error: bool = False,
-        cache_control: bool = False,
+        stale_if_error: bool = False,
         **kwargs,
     ):
-        self.cache = init_backend(backend, cache_name, **kwargs)
+        self.cache = init_backend(cache_name, backend, **kwargs)
         self.allowable_codes = allowable_codes
         self.allowable_methods = allowable_methods
         self.expire_after = expire_after
         self.urls_expire_after = urls_expire_after
-        self.filter_fn = filter_fn or (lambda r: True)
-        self.old_data_on_error = old_data_on_error
         self.cache_control = cache_control
+        self.filter_fn = filter_fn or (lambda r: True)
+        self.stale_if_error = stale_if_error or kwargs.pop('old_data_on_error', False)
 
         self.cache.name = cache_name  # Set to handle backend=<instance>
         self._request_expire_after: ExpirationTime = None
@@ -146,7 +146,7 @@ class CacheMixin(MIXIN_BASE):
         # If the response is expired, missing, or the cache is disabled, then fetch a new response
         if cached_response is None:
             response = self._send_and_cache(request, actions, **kwargs)
-        elif is_expired and self.old_data_on_error:
+        elif is_expired and self.stale_if_error:
             response = self._resend_and_ignore(request, actions, cached_response, **kwargs)
         elif is_expired:
             response = self._resend(request, actions, cached_response, **kwargs)
@@ -275,7 +275,7 @@ class CacheMixin(MIXIN_BASE):
             'urls_expire_after',
             'allowable_codes',
             'allowable_methods',
-            'old_data_on_error',
+            'stale_if_error',
             'cache_control',
         ]
         attr_strs = [f'{k}={repr(getattr(self, k))}' for k in repr_attrs]
@@ -297,6 +297,7 @@ class CachedSession(CacheMixin, OriginalSession):
             ``['pickle', 'json', 'yaml', 'bson']``.
         expire_after: Time after which cached items will expire
         urls_expire_after: Expiration times to apply for different URL patterns
+        cache_control: Use Cache-Control headers to set expiration
         allowable_codes: Only cache responses with one of these status codes
         allowable_methods: Cache only responses for one of these HTTP methods
         match_headers: Match request headers when reading from the cache; may be either a boolean
@@ -306,8 +307,7 @@ class CachedSession(CacheMixin, OriginalSession):
             indicating whether or not that response should be cached. Will be applied to both new
             and previously cached responses.
         key_fn: Function for generating custom cache keys based on request info
-        old_data_on_error: Return stale cache data if a new request raises an exception
-        cache_control: Use Cache-Control request and response headers
+        stale_if_error: Return stale cache data if a new request raises an exception
     """
 
 
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index 075fba3..45ebb74 100644
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -434,9 +434,9 @@ def test_cache_error(exception_cls, mock_session):
 
 
 def test_expired_request_error(mock_session):
-    """Without old_data_on_error (default), if there is an error while re-fetching an expired
+    """Without stale_if_error (default), if there is an error while re-fetching an expired
     response, the request should be re-raised and the expired item deleted"""
-    mock_session.old_data_on_error = False
+    mock_session.stale_if_error = False
     mock_session.expire_after = 0.01
     mock_session.get(MOCKED_URL)
     time.sleep(0.01)
@@ -447,9 +447,9 @@ def test_expired_request_error(mock_session):
     assert len(mock_session.cache.responses) == 0
 
 
-def test_old_data_on_error__exception(mock_session):
-    """With old_data_on_error, expect to get old cache data if there is an exception during a request"""
-    mock_session.old_data_on_error = True
+def test_stale_if_error__exception(mock_session):
+    """With stale_if_error, expect to get old cache data if there is an exception during a request"""
+    mock_session.stale_if_error = True
     mock_session.expire_after = 0.2
 
     assert mock_session.get(MOCKED_URL).from_cache is False
@@ -460,9 +460,9 @@ def test_old_data_on_error__exception(mock_session):
         assert response.from_cache is True and response.is_expired is True
 
 
-def test_old_data_on_error__error_code(mock_session):
-    """With old_data_on_error, expect to get old cache data if a response has an error status code"""
-    mock_session.old_data_on_error = True
+def test_stale_if_error__error_code(mock_session):
+    """With stale_if_error, expect to get old cache data if a response has an error status code"""
+    mock_session.stale_if_error = True
     mock_session.expire_after = 0.2
     mock_session.allowable_codes = (200, 404)
 
@@ -473,6 +473,12 @@ def test_old_data_on_error__error_code(mock_session):
     assert response.from_cache is True and response.is_expired is True
 
 
+def test_old_data_on_error():
+    """stale_if_error is aliased to old_data_on_error for backwards-compatibility"""
+    session = CachedSession(old_data_on_error=True, backend='memory')
+    assert session.stale_if_error is True
+
+
 def test_cache_disabled(mock_session):
     mock_session.get(MOCKED_URL)
     with mock_session.cache_disabled():