Skip to content

Commit 9f5e2bd

Browse files
committed
Remove cache init function
1 parent 19f1274 commit 9f5e2bd

2 files changed

Lines changed: 8 additions & 71 deletions

File tree

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
@@ -52,8 +51,6 @@
5251

5352
@pytest.fixture(autouse=True)
5453
def reset_global_manifests_cache() -> None:
55-
with manifest_module._manifest_cache_lock:
56-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()
5754
clear_manifest_cache()
5855

5956

@@ -1042,45 +1039,3 @@ def test_clear_manifest_cache() -> None:
10421039
cache_after = manifest_module._manifest_cache
10431040
assert cache_after is not None, "Cache should still be enabled after clear"
10441041
assert len(cache_after) == 0, "Cache should be empty after clear"
1045-
1046-
1047-
@pytest.mark.parametrize(
1048-
"env_vars,expected_enabled,expected_size",
1049-
[
1050-
({}, True, 128), # defaults
1051-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "64"}, True, 64),
1052-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "256"}, True, 256),
1053-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "0"}, False, 0), # size=0 disables cache
1054-
],
1055-
)
1056-
def test_manifest_cache_config_valid_values(env_vars: dict[str, str], expected_enabled: bool, expected_size: int) -> None:
1057-
"""Test that valid config values are applied correctly."""
1058-
import os
1059-
1060-
with mock.patch.dict(os.environ, env_vars, clear=False):
1061-
with manifest_module._manifest_cache_lock:
1062-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()
1063-
cache = manifest_module._manifest_cache
1064-
1065-
if expected_enabled:
1066-
assert cache is not None, "Cache should be enabled"
1067-
assert cache.maxsize == expected_size, f"Cache size should be {expected_size}"
1068-
else:
1069-
assert cache is None, "Cache should be disabled"
1070-
1071-
1072-
@pytest.mark.parametrize(
1073-
"env_vars,expected_error_substring",
1074-
[
1075-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "abc"}, "manifest-cache-size should be an integer"),
1076-
({"PYICEBERG_MANIFEST_CACHE_SIZE": "-5"}, "manifest-cache-size must be >= 0"),
1077-
],
1078-
)
1079-
def test_manifest_cache_config_invalid_values(env_vars: dict[str, str], expected_error_substring: str) -> None:
1080-
"""Test that invalid config values raise ValueError with appropriate message."""
1081-
import os
1082-
1083-
with mock.patch.dict(os.environ, env_vars, clear=False):
1084-
with pytest.raises(ValueError, match=expected_error_substring):
1085-
with manifest_module._manifest_cache_lock:
1086-
manifest_module._manifest_cache = manifest_module._init_manifest_cache()

0 commit comments

Comments
 (0)