From 350804b7aba4319dfd9eb469b27666ddc55f8373 Mon Sep 17 00:00:00 2001 From: Fokko Driesprong Date: Tue, 13 Feb 2024 23:40:34 +0100 Subject: [PATCH 1/3] Fix environment variable parsing ```bash export PYICEBERG_CATALOG__SOMETHING__S3__REGION=eu-north-1 ``` Before: ```python >>> from pyiceberg.catalog import load_catalog >>> load_catalog('something').properties {'s3': {'region': 'eu-north-1'}, ...} ``` After: ```python >>> from pyiceberg.catalog import load_catalog >>> load_catalog('something').properties {'s3.region': 'eu-north-1', ...} ``` Which correspondents with the key `s3.region` that we use. --- pyiceberg/utils/config.py | 4 ++-- tests/utils/test_config.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pyiceberg/utils/config.py b/pyiceberg/utils/config.py index 31ba0b36ed..e038005469 100644 --- a/pyiceberg/utils/config.py +++ b/pyiceberg/utils/config.py @@ -125,8 +125,8 @@ def set_property(_config: RecursiveDict, path: List[str], config_value: str) -> env_var_lower = env_var.lower() if env_var_lower.startswith(PYICEBERG.lower()): key = env_var_lower[len(PYICEBERG) :] - parts = key.split("__") - parts_normalized = [part.replace("_", "-") for part in parts] + parts = key.split("__", maxsplit=2) + parts_normalized = [part.replace('__', '.').replace("_", "-") for part in parts] set_property(config, parts_normalized, config_value) return config diff --git a/tests/utils/test_config.py b/tests/utils/test_config.py index d694e15562..13de027a05 100644 --- a/tests/utils/test_config.py +++ b/tests/utils/test_config.py @@ -41,6 +41,11 @@ def test_from_environment_variables_uppercase() -> None: assert Config().get_catalog_config("PRODUCTION") == {"uri": "https://service.io/api"} +@mock.patch.dict(os.environ, {"PYICEBERG_CATALOG__PRODUCTION__S3__REGION": "eu-north-1"}) +def test_fix_nested_objects_from_environment_variables() -> None: + assert Config().get_catalog_config("PRODUCTION") == {'s3.region': 'eu-north-1'} + + def test_from_configuration_files(tmp_path_factory: pytest.TempPathFactory) -> None: config_path = str(tmp_path_factory.mktemp("config")) with open(f"{config_path}/.pyiceberg.yaml", "w", encoding=UTF8) as file: From 4b61434ba29a487e49072dfd482927fa4d810f8e Mon Sep 17 00:00:00 2001 From: Fokko Driesprong Date: Wed, 14 Feb 2024 00:03:55 +0100 Subject: [PATCH 2/3] Add second test Co-authored-by: Hussein Awala --- tests/utils/test_config.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/utils/test_config.py b/tests/utils/test_config.py index 13de027a05..25fabe47e6 100644 --- a/tests/utils/test_config.py +++ b/tests/utils/test_config.py @@ -41,9 +41,15 @@ def test_from_environment_variables_uppercase() -> None: assert Config().get_catalog_config("PRODUCTION") == {"uri": "https://service.io/api"} -@mock.patch.dict(os.environ, {"PYICEBERG_CATALOG__PRODUCTION__S3__REGION": "eu-north-1"}) +@mock.patch.dict(os.environ, { + "PYICEBERG_CATALOG__PRODUCTION__S3__REGION": "eu-north-1", + "PYICEBERG_CATALOG__PRODUCTION__S3__ACCESS_KEY_ID": "username", +}) def test_fix_nested_objects_from_environment_variables() -> None: - assert Config().get_catalog_config("PRODUCTION") == {'s3.region': 'eu-north-1'} + assert Config().get_catalog_config("PRODUCTION") == { + 's3.region': 'eu-north-1', + 's3.access-key-id': 'username', + } def test_from_configuration_files(tmp_path_factory: pytest.TempPathFactory) -> None: From 00d1396b25d42e5651d2b616d124d91fa28048ba Mon Sep 17 00:00:00 2001 From: Fokko Driesprong Date: Wed, 14 Feb 2024 00:07:08 +0100 Subject: [PATCH 3/3] lint --- tests/utils/test_config.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/utils/test_config.py b/tests/utils/test_config.py index 25fabe47e6..5e3f72ccc6 100644 --- a/tests/utils/test_config.py +++ b/tests/utils/test_config.py @@ -41,10 +41,13 @@ def test_from_environment_variables_uppercase() -> None: assert Config().get_catalog_config("PRODUCTION") == {"uri": "https://service.io/api"} -@mock.patch.dict(os.environ, { - "PYICEBERG_CATALOG__PRODUCTION__S3__REGION": "eu-north-1", - "PYICEBERG_CATALOG__PRODUCTION__S3__ACCESS_KEY_ID": "username", -}) +@mock.patch.dict( + os.environ, + { + "PYICEBERG_CATALOG__PRODUCTION__S3__REGION": "eu-north-1", + "PYICEBERG_CATALOG__PRODUCTION__S3__ACCESS_KEY_ID": "username", + }, +) def test_fix_nested_objects_from_environment_variables() -> None: assert Config().get_catalog_config("PRODUCTION") == { 's3.region': 'eu-north-1',