From 668857d4848c173b3a9bfaced67d8d27e5c68f03 Mon Sep 17 00:00:00 2001 From: Shea Hawkins Date: Wed, 25 Oct 2023 13:11:57 -0600 Subject: [PATCH 1/5] Implement pre-existing session support for dynamodb catalog --- pyiceberg/catalog/dynamodb.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py index 848ec03126..3464d0bb64 100644 --- a/pyiceberg/catalog/dynamodb.py +++ b/pyiceberg/catalog/dynamodb.py @@ -80,7 +80,15 @@ class DynamoDbCatalog(Catalog): def __init__(self, name: str, **properties: str): super().__init__(name, **properties) - self.dynamodb = boto3.client(DYNAMODB_CLIENT) + session = boto3.Session( + profile_name=properties.get("profile_name"), + region_name=properties.get("region_name"), + botocore_session=properties.get("botocore_session"), + aws_access_key_id=properties.get("aws_access_key_id"), + aws_secret_access_key=properties.get("aws_secret_access_key"), + aws_session_token=properties.get("aws_session_token"), + ) + self.dynamodb = session.client(DYNAMODB_CLIENT) self.dynamodb_table_name = self.properties.get(DYNAMODB_TABLE_NAME, DYNAMODB_TABLE_NAME_DEFAULT) self._ensure_catalog_table_exists_or_create() From 74f1fdf21f6c96f891bbb27c6c7c3ab5c2cc181f Mon Sep 17 00:00:00 2001 From: Shea Hawkins Date: Wed, 25 Oct 2023 13:35:52 -0600 Subject: [PATCH 2/5] Update documentation --- mkdocs/docs/configuration.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/mkdocs/docs/configuration.md b/mkdocs/docs/configuration.md index a56baff7b5..ddfd45d679 100644 --- a/mkdocs/docs/configuration.md +++ b/mkdocs/docs/configuration.md @@ -195,6 +195,19 @@ catalog: table-name: iceberg ``` +If you prefer to pass the credentials explicitly to the client instead of relying on environment variables, + +```yaml +catalog: + default: + type: dynamodb + table-name: iceberg + aws_access_key_id: + aws_secret_access_key: + aws_session_token: + region_name: +``` + # Concurrency PyIceberg uses multiple threads to parallelize operations. The number of workers can be configured by supplying a `max-workers` entry in the configuration file, or by setting the `PYICEBERG_MAX_WORKERS` environment variable. The default value depends on the system hardware and Python version. See [the Python documentation](https://docs.python.org/3/library/concurrent.futures.html#threadpoolexecutor) for more details. From d44eb118fb02cab3e2f8595ba0efa842ed7d6756 Mon Sep 17 00:00:00 2001 From: Shea Hawkins Date: Thu, 2 Nov 2023 14:06:18 -0600 Subject: [PATCH 3/5] Add unit test --- mkdocs/docs/contributing.md | 4 +++- pyiceberg/catalog/dynamodb.py | 2 +- tests/catalog/test_dynamodb.py | 28 ++++++++++++++++++++++++++-- 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/mkdocs/docs/contributing.md b/mkdocs/docs/contributing.md index 3973b763a0..8ec6dcb2d2 100644 --- a/mkdocs/docs/contributing.md +++ b/mkdocs/docs/contributing.md @@ -30,10 +30,12 @@ For the development, Poetry is used for packing and dependency management. You c pip install poetry ``` -If you have an older version of pip and virtualenv you need to update these: +Make sure you're using an up-to-date environment from venv ```bash pip install --upgrade virtualenv pip +python -m venv ./venv +source ./venv/bin/activate ``` To get started, you can run `make install`, which installs Poetry and all the dependencies of the Iceberg library. This also installs the development dependencies. If you don't want to install the development dependencies, you need to install using `poetry install --no-dev`. diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py index 3464d0bb64..4a305e8eee 100644 --- a/pyiceberg/catalog/dynamodb.py +++ b/pyiceberg/catalog/dynamodb.py @@ -118,7 +118,7 @@ def _dynamodb_table_exists(self) -> bool: return False except self.dynamodb.exceptions.InternalServerError as e: raise GenericDynamoDbError(e.message) from e - + print(response["Table"]["TableStatus"]) if response["Table"]["TableStatus"] != ACTIVE: raise GenericDynamoDbError(f"DynamoDB table for catalog {self.dynamodb_table_name} is not {ACTIVE}") else: diff --git a/tests/catalog/test_dynamodb.py b/tests/catalog/test_dynamodb.py index 582cb034e8..c0910b8af4 100644 --- a/tests/catalog/test_dynamodb.py +++ b/tests/catalog/test_dynamodb.py @@ -14,10 +14,11 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from typing import List +from typing import Any, Dict, List import pytest from moto import mock_dynamodb +from unittest import mock from pyiceberg.catalog import METADATA_LOCATION, TABLE_TYPE from pyiceberg.catalog.dynamodb import ( @@ -25,6 +26,7 @@ DYNAMODB_COL_IDENTIFIER, DYNAMODB_COL_NAMESPACE, DYNAMODB_TABLE_NAME_DEFAULT, + ACTIVE, DynamoDbCatalog, _add_property_prefix, ) @@ -46,12 +48,13 @@ def test_create_dynamodb_catalog_with_table_name(_dynamodb, _bucket_initialize: DynamoDbCatalog("test_ddb_catalog") response = _dynamodb.describe_table(TableName=DYNAMODB_TABLE_NAME_DEFAULT) assert response["Table"]["TableName"] == DYNAMODB_TABLE_NAME_DEFAULT + assert response["Table"]["TableStatus"] == ACTIVE custom_table_name = "custom_table_name" DynamoDbCatalog("test_ddb_catalog", **{"table-name": custom_table_name}) response = _dynamodb.describe_table(TableName=custom_table_name) assert response["Table"]["TableName"] == custom_table_name - + assert response["Table"]["TableStatus"] == ACTIVE @mock_dynamodb def test_create_table_with_database_location( @@ -466,3 +469,24 @@ def test_update_namespace_properties_overlap_update_removal( test_catalog.update_namespace_properties(database_name, removals, updates) # should not modify the properties assert test_catalog.load_namespace_properties(database_name) == test_properties + +def test_passing_provided_profile() -> None: + catalog_name = "test_ddb_catalog" + session_props = { + "aws_access_key_id": "abc", + "aws_secret_access_key": "def", + "aws_session_token": "ghi", + "region_name": "eu-central-1", + "botocore_session": None, + "profile_name": None + } + props = {"py-io-impl": "pyiceberg.io.fsspec.FsspecFileIO"} + props.update(session_props) + with mock.patch('boto3.Session', return_value=mock.Mock()) as mock_session: + mock_client = mock.Mock() + mock_session.return_value.client.return_value = mock_client + mock_client.describe_table.return_value = {'Table': {'TableStatus': 'ACTIVE'}} + test_catalog = DynamoDbCatalog(catalog_name, **props) + assert test_catalog.dynamodb is mock_client + mock_session.assert_called_with(**session_props) + assert test_catalog.dynamodb is mock_session().client() \ No newline at end of file From 6aca298d876b6e96ac54e597627fe2a0f3ea6d32 Mon Sep 17 00:00:00 2001 From: Shea Hawkins Date: Thu, 2 Nov 2023 14:15:06 -0600 Subject: [PATCH 4/5] EOF --- tests/catalog/test_dynamodb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/catalog/test_dynamodb.py b/tests/catalog/test_dynamodb.py index c0910b8af4..bfc51e197e 100644 --- a/tests/catalog/test_dynamodb.py +++ b/tests/catalog/test_dynamodb.py @@ -489,4 +489,4 @@ def test_passing_provided_profile() -> None: test_catalog = DynamoDbCatalog(catalog_name, **props) assert test_catalog.dynamodb is mock_client mock_session.assert_called_with(**session_props) - assert test_catalog.dynamodb is mock_session().client() \ No newline at end of file + assert test_catalog.dynamodb is mock_session().client() From 2f7d1c65d47324f6c849fca0ea263f30225d54cc Mon Sep 17 00:00:00 2001 From: waifairer Date: Thu, 2 Nov 2023 19:19:18 -0600 Subject: [PATCH 5/5] Remove print --- pyiceberg/catalog/dynamodb.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyiceberg/catalog/dynamodb.py b/pyiceberg/catalog/dynamodb.py index 4a305e8eee..3464d0bb64 100644 --- a/pyiceberg/catalog/dynamodb.py +++ b/pyiceberg/catalog/dynamodb.py @@ -118,7 +118,7 @@ def _dynamodb_table_exists(self) -> bool: return False except self.dynamodb.exceptions.InternalServerError as e: raise GenericDynamoDbError(e.message) from e - print(response["Table"]["TableStatus"]) + if response["Table"]["TableStatus"] != ACTIVE: raise GenericDynamoDbError(f"DynamoDB table for catalog {self.dynamodb_table_name} is not {ACTIVE}") else: