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."""
--
To view, visit
https://gerrit.wikimedia.org/r/c/pywikibot/core/+/858701
To unsubscribe, or for help writing mail filters, visit
https://gerrit.wikimedia.org/r/settings
Gerrit-Project: pywikibot/core
Gerrit-Branch: master
Gerrit-Change-Id: Ie5ed280e82fce93a0dc99f37e22b2aec9ec703b3
Gerrit-Change-Number: 858701
Gerrit-PatchSet: 2
Gerrit-Owner: Xqt <info(a)gno.de>
Gerrit-Reviewer: Bináris <wikiposta(a)gmail.com>
Gerrit-Reviewer: D3r1ck01 <xsavitar.wiki(a)aol.com>
Gerrit-Reviewer: Xqt <info(a)gno.de>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged