Skip to content

Commit a87cc1c

Browse files
committed
Remove cache init function
1 parent f411547 commit a87cc1c

File tree

2 files changed

+8
-71
lines changed

2 files changed

+8
-71
lines changed

pyiceberg/manifest.py

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -893,29 +893,15 @@ def __hash__(self) -> int:
893893

894894

895895
_DEFAULT_MANIFEST_CACHE_SIZE = 128
896+
_manifest_cache_size = Config().get_int("manifest-cache-size") or _DEFAULT_MANIFEST_CACHE_SIZE
896897
_manifest_cache_lock = threading.RLock()
897-
898-
899-
def _init_manifest_cache() -> LRUCache[str, ManifestFile] | None:
900-
"""Initialize the manifest cache from config."""
901-
manifest_cache_size = Config().get_int("manifest-cache-size")
902-
if manifest_cache_size is None:
903-
manifest_cache_size = _DEFAULT_MANIFEST_CACHE_SIZE
904-
if manifest_cache_size < 0:
905-
raise ValueError(f"manifest-cache-size must be >= 0. Current value: {manifest_cache_size}")
906-
if manifest_cache_size == 0:
907-
return None
908-
return LRUCache(maxsize=manifest_cache_size)
909-
910-
911-
_manifest_cache = _init_manifest_cache()
898+
_manifest_cache: LRUCache[str, ManifestFile] = LRUCache(maxsize=_manifest_cache_size)
912899

913900

914901
def clear_manifest_cache() -> None:
915-
"""Clear the manifest cache. No-op if cache is disabled."""
916-
if _manifest_cache is not None:
917-
with _manifest_cache_lock:
918-
_manifest_cache.clear()
902+
"""Clear the manifest cache."""
903+
with _manifest_cache_lock:
904+
_manifest_cache.clear()
919905

920906

921907
def _manifests(io: FileIO, manifest_list: str) -> tuple[ManifestFile, ...]:
@@ -945,18 +931,14 @@ def _manifests(io: FileIO, manifest_list: str) -> tuple[ManifestFile, ...]:
945931
file = io.new_input(manifest_list)
946932
manifest_files = list(read_manifest_list(file))
947933

948-
if _manifest_cache is None:
949-
return tuple(manifest_files)
950-
951934
result = []
952935
with _manifest_cache_lock:
953-
cache = _manifest_cache
954936
for manifest_file in manifest_files:
955937
manifest_path = manifest_file.manifest_path
956-
if manifest_path in cache:
957-
result.append(cache[manifest_path])
938+
if manifest_path in _manifest_cache:
939+
result.append(_manifest_cache[manifest_path])
958940
else:
959-
cache[manifest_path] = manifest_file
941+
_manifest_cache[manifest_path] = manifest_file
960942
result.append(manifest_file)
961943

962944
return tuple(result)

tests/utils/test_manifest.py

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
# under the License.
1717
# pylint: disable=redefined-outer-name,arguments-renamed,fixme
1818
from tempfile import TemporaryDirectory
19-
from unittest import mock
2019

2120
import fastavro
2221
import pytest
@@ -49,8 +48,6 @@
4948

5049
@pytest.fixture(autouse=True)
5150
def reset_global_manifests_cache() -> None:
52-
with manifest_module._manifest_cache_lock:
53-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()
5451
clear_manifest_cache()
5552

5653

@@ -1001,45 +998,3 @@ def test_clear_manifest_cache() -> None:
1001998
cache_after = manifest_module._manifest_cache
1002999
assert cache_after is not None, "Cache should still be enabled after clear"
10031000
assert len(cache_after) == 0, "Cache should be empty after clear"
1004-
1005-
1006-
@pytest.mark.parametrize(
1007-
"env_vars,expected_enabled,expected_size",
1008-
[
1009-
({}, True, 128), # defaults
1010-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "64"}, True, 64),
1011-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "256"}, True, 256),
1012-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "0"}, False, 0), # size=0 disables cache
1013-
],
1014-
)
1015-
def test_manifest_cache_config_valid_values(env_vars: dict[str, str], expected_enabled: bool, expected_size: int) -> None:
1016-
"""Test that valid config values are applied correctly."""
1017-
import os
1018-
1019-
with mock.patch.dict(os.environ, env_vars, clear=False):
1020-
with manifest_module._manifest_cache_lock:
1021-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()
1022-
cache = manifest_module._manifest_cache
1023-
1024-
if expected_enabled:
1025-
assert cache is not None, "Cache should be enabled"
1026-
assert cache.maxsize == expected_size, f"Cache size should be {expected_size}"
1027-
else:
1028-
assert cache is None, "Cache should be disabled"
1029-
1030-
1031-
@pytest.mark.parametrize(
1032-
"env_vars,expected_error_substring",
1033-
[
1034-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "abc"}, "manifest-cache-size should be an integer"),
1035-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "-5"}, "manifest-cache-size must be >= 0"),
1036-
],
1037-
)
1038-
def test_manifest_cache_config_invalid_values(env_vars: dict[str, str], expected_error_substring: str) -> None:
1039-
"""Test that invalid config values raise ValueError with appropriate message."""
1040-
import os
1041-
1042-
with mock.patch.dict(os.environ, env_vars, clear=False):
1043-
with pytest.raises(ValueError, match=expected_error_substring):
1044-
with manifest_module._manifest_cache_lock:
1045-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()

0 commit comments

Comments
 (0)