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 <[email protected]>
Gerrit-Reviewer: BinĂ¡ris <[email protected]>
Gerrit-Reviewer: D3r1ck01 <[email protected]>
Gerrit-Reviewer: Xqt <[email protected]>
Gerrit-Reviewer: jenkins-bot
Gerrit-MessageType: merged
_______________________________________________
Pywikibot-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to