Repository: ecmwf/earthkit-data
Pull request #444 (2024-09-09 06:49:13)

Merge main 0.10.1 into develop


Patch:
diff --git a/docs/release_notes/version_0.10_updates.rst b/docs/release_notes/version_0.10_updates.rst
index 66522bd..a85937e 100644
--- a/docs/release_notes/version_0.10_updates.rst
+++ b/docs/release_notes/version_0.10_updates.rst
@@ -2,6 +2,15 @@ Version 0.10 Updates
 /////////////////////////
 
 
+Version 0.10.1
+===============
+
+Fixes
+++++++
+
+- Fixed memory leak in GRIB field metadata cache
+
+
 Version 0.10.0
 ===============
 
diff --git a/src/earthkit/data/core/metadata.py b/src/earthkit/data/core/metadata.py
index 851b5ad..61ab9b8 100644
--- a/src/earthkit/data/core/metadata.py
+++ b/src/earthkit/data/core/metadata.py
@@ -9,16 +9,10 @@
 
 from abc import ABCMeta
 from abc import abstractmethod
-from functools import lru_cache
 
 from earthkit.data.core.constants import DATETIME
 from earthkit.data.core.constants import GRIDSPEC
 
-try:
-    from functools import cache as memoise  # noqa
-except ImportError:
-    memoise = lru_cache
-
 
 class Metadata(metaclass=ABCMeta):
     r"""Base class to represent metadata.
@@ -54,8 +48,10 @@ class Metadata(metaclass=ABCMeta):
     def __init__(self, extra=None, cache=False):
         if extra is not None:
             self.extra = extra
-        if cache:
-            self.get = memoise(self.get)
+        if cache is False:
+            self._cache = None
+        else:
+            self._cache = dict() if cache is True else cache
 
     def __iter__(self):
         """Return an iterator over the metadata keys."""
@@ -196,12 +192,21 @@ class Metadata(metaclass=ABCMeta):
             a missing value.
 
         """
+        if self._cache is not None:
+            cache_id = (key, default, astype, raise_on_missing)
+            if cache_id in self._cache:
+                return self._cache[cache_id]
+
         if self._is_extra_key(key):
             v = self._get_extra_key(key, default=default, astype=astype)
         elif self._is_custom_key(key):
             v = self._get_custom_key(key, default=default, astype=astype, raise_on_missing=raise_on_missing)
         else:
             v = self._get(key, default=default, astype=astype, raise_on_missing=raise_on_missing)
+
+        if self._cache is not None:
+            self._cache[cache_id] = v
+
         return v
 
     @abstractmethod
diff --git a/src/earthkit/data/readers/grib/codes.py b/src/earthkit/data/readers/grib/codes.py
index c61a6e7..3a9986d 100644
--- a/src/earthkit/data/readers/grib/codes.py
+++ b/src/earthkit/data/readers/grib/codes.py
@@ -245,18 +245,19 @@ class GribField(Field):
 
     _handle = None
 
-    def __init__(self, path, offset, length, backend, manager=None):
+    def __init__(self, path, offset, length, backend, handle_manager=None, use_metadata_cache=False):
         super().__init__(backend)
         self.path = path
         self._offset = offset
         self._length = length
-        self._manager = manager
+        self._handle_manager = handle_manager
+        self._use_metadata_cache = use_metadata_cache
 
     @property
     def handle(self):
         r""":class:`CodesHandle`: Get an object providing access to the low level GRIB message structure."""
-        if self._manager is not None:
-            handle = self._manager.handle(self, self._create_handle)
+        if self._handle_manager is not None:
+            handle = self._handle_manager.handle(self, self._create_handle)
             if handle is None:
                 raise RuntimeError(f"Could not get a handle for offset={self.offset} in {self.path}")
             return handle
@@ -282,11 +283,14 @@ class GribField(Field):
 
     @cached_property
     def _metadata(self):
-        cache = False
-        if self._manager is not None:
-            cache = self._manager.use_grib_metadata_cache
+        cache = self._use_metadata_cache
+        if cache:
+            cache = self._make_metadata_cache()
         return GribFieldMetadata(self, cache=cache)
 
+    def _make_metadata_cache(self):
+        return dict()
+
     def __repr__(self):
         return "GribField(%s,%s,%s,%s,%s,%s)" % (
             self._metadata.get("shortName", None),
@@ -329,10 +333,10 @@ class GribField(Field):
     def _diag(self):
         r = r = defaultdict(int)
         try:
-            md_cache = self._metadata.get.cache_info()
+            md_cache = self._metadata._cache
+            r["metadata_cache_size"] += len(md_cache)
             r["metadata_cache_hits"] += md_cache.hits
             r["metadata_cache_misses"] += md_cache.misses
-            r["metadata_cache_size"] += md_cache.currsize
         except Exception:
             pass
         return r
diff --git a/src/earthkit/data/readers/grib/index/__init__.py b/src/earthkit/data/readers/grib/index/__init__.py
index 0ffaa85..fe59812 100644
--- a/src/earthkit/data/readers/grib/index/__init__.py
+++ b/src/earthkit/data/readers/grib/index/__init__.py
@@ -217,123 +217,106 @@ class GribMultiFieldList(GribFieldList, MultiIndex):
         FieldList._init_from_multi(self, self)
 
 
-class GribResourceManager:
-    def __init__(
-        self, owner, grib_field_policy, grib_handle_policy, grib_handle_cache_size, use_grib_metadata_cache
-    ):
-        from earthkit.data.core.settings import SETTINGS
-
-        def _get(v, name):
-            return v if v is not None else SETTINGS.get(name)
-
-        self.grib_field_policy = _get(grib_field_policy, "grib-field-policy")
-        self.grib_handle_policy = _get(grib_handle_policy, "grib-handle-policy")
-        self.grib_handle_cache_size = _get(grib_handle_cache_size, "grib-handle-cache-size")
-        self.use_grib_metadata_cache = _get(use_grib_metadata_cache, "use-grib-metadata-cache")
+class GribFieldManager:
+    def __init__(self, policy, owner):
+        self.policy = policy
+        self.cache = None
 
-        # fields
-        self.field_cache = None
-        if self.grib_field_policy == "persistent":
+        if self.policy == "persistent":
             from lru import LRU
 
             # TODO: the number of fields might only be available only later (e.g. fieldlists with
-            # an SQL index). Consider making _field_cache a cached property.
+            # an SQL index). Consider making cache a cached property.
             n = len(owner)
             if n > 0:
-                self.field_cache = LRU(n)
-
-        # handles
-        self.handle_cache = None
-        if self.grib_handle_policy == "cache":
-            if self.grib_handle_cache_size > 0:
-                from lru import LRU
+                self.cache = LRU(n)
 
-                self.handle_cache = LRU(self.grib_handle_cache_size)
-            else:
-                raise ValueError(
-                    'grib_handle_cache_size must be greater than 0 when grib_handle_policy="cache"'
-                )
-
-        self.handle_create_count = 0
         self.field_create_count = 0
 
         # check consistency
-        if self.field_cache is not None:
-            self.grib_field_policy == "persistent"
-        else:
-            self.grib_field_policy == "temporary"
-
-        if self.handle_cache is not None:
-            self.grib_handle_policy == "cache"
-        else:
-            self.grib_handle_policy in ["persistent", "temporary"]
+        if self.cache is not None:
+            assert self.policy == "persistent"
 
     def field(self, n, create):
-        if self.grib_field_policy == "persistent":
-            if n in self.field_cache:
-                return self.field_cache[n]
+        if self.cache is not None:
+            if n in self.cache:
+                return self.cache[n]
             else:
                 field = create(n)
                 self._field_created()
-                self.field_cache[n] = field
+                self.cache[n] = field
                 return field
         else:
             self._field_created()
             return create(n)
 
+    def _field_created(self):
+        self.field_create_count += 1
+
+    def diag(self):
+        r = defaultdict(int)
+        r["grib_field_policy"] = self.policy
+        if self.cache is not None:
+            r["field_cache_size"] = len(self.cache)
+
+        r["field_create_count"] = self.field_create_count
+        return r
+
+
+class GribHandleManager:
+    def __init__(self, policy, cache_size):
+        self.policy = policy
+        self.max_cache_size = cache_size
+        self.cache = None
+
+        if self.policy == "cache":
+            if self.max_cache_size > 0:
+                from lru import LRU
+
+                self.cache = LRU(self.max_cache_size)
+            else:
+                raise ValueError(
+                    'grib_handle_cache_size must be greater than 0 when grib_handle_policy="cache"'
+                )
+
+        self.handle_create_count = 0
+
+        # check consistency
+        if self.cache is not None:
+            self.policy == "cache"
+        else:
+            self.policy in ["persistent", "temporary"]
+
     def handle(self, field, create):
-        if self.grib_handle_policy == "cache":
+        if self.policy == "cache":
             key = (field.path, field._offset)
-            if key in self.handle_cache:
-                return self.handle_cache[key]
+            if key in self.cache:
+                return self.cache[key]
             else:
                 handle = create()
                 self._handle_created()
-                self.handle_cache[key] = handle
+                self.cache[key] = handle
                 return handle
-        elif self.grib_handle_policy == "persistent":
+        elif self.policy == "persistent":
             if field._handle is None:
                 field._handle = create()
                 self._handle_created()
             return field._handle
-        elif self.grib_handle_policy == "temporary":
+        elif self.policy == "temporary":
             self._handle_created()
             return create()
 
-    def _field_created(self):
-        self.field_create_count += 1
-
     def _handle_created(self):
         self.handle_create_count += 1
 
     def diag(self):
         r = defaultdict(int)
-        r["grib_field_policy"] = self.grib_field_policy
-        r["grib_handle_policy"] = self.grib_handle_policy
-        r["grib_handle_cache_size"] = self.grib_handle_cache_size
-
-        if self.field_cache is not None:
-            r["field_cache_size"] = len(self.field_cache)
-
-        r["field_create_count"] = self.field_create_count
-
-        if self.handle_cache is not None:
-            r["handle_cache_size"] = len(self.handle_cache)
+        r["grib_handle_policy"] = self.policy
+        r["grib_handle_cache_size"] = self.max_cache_size
+        if self.cache is not None:
+            r["handle_cache_size"] = len(self.cache)
 
         r["handle_create_count"] = self.handle_create_count
-
-        if self.field_cache is not None:
-            for f in self.field_cache.values():
-                if f._handle is not None:
-                    r["current_handle_count"] += 1
-
-                try:
-                    md_cache = f._diag()
-                    for k in ["metadata_cache_hits", "metadata_cache_misses", "metadata_cache_size"]:
-                        r[k] += md_cache[k]
-                except Exception:
-                    pass
-
         return r
 
 
@@ -348,10 +331,20 @@ class GribFieldListInFiles(GribFieldList):
         **kwargs,
     ):
         super().__init__(*args, **kwargs)
-        self._manager = GribResourceManager(
-            self, grib_field_policy, grib_handle_policy, grib_handle_cache_size, use_grib_metadata_cache
+
+        from earthkit.data.core.settings import SETTINGS
+
+        def _get_opt(v, name):
+            return v if v is not None else SETTINGS.get(name)
+
+        self._field_manager = GribFieldManager(_get_opt(grib_field_policy, "grib-field-policy"), self)
+        self._handle_manager = GribHandleManager(
+            _get_opt(grib_handle_policy, "grib-handle-policy"),
+            _get_opt(grib_handle_cache_size, "grib-handle-cache-size"),
         )
 
+        self._use_metadata_cache = _get_opt(use_grib_metadata_cache, "use-grib-metadata-cache")
+
     def _create_field(self, n):
         part = self.part(n)
         field = GribField(
@@ -359,7 +352,8 @@ class GribFieldListInFiles(GribFieldList):
             part.offset,
             part.length,
             self.array_backend,
-            manager=self._manager,
+            handle_manager=self._handle_manager,
+            use_metadata_cache=self._use_metadata_cache,
         )
         if field is None:
             raise RuntimeError(f"Could not get a handle for part={part}")
@@ -373,13 +367,29 @@ class GribFieldListInFiles(GribFieldList):
             if n >= len(self):
                 raise IndexError(f"Index {n} out of range")
 
-            return self._manager.field(n, self._create_field)
+            return self._field_manager.field(n, self._create_field)
 
     def __len__(self):
         return self.number_of_parts()
 
     def _diag(self):
-        return self._manager.diag()
+        r = defaultdict(int)
+        r.update(self._field_manager.diag())
+        r.update(self._handle_manager.diag())
+
+        if self._field_manager.cache is not None:
+            for f in self._field_manager.cache.values():
+                if f._handle is not None:
+                    r["current_handle_count"] += 1
+
+                if self._use_metadata_cache:
+                    try:
+                        md_cache = f._diag()
+                        for k in ["metadata_cache_hits", "metadata_cache_misses", "metadata_cache_size"]:
+                            r[k] += md_cache[k]
+                    except Exception:
+                        pass
+        return r
 
     @abstractmethod
     def part(self, n):


Test patch:
diff --git a/tests/grib/test_grib_cache.py b/tests/grib/test_grib_cache.py
index fd9c255..93323fc 100644
--- a/tests/grib/test_grib_cache.py
+++ b/tests/grib/test_grib_cache.py
@@ -16,13 +16,44 @@ from earthkit.data import settings
 from earthkit.data.testing import earthkit_examples_file
 
 
+class TestMetadataCache:
+    def __init__(self):
+        self.hits = 0
+        self.misses = 0
+        self.data = {}
+
+    def __contains__(self, key):
+        return key in self.data
+
+    def __getitem__(self, key):
+        self.hits += 1
+        return self.data[key]
+
+    def __setitem__(self, key, value):
+        self.misses += 1
+        self.data[key] = value
+
+    def __len__(self):
+        return len(self.data)
+
+
+@pytest.fixture
+def patch_metadata_cache(monkeypatch):
+    from earthkit.data.readers.grib.codes import GribField
+
+    def patched_make_metadata_cache(self):
+        return TestMetadataCache()
+
+    monkeypatch.setattr(GribField, "_make_metadata_cache", patched_make_metadata_cache)
+
+
 def _check_diag(diag, ref):
     for k, v in ref.items():
         assert diag[k] == v, f"{k}={diag[k]} != {v}"
 
 
 @pytest.mark.parametrize("handle_cache_size", [1, 5])
-def test_grib_cache_basic(handle_cache_size):
+def test_grib_cache_basic(handle_cache_size, patch_metadata_cache):
 
     with settings.temporary(
         {
@@ -35,9 +66,6 @@ def test_grib_cache_basic(handle_cache_size):
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ref_vals = ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
@@ -55,8 +83,8 @@ def test_grib_cache_basic(handle_cache_size):
         _check_diag(ds._diag(), ref)
 
         for i, f in enumerate(ds):
-            assert i in cache.field_cache, f"{i} not in cache"
-            assert id(f) == id(cache.field_cache[i]), f"{i} not the same object"
+            assert i in ds._field_manager.cache, f"{i} not in cache"
+            assert id(f) == id(ds._field_manager.cache[i]), f"{i} not the same object"
 
         _check_diag(ds._diag(), ref)
 
@@ -99,11 +127,14 @@ def test_grib_cache_basic(handle_cache_size):
         assert ds[0].handle == md._handle
 
 
-def test_grib_cache_options_1():
+def test_grib_cache_basic_non_patched():
+    """This test is the same as test_grib_cache_basic but without the patch_metadata_cache fixture.
+    So metadata cache hits and misses are not counted."""
+
     with settings.temporary(
         {
             "grib-field-policy": "persistent",
-            "grib-handle-policy": "temporary",
+            "grib-handle-policy": "cache",
             "grib-handle-cache-size": 1,
             "use-grib-metadata-cache": True,
         }
@@ -111,14 +142,80 @@ def test_grib_cache_options_1():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
+        # unique values
+        ref_vals = ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
+
+        ref = {
+            "field_cache_size": 18,
+            "field_create_count": 18,
+            "handle_cache_size": 1,
+            "handle_create_count": 18,
+            "current_handle_count": 0,
+            # "metadata_cache_hits": 0,
+            # "metadata_cache_misses": 18 * 6,
+            "metadata_cache_size": 18 * 6,
+        }
+        _check_diag(ds._diag(), ref)
+
+        for i, f in enumerate(ds):
+            assert i in ds._field_manager.cache, f"{i} not in cache"
+            assert id(f) == id(ds._field_manager.cache[i]), f"{i} not the same object"
+
+        _check_diag(ds._diag(), ref)
+
+        # unique values repeated
+        vals = ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
+
+        assert vals == ref_vals
+
+        ref = {
+            "field_cache_size": 18,
+            "field_create_count": 18,
+            "handle_cache_size": 1,
+            "handle_create_count": 18,
+            "current_handle_count": 0,
+            # "metadata_cache_hits": 18 * 4,
+            # "metadata_cache_misses": 18 * 6,
+            "metadata_cache_size": 18 * 6,
+        }
+        _check_diag(ds._diag(), ref)
+
+        # order by
+        ds.order_by(["levelist", "valid_datetime", "paramId", "levtype"])
+        ref = {
+            "field_cache_size": 18,
+            "field_create_count": 18,
+            "handle_cache_size": 1,
+            "handle_create_count": 18,
+            "current_handle_count": 0,
+            # "metadata_cache_misses": 18 * 6,
+            "metadata_cache_size": 18 * 6,
+        }
+        _check_diag(ds._diag(), ref)
+
+        # metadata object is not decoupled from the field object
+        md = ds[0].metadata()
+        assert hasattr(md, "_field")
+        assert ds[0].handle == md._handle
+
+
+def test_grib_cache_options_1(patch_metadata_cache):
+    with settings.temporary(
+        {
+            "grib-field-policy": "persistent",
+            "grib-handle-policy": "temporary",
+            "grib-handle-cache-size": 1,
+            "use-grib-metadata-cache": True,
+        }
+    ):
+        ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
+        assert len(ds) == 18
 
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
-        assert cache.field_cache is not None
-        assert cache.handle_cache is None
+        assert ds._field_manager.cache is not None
+        assert ds._handle_manager.cache is None
 
         ref = {
             "field_cache_size": 18,
@@ -133,8 +230,8 @@ def test_grib_cache_options_1():
         _check_diag(ds._diag(), ref)
 
         for i, f in enumerate(ds):
-            assert i in cache.field_cache, f"{i} not in cache"
-            assert id(f) == id(cache.field_cache[i]), f"{i} not the same object"
+            assert i in ds._field_manager.cache, f"{i} not in cache"
+            assert id(f) == id(ds._field_manager.cache[i]), f"{i} not the same object"
 
         _check_diag(ds._diag(), ref)
 
@@ -179,7 +276,7 @@ def test_grib_cache_options_1():
         _check_diag(ds._diag(), ref)
 
 
-def test_grib_cache_options_2():
+def test_grib_cache_options_2(patch_metadata_cache):
     with settings.temporary(
         {
             "grib-field-policy": "persistent",
@@ -191,14 +288,11 @@ def test_grib_cache_options_2():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
-        assert cache.field_cache is not None
-        assert cache.handle_cache is None
+        assert ds._field_manager.cache is not None
+        assert ds._handle_manager.cache is None
 
         ref = {
             "field_cache_size": 18,
@@ -213,8 +307,8 @@ def test_grib_cache_options_2():
         _check_diag(ds._diag(), ref)
 
         for i, f in enumerate(ds):
-            assert i in cache.field_cache, f"{i} not in cache"
-            assert id(f) == id(cache.field_cache[i]), f"{i} not the same object"
+            assert i in ds._field_manager.cache, f"{i} not in cache"
+            assert id(f) == id(ds._field_manager.cache[i]), f"{i} not the same object"
 
         _check_diag(ds._diag(), ref)
 
@@ -261,7 +355,7 @@ def test_grib_cache_options_2():
         _check_diag(ds._diag(), ref)
 
 
-def test_grib_cache_options_3():
+def test_grib_cache_options_3(patch_metadata_cache):
     with settings.temporary(
         {
             "grib-field-policy": "persistent",
@@ -273,14 +367,11 @@ def test_grib_cache_options_3():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
-        assert cache.field_cache is not None
-        assert cache.handle_cache is not None
+        assert ds._field_manager.cache is not None
+        assert ds._handle_manager.cache is not None
 
         ref = {
             "field_cache_size": 18,
@@ -296,8 +387,8 @@ def test_grib_cache_options_3():
         _check_diag(ds._diag(), ref)
 
         for i, f in enumerate(ds):
-            assert i in cache.field_cache, f"{i} not in cache"
-            assert id(f) == id(cache.field_cache[i]), f"{i} not the same object"
+            assert i in ds._field_manager.cache, f"{i} not in cache"
+            assert id(f) == id(ds._field_manager.cache[i]), f"{i} not the same object"
 
         _check_diag(ds._diag(), ref)
 
@@ -341,7 +432,7 @@ def test_grib_cache_options_3():
         _check_diag(ds._diag(), ref)
 
 
-def test_grib_cache_options_4():
+def test_grib_cache_options_4(patch_metadata_cache):
     with settings.temporary(
         {
             "grib-field-policy": "temporary",
@@ -353,9 +444,6 @@ def test_grib_cache_options_4():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
@@ -372,8 +460,8 @@ def test_grib_cache_options_4():
 
         _check_diag(ds._diag(), ref)
 
-        assert cache.field_cache is None
-        assert cache.handle_cache is None
+        assert ds._field_manager.cache is None
+        assert ds._handle_manager.cache is None
 
         # metadata object is not decoupled from the field object
         md = ds[0].metadata()
@@ -420,6 +508,7 @@ def test_grib_cache_options_4():
         _check_diag(
             ds[0]._diag(), {"metadata_cache_hits": 0, "metadata_cache_misses": 0, "metadata_cache_size": 0}
         )
+
         ref["field_create_count"] += 2
         ref["handle_create_count"] += 1
         _check_diag(ds._diag(), ref)
@@ -428,12 +517,13 @@ def test_grib_cache_options_4():
         _check_diag(
             ds[0]._diag(), {"metadata_cache_hits": 0, "metadata_cache_misses": 0, "metadata_cache_size": 0}
         )
+
         ref["field_create_count"] += 2
         ref["handle_create_count"] += 1
         _check_diag(ds._diag(), ref)
 
 
-def test_grib_cache_options_5():
+def test_grib_cache_options_5(patch_metadata_cache):
     with settings.temporary(
         {
             "grib-field-policy": "temporary",
@@ -445,9 +535,6 @@ def test_grib_cache_options_5():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
@@ -464,8 +551,8 @@ def test_grib_cache_options_5():
 
         _check_diag(ds._diag(), ref)
 
-        assert cache.field_cache is None
-        assert cache.handle_cache is None
+        assert ds._field_manager.cache is None
+        assert ds._handle_manager.cache is None
 
         # metadata object is not decoupled from the field object
         md = ds[0].metadata()
@@ -529,7 +616,7 @@ def test_grib_cache_options_5():
         _check_diag(ds._diag(), ref)
 
 
-def test_grib_cache_options_6():
+def test_grib_cache_options_6(patch_metadata_cache):
     with settings.temporary(
         {
             "grib-field-policy": "temporary",
@@ -541,9 +628,6 @@ def test_grib_cache_options_6():
         ds = from_source("file", earthkit_examples_file("tuv_pl.grib"))
         assert len(ds) == 18
 
-        cache = ds._manager
-        assert cache
-
         # unique values
         ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")
 
@@ -560,8 +644,8 @@ def test_grib_cache_options_6():
 
         _check_diag(ds._diag(), ref)
 
-        assert cache.field_cache is None
-        assert cache.handle_cache is not None
+        assert ds._field_manager.cache is None
+        assert ds._handle_manager.cache is not None
 
         # metadata object is not decoupled from the field object
         md = ds[0].metadata()
@@ -629,9 +713,6 @@ def test_grib_cache_use_kwargs_1():
     ds = from_source("file", earthkit_examples_file("tuv_pl.grib"), **_kwargs)
     assert len(ds) == 18
 
-    cache = ds._manager
-    assert cache
-
     # unique values
     ds.unique_values("paramId", "levelist", "levtype", "valid_datetime")