From 6c798f2adda8e2272545e62e70d47e1d704b2a9b Mon Sep 17 00:00:00 2001 From: jx2lee Date: Tue, 31 Mar 2026 00:21:25 +0900 Subject: [PATCH 1/4] cached env config --- pyiceberg/utils/config.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pyiceberg/utils/config.py b/pyiceberg/utils/config.py index ab9b549d25..d3a85b690c 100644 --- a/pyiceberg/utils/config.py +++ b/pyiceberg/utils/config.py @@ -16,6 +16,7 @@ # under the License. import logging import os +from functools import lru_cache import strictyaml @@ -179,3 +180,8 @@ def get_bool(self, key: str) -> bool | None: except ValueError as err: raise ValueError(f"{key} should be a boolean or left unset. Current value: {val}") from err return None + + +@lru_cache(maxsize=1) +def get_env_config() -> Config: + return Config() From 23815d87405062e0ec87d8e8ecf149cb4f3d034b Mon Sep 17 00:00:00 2001 From: jx2lee Date: Tue, 31 Mar 2026 00:21:44 +0900 Subject: [PATCH 2/4] remove _ENV_CONFIG in catalog --- pyiceberg/catalog/__init__.py | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index 5797e1f050..d9f06e6b71 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -66,7 +66,7 @@ RecursiveDict, TableVersion, ) -from pyiceberg.utils.config import Config, merge_config +from pyiceberg.utils.config import Config, get_env_config, merge_config from pyiceberg.utils.properties import property_as_bool from pyiceberg.view import View from pyiceberg.view.metadata import ViewVersion @@ -76,8 +76,6 @@ logger = logging.getLogger(__name__) -_ENV_CONFIG = Config() - TOKEN = "token" TYPE = "type" PY_CATALOG_IMPL = "py-catalog-impl" @@ -228,7 +226,7 @@ def _check_required_catalog_properties(name: str, catalog_type: CatalogType, con ) -def load_catalog(name: str | None = None, **properties: str | None) -> Catalog: +def load_catalog(name: str | None = None, config: Config | None = None, **properties: str | None) -> Catalog: """Load the catalog based on the properties. Will look up the properties from the config, based on the name. @@ -244,10 +242,11 @@ def load_catalog(name: str | None = None, **properties: str | None) -> Catalog: ValueError: Raises a ValueError in case properties are missing or malformed, or if it could not determine the catalog based on the properties. """ + config = config or get_env_config() if name is None: - name = _ENV_CONFIG.get_default_catalog_name() + name = config.get_default_catalog_name() - env = _ENV_CONFIG.get_catalog_config(name) + env = config.get_catalog_config(name) conf: RecursiveDict = merge_config(env or {}, cast(RecursiveDict, properties)) catalog_type: CatalogType | None @@ -279,8 +278,8 @@ def load_catalog(name: str | None = None, **properties: str | None) -> Catalog: raise ValueError(f"Could not initialize catalog with the following properties: {properties}") -def list_catalogs() -> list[str]: - return _ENV_CONFIG.get_known_catalogs() +def list_catalogs(config: Config) -> list[str]: + return config.get_known_catalogs() def delete_files(io: FileIO, files_to_delete: set[str], file_type: str) -> None: From a049f89a37003f8fbb42dccff02f8fe65b9604a9 Mon Sep 17 00:00:00 2001 From: jx2lee Date: Tue, 31 Mar 2026 00:27:50 +0900 Subject: [PATCH 3/4] test_missing_uri test to success ! --- tests/cli/test_console.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/cli/test_console.py b/tests/cli/test_console.py index 5dd1a7bc3b..b9d69c0442 100644 --- a/tests/cli/test_console.py +++ b/tests/cli/test_console.py @@ -34,19 +34,17 @@ from pyiceberg.transforms import IdentityTransform from pyiceberg.typedef import Properties from pyiceberg.types import LongType, NestedField -from pyiceberg.utils.config import Config def test_missing_uri(mocker: MockFixture, empty_home_dir_path: str) -> None: # mock to prevent parsing ~/.pyiceberg.yaml or {PYICEBERG_HOME}/.pyiceberg.yaml mocker.patch.dict(os.environ, values={"HOME": empty_home_dir_path, "PYICEBERG_HOME": empty_home_dir_path}) - mocker.patch("pyiceberg.catalog._ENV_CONFIG", return_value=Config()) runner = CliRunner() result = runner.invoke(run, ["list"]) assert result.exit_code == 1 - assert result.output == "Could not initialize catalog with the following properties: {}\n" + assert "URI missing" in result.output def test_hive_catalog_missing_uri_shows_helpful_error(mocker: MockFixture) -> None: From 8789abefafe4e4562463c37f4c150f9ffe422431 Mon Sep 17 00:00:00 2001 From: jx2lee Date: Tue, 31 Mar 2026 18:59:46 +0900 Subject: [PATCH 4/4] pass unit test / lint --- pyiceberg/catalog/__init__.py | 4 ++-- tests/catalog/test_rest.py | 8 +++++--- tests/cli/test_console.py | 2 +- tests/conftest.py | 15 --------------- tests/utils/test_config.py | 3 ++- 5 files changed, 10 insertions(+), 22 deletions(-) diff --git a/pyiceberg/catalog/__init__.py b/pyiceberg/catalog/__init__.py index d9f06e6b71..f689639c4b 100644 --- a/pyiceberg/catalog/__init__.py +++ b/pyiceberg/catalog/__init__.py @@ -226,7 +226,7 @@ def _check_required_catalog_properties(name: str, catalog_type: CatalogType, con ) -def load_catalog(name: str | None = None, config: Config | None = None, **properties: str | None) -> Catalog: +def load_catalog(name: str | None = None, **properties: str | None) -> Catalog: """Load the catalog based on the properties. Will look up the properties from the config, based on the name. @@ -242,7 +242,7 @@ def load_catalog(name: str | None = None, config: Config | None = None, **proper ValueError: Raises a ValueError in case properties are missing or malformed, or if it could not determine the catalog based on the properties. """ - config = config or get_env_config() + config = get_env_config() if name is None: name = config.get_default_catalog_name() diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index 99d1ef947b..baadedeba8 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -2036,8 +2036,8 @@ def test_catalog_from_environment_variables(catalog_config_mock: mock.Mock, rest @mock.patch.dict(os.environ, EXAMPLE_ENV) -@mock.patch("pyiceberg.catalog._ENV_CONFIG.get_catalog_config") -def test_catalog_from_environment_variables_override(catalog_config_mock: mock.Mock, rest_mock: Mocker) -> None: +@mock.patch("pyiceberg.catalog.get_env_config") +def test_catalog_from_environment_variables_override(get_env_config_mock: mock.Mock, rest_mock: Mocker) -> None: rest_mock.get( "https://other-service.io/api/v1/config", json={"defaults": {}, "overrides": {}}, @@ -2045,7 +2045,9 @@ def test_catalog_from_environment_variables_override(catalog_config_mock: mock.M ) env_config: RecursiveDict = Config._from_environment_variables({}) - catalog_config_mock.return_value = cast(RecursiveDict, env_config.get("catalog")).get("production") + mock_env_config = mock.Mock() + mock_env_config.get_catalog_config.return_value = cast(RecursiveDict, env_config.get("catalog")).get("production") + get_env_config_mock.return_value = mock_env_config catalog = cast(RestCatalog, load_catalog("production", uri="https://other-service.io/api")) assert catalog.uri == "https://other-service.io/api" diff --git a/tests/cli/test_console.py b/tests/cli/test_console.py index b9d69c0442..f1d16c6cab 100644 --- a/tests/cli/test_console.py +++ b/tests/cli/test_console.py @@ -50,7 +50,7 @@ def test_missing_uri(mocker: MockFixture, empty_home_dir_path: str) -> None: def test_hive_catalog_missing_uri_shows_helpful_error(mocker: MockFixture) -> None: mock_env_config = mocker.MagicMock() mock_env_config.get_catalog_config.return_value = {"type": "hive"} - mocker.patch("pyiceberg.catalog._ENV_CONFIG", mock_env_config) + mocker.patch("pyiceberg.catalog.get_env_config", return_value=mock_env_config) runner = CliRunner() result = runner.invoke(run, ["--catalog", "my_hive_catalog", "list"]) diff --git a/tests/conftest.py b/tests/conftest.py index b7e62c7c42..4836cce31b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -40,7 +40,6 @@ TYPE_CHECKING, Any, ) -from unittest import mock import boto3 import pytest @@ -111,20 +110,6 @@ def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: item.add_marker("unmarked") -@pytest.fixture(autouse=True, scope="session") -def _isolate_pyiceberg_config() -> None: - """Make test runs ignore your local PyIceberg config. - - Without this, tests will attempt to resolve a local ~/.pyiceberg.yaml while running pytest. - This replaces the global catalog config once at session start with an env-only config. - """ - import pyiceberg.catalog as _catalog_module - from pyiceberg.utils.config import Config - - with mock.patch.object(Config, "_from_configuration_files", return_value=None): - _catalog_module._ENV_CONFIG = Config() - - def pytest_addoption(parser: pytest.Parser) -> None: # S3 options parser.addoption( diff --git a/tests/utils/test_config.py b/tests/utils/test_config.py index 5cd6a7203a..6ed25b20fb 100644 --- a/tests/utils/test_config.py +++ b/tests/utils/test_config.py @@ -71,7 +71,8 @@ def test_from_configuration_files(tmp_path_factory: pytest.TempPathFactory) -> N file.write(yaml_str) os.environ["PYICEBERG_HOME"] = config_path - assert Config().get_catalog_config("production") == {"uri": "https://service.io/api"} + config = Config() + assert config.get_catalog_config("production") == {"uri": "https://service.io/api"} def test_lowercase_dictionary_keys() -> None: