diff --git a/CHANGES.md b/CHANGES.md index 950000b..a91fd72 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,7 +1,12 @@ +### 1.4.0 + +- Improved DynamoDB Item-related Exceptions for `GetItem`, + `put_but_raise_if_exists`, and `versioned_diffed_update_item`. + ### 1.3.3 - Allow any characters for attribute names in `add_variables_to_expression`. - * We have a lot of snake_cased attribute names. We should be able to use this function with those. + - We have a lot of snake_cased attribute names. We should be able to use this function with those. ### 1.3.2 diff --git a/tests/xoto3/dynamodb/exceptions_test.py b/tests/xoto3/dynamodb/exceptions_test.py new file mode 100644 index 0000000..72dbb1a --- /dev/null +++ b/tests/xoto3/dynamodb/exceptions_test.py @@ -0,0 +1,38 @@ +import pytest + +from xoto3.dynamodb.exceptions import ( + get_item_exception_type, + raise_if_empty_getitem_response, + ItemNotFoundException, + ItemAlreadyExistsException, + AlreadyExistsException, +) + + +def test_dynamically_named_exceptions_names_and_caches_different_types(): + media_not_found = get_item_exception_type("Media", ItemNotFoundException) + + assert media_not_found.__name__ == "MediaNotFoundException" + assert issubclass(media_not_found, ItemNotFoundException) + + assert get_item_exception_type("Media", ItemNotFoundException) is media_not_found + + media_already_exists = get_item_exception_type("Media", ItemAlreadyExistsException) + assert issubclass(media_already_exists, AlreadyExistsException) + assert media_already_exists.__name__ == "MediaAlreadyExistsException" + assert not issubclass(media_already_exists, ItemNotFoundException) + + +def test_raises_uses_nicename(): + with pytest.raises(ItemNotFoundException) as infe_info: + raise_if_empty_getitem_response(dict(), nicename="Duck") + assert infe_info.value.__class__.__name__ == "DuckNotFoundException" + + +def test_raises_includes_key_and_table_name(): + with pytest.raises(ItemNotFoundException) as infe_info: + raise_if_empty_getitem_response( + dict(), nicename="Plant", table_name="Greenhouse", key=dict(id="p0001") + ) + assert infe_info.value.key == dict(id="p0001") + assert infe_info.value.table_name == "Greenhouse" diff --git a/tests/xoto3/dynamodb/put_test.py b/tests/xoto3/dynamodb/put_test.py new file mode 100644 index 0000000..3792e8b --- /dev/null +++ b/tests/xoto3/dynamodb/put_test.py @@ -0,0 +1,39 @@ +import os +import random + +import pytest +import boto3 + +import xoto3.dynamodb.put as xput + +from tests.xoto3.dynamodb.testing_utils import make_pytest_put_fixture_for_table + + +XOTO3_INTEGRATION_TEST_ID_TABLE_NAME = os.environ.get( + "XOTO3_INTEGRATION_TEST_DYNAMODB_ID_TABLE_NAME" +) + + +_INTEGRATION_ID_TABLE = ( + boto3.resource("dynamodb").Table(XOTO3_INTEGRATION_TEST_ID_TABLE_NAME) + if XOTO3_INTEGRATION_TEST_ID_TABLE_NAME + else None +) +integration_table_put = make_pytest_put_fixture_for_table(_INTEGRATION_ID_TABLE) + + +@pytest.mark.skipif( + not XOTO3_INTEGRATION_TEST_ID_TABLE_NAME, reason="No integration id table was defined" +) +def test_put_already_exists(integration_table_put): + + random_key = dict(id=str(random.randint(9999999, 999999999999999999999))) + item = dict(random_key, test_item_please_ignore=True) + integration_table_put(item) + + with pytest.raises(xput.ItemAlreadyExistsException) as ae_info: + xput.put_but_raise_if_exists( + _INTEGRATION_ID_TABLE, dict(item, new_attribute="testing attr"), nicename="TestThing" + ) + + assert ae_info.value.__class__.__name__ == "TestThingAlreadyExistsException" diff --git a/tests/xoto3/dynamodb/testing_utils.py b/tests/xoto3/dynamodb/testing_utils.py new file mode 100644 index 0000000..b4f7413 --- /dev/null +++ b/tests/xoto3/dynamodb/testing_utils.py @@ -0,0 +1,22 @@ +import pytest + +from xoto3.dynamodb.put import PutItem +from xoto3.dynamodb.types import TableResource, InputItem +from xoto3.dynamodb.utils.table import extract_key_from_item + + +def make_pytest_put_fixture_for_table(table: TableResource): + @pytest.fixture + def put_item_fixture(): + keys_put = list() + + def _put_item(item: InputItem): + keys_put.append(extract_key_from_item(table, item)) + PutItem(table, item) + + yield _put_item + + for key in keys_put: + table.delete_item(Key=key) + + return put_item_fixture diff --git a/tests/xoto3/dynamodb/update/versioned_test.py b/tests/xoto3/dynamodb/update/versioned_test.py index 6cc3aec..4a60ac9 100644 --- a/tests/xoto3/dynamodb/update/versioned_test.py +++ b/tests/xoto3/dynamodb/update/versioned_test.py @@ -26,7 +26,7 @@ def test_versioned_diffed_update_item(): def test_transform(item: Item) -> Item: item.pop("to_remove") - item["new"] = "value" + item["new"] = "abcxyz" return item called_times = [0] @@ -148,14 +148,20 @@ def fail_forever_updater_func( assert set_attrs and "item_version" in set_attrs raise ClientError({"Error": {"Code": "ConditionalCheckFailedException"}}, "update_item") - with pytest.raises(VersionedUpdateFailure): + with pytest.raises(VersionedUpdateFailure) as ve_info: versioned_diffed_update_item( FakeTableResource(), test_transform, test_item, - get_item=lambda x, y: test_item, + get_item=lambda x, y: dict(test_item, item_version=called_times[0]), update_item=fail_forever_updater_func, ) + ve = ve_info.value + assert ve.table_name == "Fake" + assert ve.key == test_item + assert ve.update_arguments["remove_attrs"] == {"to_remove"} + assert ve.update_arguments["set_attrs"]["item_version"] == 25 + assert ve.update_arguments["set_attrs"]["new"] == "abcxyz" assert called_times[0] == DEFAULT_MAX_ATTEMPTS_BEFORE_FAILURE diff --git a/xoto3/__about__.py b/xoto3/__about__.py index d3f01e8..1ddca1a 100644 --- a/xoto3/__about__.py +++ b/xoto3/__about__.py @@ -1,4 +1,4 @@ """xoto3""" -__version__ = "1.3.3" +__version__ = "1.4.0" __author__ = "Peter Gaultney" __author_email__ = "pgaultney@xoi.io" diff --git a/xoto3/cloudformation/__init__.py b/xoto3/cloudformation/__init__.py index c2e20b3..a9138f8 100644 --- a/xoto3/cloudformation/__init__.py +++ b/xoto3/cloudformation/__init__.py @@ -13,7 +13,8 @@ def _get_cached_stack(stack_name: str): if stack_name not in _STACKS: - _STACKS[stack_name] = _CF_RESOURCE().Stack(stack_name) + stack = _CF_RESOURCE().Stack(stack_name) # type: ignore + _STACKS[stack_name] = stack return _STACKS[stack_name] diff --git a/xoto3/cloudwatch/metrics.py b/xoto3/cloudwatch/metrics.py index d62da11..042a3ed 100644 --- a/xoto3/cloudwatch/metrics.py +++ b/xoto3/cloudwatch/metrics.py @@ -105,7 +105,7 @@ def __call__( metric_dict = dict(Namespace=self.namespace, MetricData=[metric_data]) logger.debug("put_metric", extra=dict(put_metric=metric_dict)) - CLOUDWATCH_CLIENT().put_metric_data(**metric_dict) + CLOUDWATCH_CLIENT().put_metric_data(**metric_dict) # type: ignore PutMetricReturner = ty.Callable[..., float] diff --git a/xoto3/dynamodb/exceptions.py b/xoto3/dynamodb/exceptions.py index e815c43..c82f508 100644 --- a/xoto3/dynamodb/exceptions.py +++ b/xoto3/dynamodb/exceptions.py @@ -1,27 +1,67 @@ """Exceptions for our Dynamo usage""" +from typing import Optional, TypeVar, Type, Dict, Tuple import botocore.exceptions +from .types import ItemKey + class DynamoDbException(Exception): """Wrapping error responses from Dynamo DB""" - pass +class DynamoDbItemException(DynamoDbException): + def __init__(self, msg: str, *, key: Optional[ItemKey] = None, table_name: str = "", **kwargs): + self.__dict__.update(kwargs) + self.key = key + self.table_name = table_name + super().__init__(msg) + + +class AlreadyExistsException(DynamoDbItemException): + """Deprecated - prefer ItemAlreadyExistsException""" -class AlreadyExistsException(DynamoDbException): - pass +class ItemAlreadyExistsException(AlreadyExistsException): + """Backwards-compatible, more consistent name""" -class ItemNotFoundException(DynamoDbException): + +class ItemNotFoundException(DynamoDbItemException): """Being more specific that an item was not found""" -def raise_if_empty_getitem_response(getitem_response: dict, nicename="Item", key=None): - """Boto3 does not raise any error if the item could not be found""" +X = TypeVar("X", bound=DynamoDbItemException) + + +_GENERATED_ITEM_EXCEPTION_TYPES: Dict[Tuple[str, str], type] = { + ("Item", "ItemNotFoundException"): ItemNotFoundException +} + + +def get_item_exception_type(item_name: str, base_exc: Type[X]) -> Type[X]: + if not item_name: + return base_exc + base_name = base_exc.__name__ + exc_key = (item_name, base_exc.__name__) + if exc_key not in _GENERATED_ITEM_EXCEPTION_TYPES: + exc_minus_Item = base_name[4:] if base_name.startswith("Item") else base_name + _GENERATED_ITEM_EXCEPTION_TYPES[exc_key] = type( + f"{item_name}{exc_minus_Item}", (base_exc,), dict() + ) + return _GENERATED_ITEM_EXCEPTION_TYPES[exc_key] + + +def raise_if_empty_getitem_response( + getitem_response: dict, nicename="Item", key=None, table_name: str = "" +): + """Boto3 does not raise any error if the item could not be found. This + is not what we want in many cases, and it's convenient to have a + standard way of identifying ItemNotFound. + """ if "Item" not in getitem_response: - if "id" in key and len(key) == 1: - key = key["id"] - raise ItemNotFoundException(f"{nicename} '{key}' does not exist!") + key_value = next(iter(key.values())) if key and len(key) == 1 else key + raise get_item_exception_type(nicename, ItemNotFoundException)( + f"{nicename} '{key_value}' does not exist!", key=key, table_name=table_name + ) def translate_clienterrors(client_error: botocore.exceptions.ClientError, names_to_messages: dict): diff --git a/xoto3/dynamodb/get.py b/xoto3/dynamodb/get.py index c9ee054..ea5ad82 100644 --- a/xoto3/dynamodb/get.py +++ b/xoto3/dynamodb/get.py @@ -16,7 +16,7 @@ def GetItem(Table: TableResource, Key: ItemKey, nicename="Item", **kwargs) -> It """ logger.debug(f"Get{nicename} {Key} from Table {Table.name}") response = Table.get_item(Key={**Key}, **kwargs) - raise_if_empty_getitem_response(response, nicename, key=Key) + raise_if_empty_getitem_response(response, nicename=nicename, key=Key, table_name=Table.name) return response["Item"] diff --git a/xoto3/dynamodb/put.py b/xoto3/dynamodb/put.py index 3dec436..a3892c4 100644 --- a/xoto3/dynamodb/put.py +++ b/xoto3/dynamodb/put.py @@ -12,18 +12,17 @@ from xoto3.errors import catch_named_clienterrors from .conditions import item_not_exists -from .exceptions import AlreadyExistsException +from .exceptions import ItemAlreadyExistsException, get_item_exception_type from .types import InputItem, TableResource, Item from .prewrite import dynamodb_prewrite -from .utils.table import table_primary_keys +from .utils.table import table_primary_keys, extract_key_from_item from .get import strongly_consistent_get_item logger = getLogger(__name__) def PutItem(Table: TableResource, Item: InputItem, *, nicename="Item", **kwargs) -> InputItem: - """Convenience wrapper that makes your item Dynamo-safe before writing. - """ + """Convenience wrapper that makes your item Dynamo-safe before writing.""" logger.debug(f"Put{nicename} into table {Table.name}", extra=dict(json=dict(item=Item))) Table.put_item(Item=dynamodb_prewrite(Item), **kwargs) return Item @@ -53,13 +52,19 @@ def put_unless_exists(Table: TableResource, item: InputItem) -> Tuple[Optional[E def put_but_raise_if_exists( Table: TableResource, item: InputItem, *, nicename: str = "Item" ) -> InputItem: - """Wrapper for put_item that raises AlreadyExistsException if the item exists. + """Wrapper for put_item that raises ItemAlreadyExistsException if the item exists, + or a custom-generated subclass thereof if you have provided a better "nicename". If successful, just returns the passed item. + """ already_exists_cerror, _response = put_unless_exists(Table, item) if already_exists_cerror: - raise AlreadyExistsException(f"{nicename} already exists and was not overwritten!") + raise get_item_exception_type(nicename, ItemAlreadyExistsException)( + f"{nicename} already exists and was not overwritten!", + key=extract_key_from_item(Table, item), + table_name=Table.name, + ) return item @@ -67,7 +72,7 @@ def put_or_return_existing(table: TableResource, item: InputItem) -> Union[Item, try: put_but_raise_if_exists(table, item) return item - except AlreadyExistsException: + except ItemAlreadyExistsException: return strongly_consistent_get_item( table, {key: item[key] for key in table_primary_keys(table)} ) diff --git a/xoto3/dynamodb/update/utils.py b/xoto3/dynamodb/update/utils.py index 2ef4efb..030f990 100644 --- a/xoto3/dynamodb/update/utils.py +++ b/xoto3/dynamodb/update/utils.py @@ -19,6 +19,7 @@ def logged_update_item( except Exception as e: # verbose logging if an error occurs logger.info("UpdateItem arguments", extra=dict(json=dict(update_args))) + e.update_item_arguments = update_args # type: ignore raise e diff --git a/xoto3/dynamodb/update/versioned.py b/xoto3/dynamodb/update/versioned.py index f9141d1..a31de81 100644 --- a/xoto3/dynamodb/update/versioned.py +++ b/xoto3/dynamodb/update/versioned.py @@ -18,6 +18,7 @@ from xoto3.errors import client_error_name from xoto3.utils.dt import iso8601strict from xoto3.utils.tree_map import SimpleTransform +from xoto3.dynamodb.exceptions import DynamoDbItemException, get_item_exception_type from xoto3.dynamodb.types import TableResource from xoto3.dynamodb.get import strongly_consistent_get_item from xoto3.dynamodb.types import ItemKey, Item, AttrDict @@ -38,7 +39,7 @@ logger = getLogger(__name__) -class VersionedUpdateFailure(Exception): +class VersionedUpdateFailure(DynamoDbItemException): pass @@ -72,7 +73,7 @@ def __call__( def versioned_diffed_update_item( table: TableResource, item_transformer: ItemTransformer, - item_id: ItemKey, + item_key: ItemKey = None, *, get_item: ItemGetter = strongly_consistent_get_item, update_item: ItemUpdater = UpdateOrCreateItem, @@ -81,6 +82,8 @@ def versioned_diffed_update_item( last_written_key: str = "last_written_at", random_sleep_on_lost_race: bool = True, prewrite_transform: ty.Optional[SimpleTransform] = _DEFAULT_PREDIFF_TRANSFORM, + item_id: ItemKey = None, # deprecated name, present for backward-compatibility + nicename: str = "Item", ) -> Item: """Performs an item read-transform-write loop until there are no intervening writes. @@ -100,11 +103,15 @@ def versioned_diffed_update_item( will revert to fetching if the transaction fails because of an intervening write. """ + item_key = item_key or item_id + assert item_key, "Must pass item_key or (deprecated) item_id" + attempt = 0 max_attempts_before_failure = int(max(1, max_attempts_before_failure)) + update_arguments = None while attempt < max_attempts_before_failure: attempt += 1 - item = get_item(table, item_id) + item = get_item(table, item_key) cur_item_version = item.get(item_version_key, 0) logger.debug(f"Current item version is {cur_item_version}") @@ -112,13 +119,13 @@ def versioned_diffed_update_item( # do the incremental update updated_item = item_transformer(copy.deepcopy(item)) if not updated_item: - logger.debug("No transformed item was returned; returning original item") + logger.debug(f"No transformed {nicename} was returned; returning original {nicename}") return item assert updated_item is not None item_diff = build_update_diff(item, updated_item, prediff_transform=prewrite_transform) if not item_diff: logger.info( - "A transformed item was returned but no meaningful difference was found.", + f"Transformed {nicename} was returned but no meaningful difference was found.", extra=dict(json=dict(item=item, updated_item=updated_item)), ) return item @@ -136,15 +143,17 @@ def versioned_diffed_update_item( expr = versioned_item_expression( cur_item_version, item_version_key, - id_that_exists=next(iter(item_id.keys())) if item else "", + id_that_exists=next(iter(item_key.keys())) if item else "", ) logger.debug(expr) - update_item(table, item_id, **select_attributes_for_set_and_remove(item_diff), **expr) + update_arguments = select_attributes_for_set_and_remove(item_diff) + # store arguments for later logging + update_item(table, item_key, **update_arguments, **expr) return updated_item except ClientError as ce: if client_error_name(ce) == "ConditionalCheckFailedException": msg = ( - "Attempt %d to update item in table %s was beaten " + "Attempt %d to update %s in table %s was beaten " + "by a different update. Sleeping for %s seconds." ) sleep = 0.0 @@ -154,17 +163,21 @@ def versioned_diffed_update_item( logger.warning( msg, attempt, + nicename, table.name, f"{sleep:.3f}", extra=dict( - json=dict(item_id=item_id, item_diff=item_diff, ce=str(ce), sleep=sleep) + json=dict(item_key=item_key, item_diff=item_diff, ce=str(ce), sleep=sleep) ), ) else: raise - raise VersionedUpdateFailure( - f"Failed to update item without performing overwrite {item_id}" - f"Was beaten to the update {attempt} times." + raise get_item_exception_type(nicename, VersionedUpdateFailure)( + f"Failed to update {nicename} without performing overwrite {item_key}. " + f"Was beaten to the update {attempt} times.", + key=item_key, + table_name=table.name, + update_arguments=update_arguments, ) diff --git a/xoto3/dynamodb/utils/table.py b/xoto3/dynamodb/utils/table.py index f566bc3..426c481 100644 --- a/xoto3/dynamodb/utils/table.py +++ b/xoto3/dynamodb/utils/table.py @@ -1,10 +1,10 @@ from typing import Tuple -from xoto3.dynamodb.types import TableResource, Item, ItemKey +from xoto3.dynamodb.types import TableResource, InputItem, ItemKey def table_primary_keys(table: TableResource) -> Tuple[str, ...]: return tuple([key["AttributeName"] for key in table.key_schema]) -def extract_key_from_item(table: TableResource, item: Item) -> ItemKey: +def extract_key_from_item(table: TableResource, item: InputItem) -> ItemKey: return {attr_name: item[attr_name] for attr_name in table_primary_keys(table)}