jenkins-bot has submitted this change. ( https://gerrit.wikimedia.org/r/c/pywikibot/core/+/858701 )
Change subject: [IMPR] Ignore OSError if cache cannot be written ......................................................................
[IMPR] Ignore OSError if cache cannot be written
OSError could be risen if for example the disk space is too low. Ignore OSError in api.CachedRequest._write_cache() in that case. Note: Pywikibot can slow down in such case
- suppress OSError in CachedRequest._write_cache() but delete created cache file in such case - use pathlib.Path in _get_cache_dir, _make_dir, _cachefile_path, _load_cache and _write_cache - use pathlib.Path in corresponding maintenance/cache.py script - update tests
Change-Id: Ie5ed280e82fce93a0dc99f37e22b2aec9ec703b3 --- M tests/dry_api_tests.py M pywikibot/data/api/_requests.py M scripts/maintenance/cache.py 3 files changed, 84 insertions(+), 33 deletions(-)
Approvals: Xqt: Looks good to me, approved jenkins-bot: Verified
diff --git a/pywikibot/data/api/_requests.py b/pywikibot/data/api/_requests.py index 8d656c8..d840b9a 100644 --- a/pywikibot/data/api/_requests.py +++ b/pywikibot/data/api/_requests.py @@ -13,7 +13,9 @@ import re import traceback from collections.abc import MutableMapping +from contextlib import suppress from email.mime.nonmultipart import MIMENonMultipart +from pathlib import Path from typing import Any, Optional, Union from urllib.parse import unquote, urlencode from warnings import warn @@ -1133,32 +1135,39 @@ raise NotImplementedError('CachedRequest cannot be created simply.')
@classmethod - def _get_cache_dir(cls) -> str: + def _get_cache_dir(cls) -> Path: """ Return the base directory path for cache entries.
The directory will be created if it does not already exist.
+ .. versionchanged:: 8.0 + return a `pathlib.Path` object. + :return: base directory path for cache entries """ - path = os.path.join(config.base_dir, - f'apicache-py{PYTHON_VERSION[0]:d}') + path = Path(config.base_dir, f'apicache-py{PYTHON_VERSION[0]:d}') cls._make_dir(path) cls._get_cache_dir = classmethod(lambda c: path) # cache the result return path
@staticmethod - def _make_dir(dir_name: str) -> str: + def _make_dir(dir_name: Union[str, Path]) -> Path: """Create directory if it does not exist already.
.. versionchanged:: 7.0 Only `FileExistsError` is ignored but other OS exceptions can be still raised + .. versionchanged:: 8.0 + use *dir_name* as str or `pathlib.Path` object but always + return a Path object.
:param dir_name: directory path - :return: unmodified directory name for test purpose + :return: directory path as `pathlib.Path` object for test purpose """ - os.makedirs(dir_name, exist_ok=True) + if isinstance(dir_name, str): + dir_name = Path(dir_name) + dir_name.mkdir(exist_ok=True) return dir_name
def _uniquedescriptionstr(self) -> str: @@ -1189,9 +1198,13 @@ self._uniquedescriptionstr().encode('utf-8') ).hexdigest()
- def _cachefile_path(self): - return os.path.join(CachedRequest._get_cache_dir(), - self._create_file_name()) + def _cachefile_path(self) -> Path: + """Create the cachefile path. + + .. versionchanged:: 8.0 + return a `pathlib.Path` object. + """ + return CachedRequest._get_cache_dir() / self._create_file_name()
def _expired(self, dt): return dt + self.expiry < datetime.datetime.utcnow() @@ -1204,30 +1217,39 @@ self._add_defaults() try: filename = self._cachefile_path() - with open(filename, 'rb') as f: + with filename.open('rb') as f: uniquedescr, self._data, self._cachetime = pickle.load(f) + if uniquedescr != self._uniquedescriptionstr(): raise RuntimeError('Expected unique description for the cache ' 'entry is different from file entry.') + if self._expired(self._cachetime): self._data = None return False - pywikibot.debug('{}: cache hit ({}) for API request: {}' - .format(self.__class__.__name__, filename, - uniquedescr)) - return True + + pywikibot.debug( + f'{type(self).__name__}: cache ({filename.parent}) hit\n' + f'{filename.name}, API request:\n{uniquedescr}') + except OSError: - # file not found - return False + pass # file not found except Exception as e: pywikibot.info(f'Could not load cache: {e!r}') - return False + else: + return True + + return False
def _write_cache(self, data) -> None: """Write data to self._cachefile_path().""" data = (self._uniquedescriptionstr(), data, datetime.datetime.utcnow()) - with open(self._cachefile_path(), 'wb') as f: + path = self._cachefile_path() + with suppress(OSError), path.open('wb') as f: pickle.dump(data, f, protocol=config.pickle_protocol) + return + # delete invalid cache entry + path.unlink()
def submit(self): """Submit cached request.""" diff --git a/scripts/maintenance/cache.py b/scripts/maintenance/cache.py index ae4a44f..2340fab 100755 --- a/scripts/maintenance/cache.py +++ b/scripts/maintenance/cache.py @@ -73,6 +73,7 @@ import os import pickle import sys +from pathlib import Path
import pywikibot from pywikibot.data import api @@ -93,7 +94,7 @@
"""A Request cache entry."""
- def __init__(self, directory, filename): + def __init__(self, directory: str, filename: str): """Initializer.""" self.directory = directory self.filename = filename @@ -104,24 +105,31 @@
def __repr__(self): """Representation of object.""" - return self._cachefile_path() + return str(self._cachefile_path())
def _create_file_name(self): """Filename of the cached entry.""" return self.filename
- def _get_cache_dir(self): - """Directory of the cached entry.""" - return self.directory + def _get_cache_dir(self) -> Path: + """Directory of the cached entry.
- def _cachefile_path(self): - """Return cache file path.""" - return os.path.join(self._get_cache_dir(), - self._create_file_name()) + .. versionchanged:: 8.0 + return a `pathlib.Path` object. + """ + return Path(self.directory) + + def _cachefile_path(self) -> Path: + """Return cache file path. + + .. versionchanged:: 8.0 + return a `pathlib.Path` object. + """ + return self._get_cache_dir() / self._create_file_name()
def _load_cache(self): """Load the cache entry.""" - with open(self._cachefile_path(), 'rb') as f: + with self._cachefile_path().open('rb') as f: self.key, self._data, self._cachetime = pickle.load(f) return True
@@ -206,7 +214,7 @@
def _delete(self): """Delete the cache entry.""" - os.remove(self._cachefile_path()) + self._cachefile_path().unlink()
def process_entries(cache_path, func, use_accesstime=None, output_func=None, @@ -266,8 +274,7 @@ try: entry._load_cache() except ValueError: - pywikibot.error('Failed loading {}'.format( - entry._cachefile_path())) + pywikibot.error(f'Failed loading {entry._cachefile_path()}') pywikibot.exception() continue
diff --git a/tests/dry_api_tests.py b/tests/dry_api_tests.py index 53ebd97..a110d8a 100755 --- a/tests/dry_api_tests.py +++ b/tests/dry_api_tests.py @@ -6,6 +6,7 @@ # Distributed under the terms of the MIT license. # import datetime +from pathlib import Path from unittest.mock import patch
import pywikibot @@ -18,7 +19,7 @@ from pywikibot.exceptions import Error from pywikibot.family import Family from pywikibot.login import LoginStatus -from pywikibot.tools import suppress_warnings +from pywikibot.tools import suppress_warnings, PYTHON_VERSION from tests import join_images_path from tests.aspects import ( DefaultDrySiteTestCase, @@ -103,7 +104,8 @@ def test_get_cache_dir(self): """Test that 'apicache' is in the cache dir.""" retval = self.req._get_cache_dir() - self.assertIn('apicache', retval) + self.assertIsInstance(retval, Path) + self.assertIn(f'apicache-py{PYTHON_VERSION[0]:d}', retval.parts)
def test_create_file_name(self): """Test the file names for the cache."""
pywikibot-commits@lists.wikimedia.org