Merge branch 'develop' into chore/update-linting-tools

Author: sandorkertesz

docs/guide/caching.rst

@@ -236,10 +236,10 @@ Maximum-cache-size
 
 Maximum-cache-disk-usage
   The ``maximum-cache-disk-usage`` setting ensures that earthkit-data
-  leaves does not fill your disk.
-  Its values sets the maximum disk usage as % of the filesystem containing the cache
-  directory. When the disk space goes below this limit, earthkit-data triggers
-  its cache cleaning mechanism before downloading additional data.
+  does not fill your disk. It specifies the maximum disk usage (as a percentage) on the filesystem
+  containing the cache directory. When the total disk usage (so this is not the cache usage alone) goes above
+  this limit, earthkit-data triggers its cache cleaning mechanism to free up space before
+  downloading additional data.
   The value of maximum-cache-disk-usage is relative (such as "90%" or "100%").
   To disable it use None.
 

docs/release_notes/version_0.19_updates.rst

@@ -2,6 +2,13 @@
 Version 0.19 Updates
 /////////////////////////
 
+Version 0.19.2
+===============
+
+Fixes
+++++++++
+
+- Fixed a bug in the cache when files were not deleted if cache size exceeded the limit when cache-policy was set to "user" or "temporary" (:pr:`937`).
 
 
 Version 0.19.1

src/earthkit/data/core/caching.py

@@ -195,6 +195,9 @@ def _latest_date(self):
         So we do not purge files being downloaded.
         """
         with self.connection as db:
+            # TODO; this is not perfect, since it is not guaranteed that an empty file is actually being
+            # downloaded. So an old empty file will block removing the files by _decache when the cache
+            # limit is exceeded. Must be improved by at least adding a time margin
             latest = db.execute("SELECT MIN(creation_date) FROM cache WHERE size IS NULL").fetchone()[0]
             if latest is None:
                 latest = db.execute("SELECT MAX(creation_date) FROM cache WHERE size IS NOT NULL").fetchone()[0]
@@ -328,7 +331,7 @@ def _housekeeping(self, clean=False):
                     pass
 
                 if parent is None:
-                    LOG.warning(f"earthkit-data cache: orphan found: {full}")
+                    LOG.debug(f"earthkit-data cache: orphan found: {full}")
                 else:
                     LOG.debug(f"earthkit-data cache: orphan found: {full} with parent {parent}")
 
@@ -349,8 +352,7 @@ def _delete_file(self, path):
             else:
                 os.unlink(path)
         except Exception as e:
-            print(e)
-            LOG.exception("Deleting %s", path)
+            LOG.exception(f"Deleting {path}", e)
 
     def _entry_to_dict(self, entry):
         n = dict(entry)
@@ -377,6 +379,8 @@ def _delete_entry(self, entry):
                 pass
 
         if entry["size"] is None:
+            if not isinstance(entry, dict):
+                entry = self._entry_to_dict(entry)
             entry["size"] = 0
 
         path, size, owner, args = (
@@ -386,10 +390,12 @@ def _delete_entry(self, entry):
             entry["args"],
         )
 
-        LOG.warning(
-            "Deleting entry %s",
-            json.dumps(self._entry_to_dict(entry), indent=4, default=default_serialiser),
-        )
+        if logging.DEBUG >= LOG.getEffectiveLevel():
+            LOG.debug(
+                "Deleting entry %s",
+                json.dumps(self._entry_to_dict(entry), indent=4, default=default_serialiser),
+            )
+
         total = 0
 
         # First, delete child files, e.g. unzipped data
@@ -403,8 +409,9 @@ def _delete_entry(self, entry):
                 db.execute("DELETE FROM cache WHERE path=?", (path,))
             return total
 
-        LOG.warning(f"earthkit-data cache: deleting {path} ({humanize.bytes(size)})")
-        LOG.warning(f"earthkit-data cache: {owner} {args}")
+        LOG.debug(f"earthkit-data cache: deleting {path} ({humanize.bytes(size)})")
+        LOG.debug(f"earthkit-data cache: {owner} {args}")
+
         self._delete_file(path)
 
         with self.connection as db:
@@ -512,6 +519,12 @@ def _check_cache_size(self):
                 size = self._cache_size()
                 if size > maximum:
                     self._housekeeping()
+                    LOG.warning(
+                        (
+                            f"earthkit-data cache: cache size({humanize.bytes(size)}) exceeds limit"
+                            f" ({humanize.bytes(maximum)}) set by 'maximum-cache-size' config option"
+                        )
+                    )
                     self._decache(size - maximum)
 
             # Check relative limit
@@ -520,9 +533,14 @@ def _check_cache_size(self):
                 size = self._cache_size()
                 df = disk_usage(self._policy.directory())
                 if df.percent > usage:
-                    LOG.debug("Cache disk usage %s, limit %s", df.percent, usage)
                     self._housekeeping()
                     delta = (df.percent - usage) * df.total * 0.01
+                    LOG.warning(
+                        (
+                            f"earthkit-data cache: filesystem usage ({df.percent}%) exceeds limit ({usage}%)"
+                            f" set by 'maximum-cache-disk-usage' config option"
+                        )
+                    )
                     self._decache(delta)
 
     def _repr_html_(self):

src/earthkit/data/core/config.py

@@ -179,14 +179,19 @@ def validate(self, name, value):
     "maximum-cache-size": _(
         None,
         """Maximum disk space used by the earthkit-data cache (e.g.: 100G or 2T).
-        Can be set to None.""",
+        When exceeded, earthkit-data evicts older cached entries until the usage
+        is below the specified limit. Can be set to None.
+        Ignored when ``cache-policy`` is ``off``.
+        See :ref:`caching` for more information.""",
         getter="_as_bytes",
         none_ok=True,
     ),
     "maximum-cache-disk-usage": _(
         "95%",
-        """Disk usage threshold after which earthkit-data expires older cached
-        entries (% of the full disk capacity). Can be set to None.
+        """Specify maximum disk usage as a percentage of the full disk capacity on the filesystem the
+        cache is located (e.g.: 90%). When the total disk usage exceeds this limit (it's not limited to the
+        cache usage alone), earthkit-data evicts older cached entries until the usage is below the
+        specified limit. Can be set to None. Ignored when ``cache-policy`` is ``off``.
         See :ref:`caching` for more information.""",
         getter="_as_percent",
         none_ok=True,
@@ -415,7 +420,12 @@ def _set(self, name: str, *args, **kwargs):
             assert len(args) == 1
             assert len(kwargs) == 0
             value = args[0]
-            value = klass(value)
+            if value is not None:
+                if klass is bool:
+                    try:
+                        value = klass(value)
+                    except Exception as e:
+                        raise ValueError(f"Invalid value for config option '{name}': {value}") from e
 
         if klass is list:
             assert len(args) > 0

tests/core/test_config.py

@@ -64,6 +64,31 @@ def test_config_invalid():
         config.set("url-download-timeout", "A")
 
 
+@pytest.mark.parametrize(
+    "param,set_value,stored_value,raise_error",
+    [
+        ("check-out-of-date-urls", True, True, None),
+        ("check-out-of-date-urls", 1, True, None),
+        ("check-out-of-date-urls", 1.0, True, None),
+        ("check-out-of-date-urls", False, False, None),
+        ("check-out-of-date-urls", 0, False, None),
+        ("check-out-of-date-urls", 0.0, False, None),
+        ("check-out-of-date-urls", "true", True, None),
+        ("check-out-of-date-urls", "false", True, None),
+        ("check-out-of-date-urls", "1", True, None),
+        ("check-out-of-date-urls", "0", True, None),
+    ],
+)
+def test_config_set_bool(param, set_value, stored_value, raise_error):
+    with config.temporary():
+        if raise_error is None:
+            config.set(param, set_value)
+            assert config.get(param) == stored_value
+        else:
+            with pytest.raises(raise_error):
+                config.set(param, set_value)
+
+
 @pytest.mark.parametrize(
     "param,set_value,stored_value,raise_error",
     [