Allow override env-variables in load_catalog (#45)

* swap preference in merge_config

* typing

Author: bdilday

pyiceberg/utils/config.py

@@ -43,7 +43,7 @@ def merge_config(lhs: RecursiveDict, rhs: RecursiveDict) -> RecursiveDict:
                 new_config[rhs_key] = merge_config(lhs_value, rhs_value)
             else:
                 # Take the non-null value, with precedence on rhs
-                new_config[rhs_key] = lhs_value or rhs_value
+                new_config[rhs_key] = rhs_value or lhs_value
         else:
             # New key
             new_config[rhs_key] = rhs_value

tests/__init__.py

@@ -0,0 +1,16 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.

tests/catalog/test_rest.py

@@ -15,13 +15,16 @@
 #  specific language governing permissions and limitations
 #  under the License.
 # pylint: disable=redefined-outer-name,unused-argument
+import os
+from typing import cast
+from unittest import mock
 from uuid import UUID
 
 import pytest
 from requests_mock import Mocker
 
 import pyiceberg
-from pyiceberg.catalog import PropertiesUpdateSummary, Table
+from pyiceberg.catalog import PropertiesUpdateSummary, Table, load_catalog
 from pyiceberg.catalog.rest import RestCatalog
 from pyiceberg.exceptions import (
     NamespaceAlreadyExistsError,
@@ -38,12 +41,14 @@
 from pyiceberg.table.snapshots import Operation, Snapshot, Summary
 from pyiceberg.table.sorting import SortField, SortOrder
 from pyiceberg.transforms import IdentityTransform, TruncateTransform
+from pyiceberg.typedef import RecursiveDict
 from pyiceberg.types import (
     BooleanType,
     IntegerType,
     NestedField,
     StringType,
 )
+from pyiceberg.utils.config import Config
 
 TEST_URI = "https://iceberg-test-catalog/"
 TEST_CREDENTIALS = "client:secret"
@@ -943,3 +948,41 @@ def test_request_session_with_ssl_client_cert() -> None:
         # Missing namespace
         RestCatalog("rest", **catalog_properties)  # type: ignore
     assert "Could not find the TLS certificate file, invalid path: path_to_client_cert" in str(e.value)
+
+
+EXAMPLE_ENV = {"PYICEBERG_CATALOG__PRODUCTION__URI": TEST_URI}
+
+
+@mock.patch.dict(os.environ, EXAMPLE_ENV)
+@mock.patch("pyiceberg.catalog.Config.get_catalog_config")
+def test_catalog_from_environment_variables(catalog_config_mock: mock.Mock, rest_mock: Mocker) -> None:
+    env_config: RecursiveDict = Config._from_environment_variables({})
+    catalog_config_mock.return_value = cast(RecursiveDict, env_config.get("catalog")).get("production")
+    catalog = cast(RestCatalog, load_catalog("production"))
+    assert catalog.uri == TEST_URI
+
+
+@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:
+    rest_mock.get(
+        "https://other-service.io/api/v1/config",
+        json={"defaults": {}, "overrides": {}},
+        status_code=200,
+    )
+    env_config: RecursiveDict = Config._from_environment_variables({})
+
+    catalog_config_mock.return_value = cast(RecursiveDict, env_config.get("catalog")).get("production")
+    catalog = cast(RestCatalog, load_catalog("production", uri="https://other-service.io/api"))
+    assert catalog.uri == "https://other-service.io/api"
+
+
+def test_catalog_from_parameters_empty_env(rest_mock: Mocker) -> None:
+    rest_mock.get(
+        "https://other-service.io/api/v1/config",
+        json={"defaults": {}, "overrides": {}},
+        status_code=200,
+    )
+
+    catalog = cast(RestCatalog, load_catalog("production", uri="https://other-service.io/api"))
+    assert catalog.uri == "https://other-service.io/api"

tests/utils/test_config.py

@@ -20,7 +20,8 @@
 import pytest
 from strictyaml import as_document
 
-from pyiceberg.utils.config import Config, _lowercase_dictionary_keys
+from pyiceberg.typedef import RecursiveDict
+from pyiceberg.utils.config import Config, _lowercase_dictionary_keys, merge_config
 
 EXAMPLE_ENV = {"PYICEBERG_CATALOG__PRODUCTION__URI": "https://service.io/api"}
 
@@ -54,3 +55,10 @@ def test_lowercase_dictionary_keys() -> None:
     uppercase_keys = {"UPPER": {"NESTED_UPPER": {"YES"}}}
     expected = {"upper": {"nested_upper": {"YES"}}}
     assert _lowercase_dictionary_keys(uppercase_keys) == expected  # type: ignore
+
+
+def test_merge_config() -> None:
+    lhs: RecursiveDict = {"common_key": "abc123"}
+    rhs: RecursiveDict = {"common_key": "xyz789"}
+    result = merge_config(lhs, rhs)
+    assert result["common_key"] == rhs["common_key"]