feat(idempotency): adding redis as idempotency backend #1914

Vandita2020 · 2023-02-08T21:27:44Z

Issue number: #1181

Summary

PR adds support for Redis as Idempotecny backend

Changes

Please provide a summary of what's being changed

The following features for Redis are currently supported by this PR

Add, update, get and delete idempotency record.
Standalone and Cluster architecture for Redis. The third type Sentinel will be added to this soon.
The database index, which is kind of namespace for information to be saved/stored between them.
User ACL
TTL support

User experience

Please share what the user experience looks like before and after this change

User can now use Redis as backend. You can connect it using Redis Standalone or Redis Cluster as shown in the example.

from aws_lambda_powertools.utilities.connections import RedisStandalone, RedisCluster
from aws_lambda_powertools.utilities.idempotency import (
    idempotent,
    RedisCachePersistenceLayer,
    IdempotencyConfig
)    

# For connection using Redis Standalone architecture
redis_connection = RedisStandalone(host="192.168.68.112", port=6379, password="pass", db_index=0) 

# For connection using Redis Cluster architecture
#redis_connection = RedisCluster(url="redis://:[email protected]:6379/0")

persistence_layer = RedisCachePersistenceLayer(connection=redis_connection)
config =  IdempotencyConfig(
    expires_after_seconds=1*60,  # 1 minutes
)

@idempotent(config=config, persistence_store=persistence_layer)
def lambda_handler(event, context):
    return {"message":"Hello"}

Checklist

If your change doesn't seem to apply, please leave them unchecked.

Meet tenets criteria
I have performed a self-review of this change
Changes have been tested
Changes are documented
PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

Migration process documented
Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

boring-cyborg · 2023-02-08T21:27:48Z

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our AWS Lambda Powertools Discord: Invite link

leandrodamascena · 2023-02-17T00:08:13Z

Still working on this before converting it to "Ready for review".

codecov-commenter · 2023-02-17T00:10:56Z

Codecov Report

Base: 97.44% // Head: 96.24% // Decreases project coverage by -1.21% ⚠️

Coverage data is based on head (81c141e) compared to base (dc8971e).
Patch coverage: 22.93% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1914      +/-   ##
===========================================
- Coverage    97.44%   96.24%   -1.21%     
===========================================
  Files          144      149       +5     
  Lines         6627     6730     +103     
  Branches       475      482       +7     
===========================================
+ Hits          6458     6477      +19     
- Misses         132      215      +83     
- Partials        37       38       +1

Impacted Files	Coverage Δ
...ambda_powertools/utilities/connections/__init__.py	`0.00% <0.00%> (ø)`
...mbda_powertools/utilities/connections/base_sync.py	`0.00% <0.00%> (ø)`
...bda_powertools/utilities/connections/exceptions.py	`0.00% <0.00%> (ø)`
...s_lambda_powertools/utilities/connections/redis.py	`0.00% <0.00%> (ø)`
...ools/utilities/idempotency/persistence/dynamodb.py	`98.76% <ø> (ø)`
...ertools/utilities/idempotency/persistence/redis.py	`28.84% <28.84%> (ø)`
...wertools/utilities/idempotency/persistence/base.py	`98.20% <66.66%> (-1.19%)`	⬇️
aws_lambda_powertools/logging/formatter.py	`100.00% <100.00%> (ø)`
aws_lambda_powertools/metrics/base.py	`100.00% <100.00%> (ø)`
...ambda_powertools/utilities/idempotency/__init__.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

… the idempotency key

heitorlessa · 2023-03-10T07:43:05Z

Checking in whether we have updates

Signed-off-by: Vandita Patidar <[email protected]>

rubenfonseca

Read the code for the first time and left some first remarks. Didn't look too deep into the redis logic itself, just focused on the bigger blocks for now. Great job!

rubenfonseca · 2023-04-03T12:05:59Z

aws_lambda_powertools/utilities/connections/__init__.py

The utilities folder is used to contain Powertools public utilities, and not internal utilities. Would it make sense to move this under the idempotency utility folder instead?

rubenfonseca · 2023-04-03T12:09:21Z

aws_lambda_powertools/utilities/connections/base_sync.py

Any chance we use the BasePersistenceLayer for this instead of creating another base class?

rubenfonseca · 2023-04-03T12:11:00Z

aws_lambda_powertools/utilities/connections/redis.py

+            See: https://redis.readthedocs.io/en/latest/connections.html#redis.Redis.from_url
+        """
+
+        super().__init__(redis.cluster.RedisCluster, host, port, username, password, db_index, url, **extra_options)


If I understand correctly, the only difference between the two classes is the first parameter (redis.cluster.RedisCluster vs redis.Redis).

It that's the only difference, wouldn't it make sense to have a single class with a flag for standalone vs cluster instead of 2 different classes?

rubenfonseca · 2023-04-03T12:11:32Z

aws_lambda_powertools/utilities/idempotency/persistence/base.py

@@ -37,7 +37,7 @@ class DataRecord:

    def __init__(
        self,
-        idempotency_key: str,
+        idempotency_key: str = "",


Are we sure we want this? It's probably a breaking change to make this optional.

rubenfonseca · 2023-04-03T12:12:50Z

aws_lambda_powertools/utilities/idempotency/persistence/base.py

+        else:
+            now = datetime.datetime.now()
+            period = datetime.timedelta(seconds=self.expires_after_seconds)
+            return int((now + period).timestamp())


Use tell don't ask here. Instead of storing the type of backend, just delegate the get_expiry_timestamp to the different backends, and let them deal with the details.

rubenfonseca · 2023-04-03T12:12:57Z

aws_lambda_powertools/utilities/idempotency/persistence/dynamodb.py

@@ -26,7 +26,7 @@ class DynamoDBPersistenceLayer(BasePersistenceLayer):
    def __init__(
        self,
        table_name: str,
-        key_attr: str = "id",
+        key_attr: Optional[str] = "id",


Probably unecessary

rubenfonseca · 2023-04-03T12:14:45Z

docs/utilities/idempotency.md

+        if static_pk_value is None:
+            static_pk_value = f"idempotency#{os.getenv(constants.LAMBDA_FUNCTION_NAME_ENV, '')}"
+
+        self.static_pk_value = static_pk_value
+        self.in_progress_expiry_attr = in_progress_expiry_attr
+        self.expiry_attr = expiry_attr
+        self.status_attr = status_attr
+        self.data_attr = data_attr
+        self.validation_key_attr = validation_key_attr
+        super(RedisCachePersistenceLayer, self).__init__()
+
+    def _get_key(self, idempotency_key: str) -> dict:
+        # Need to review this after adding GETKEY logic
+        if self.sort_key_attr:
+            return {self.key_attr: self.static_pk_value, self.sort_key_attr: idempotency_key}
+        return {self.key_attr: idempotency_key}
+
+    def _item_to_data_record(self, item: Dict[str, Any]) -> DataRecord:
+        # Need to review this after adding GETKEY logic
+        return DataRecord(
+            status=item[self.status_attr],
+            expiry_timestamp=item[self.expiry_attr],
+            in_progress_expiry_timestamp=item.get(self.in_progress_expiry_attr),
+            response_data=item.get(self.data_attr),
+            payload_hash=item.get(self.validation_key_attr),
+        )
+
+    def _get_record(self, idempotency_key) -> DataRecord:
+        # See: https://redis.io/commands/hgetall/
+        response = self._connection.hgetall(idempotency_key)
+
+        try:
+            item = response
+        except KeyError:
+            raise IdempotencyItemNotFoundError
+        return self._item_to_data_record(item)
+
+    def _put_record(self, data_record: DataRecord) -> None:
+        # Redis works with hset to support hashing keys with multiple attributes
+        # See: https://redis.io/commands/hset/
+        item = {
+            "name": data_record.idempotency_key,
+            "mapping": {
+                self.in_progress_expiry_attr: data_record.in_progress_expiry_timestamp,
+                self.status_attr: data_record.status,
+                self.expiry_attr: data_record.expiry_timestamp,
+            },
+        }
+
+        if data_record.in_progress_expiry_timestamp is not None:
+            item["mapping"][self.in_progress_expiry_attr] = data_record.in_progress_expiry_timestamp
+
+        if self.payload_validation_enabled:
+            item["mapping"][self.validation_key_attr] = data_record.payload_hash
+
+        try:
+            # |     LOCKED     |         RETRY if status = "INPROGRESS"                |     RETRY
+            # |----------------|-------------------------------------------------------|-------------> .... (time)
+            # |             Lambda                                              Idempotency Record
+            # |             Timeout                                                 Timeout
+            # |       (in_progress_expiry)                                          (expiry)
+
+            # Conditions to successfully save a record:
+
+            # The idempotency key does not exist:
+            #    - first time that this invocation key is used
+            #    - previous invocation with the same key was deleted due to TTL
+            idempotency_key_not_exist = self._connection.exists(data_record.idempotency_key)
+
+            # key exists
+            if idempotency_key_not_exist == 1:
+                raise
+
+            # missing logic to compare expiration
+
+            logger.debug(f"Putting record on Redis for idempotency key: {data_record.idempotency_key}")
+            self._connection.hset(**item)
+            # hset type must set expiration after adding the record
+            # Need to review this to get ttl in seconds
+            self._connection.expire(name=data_record.idempotency_key, time=self.expires_after_seconds)
+        except Exception:
+            logger.debug(f"Failed to put record for already existing idempotency key: {data_record.idempotency_key}")
+            raise IdempotencyItemAlreadyExistsError
+
+    def _update_record(self, data_record: DataRecord) -> None:
+        item = {
+            "name": data_record.idempotency_key,
+            "mapping": {
+                self.data_attr: data_record.response_data,
+                self.status_attr: data_record.status,
+            },
+        }
+        logger.debug(f"Updating record for idempotency key: {data_record.idempotency_key}")
+        self._connection.hset(**item)
+
+    def _delete_record(self, data_record: DataRecord) -> None:
+        logger.debug(f"Deleting record for idempotency key: {data_record.idempotency_key}")
+        # See: https://redis.io/commands/del/
+        self._connection.delete(data_record.idempotency_key)
+```


rubenfonseca · 2023-04-03T12:15:11Z

pyproject.toml

@@ -98,12 +99,15 @@ tracer = ["aws-xray-sdk"]
 all = ["pydantic", "aws-xray-sdk", "fastjsonschema"]
 # allow customers to run code locally without emulators (SAM CLI, etc.)
 aws-sdk = ["boto3"]
+redis = ["redis"]


needs to added to all too on line 98

leandrodamascena · 2023-09-01T22:14:10Z

Closing this PR in favor of #2567!

Credit for Vandita's work will be mentioned when we release this feature.

leandrodamascena added 7 commits January 29, 2023 17:29

(redis): initial commit

3d5062c

feat(redis/idempotency): creating redis connections

aed821a

feat(redis/idempotency): creating redis connections

ffcf16d

feat(redis/idempotency): fixing import

b5c5791

feat(redis/idempotency): adding base class

6902e73

feat(redis/idempotency): adding logic to get record

504d258

feat(redis/idempotency): adding expiry timeout

0d1e3e9

boring-cyborg bot added the dependencies Pull requests that update a dependency file label Feb 8, 2023

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Feb 8, 2023

heitorlessa linked an issue Feb 9, 2023 that may be closed by this pull request

Add support for MemoryDB/ElasticCache/Redis as Idempotency backend #1181

Closed

heitorlessa requested a review from leandrodamascena February 9, 2023 09:06

leandrodamascena self-assigned this Feb 9, 2023

leandrodamascena added 4 commits February 9, 2023 21:29

feat(redis) - merging from develop

fce91e5

feat(redis) - refactoring connection and fixing mypy errors

3126885

feat(redis) - merging from develop

f9c1875

feat(redis) - removing wrong print

81c141e

leandrodamascena added 3 commits February 17, 2023 23:32

feat(redis) - removing fields and adding additional logic to validate…

5d6b0d0

… the idempotency key

Merge branch 'develop' into redis/newbackend

656096f

feat(redis) - adding redis as dev dependency

038926c

Update idempotency.md

ca9b16d

Signed-off-by: Vandita Patidar <[email protected]>

boring-cyborg bot added the documentation Improvements or additions to documentation label Mar 23, 2023

pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 23, 2023

Update idempotency.md

b8bcfe4

Signed-off-by: Vandita Patidar <[email protected]>

rubenfonseca suggested changes Apr 3, 2023

View reviewed changes

heitorlessa changed the title ~~feat(idempotency): adding redis as idempotency backend~~ TEST SEMANTIC PR feat(idempotency): adding redis as idempotency backend Apr 19, 2023

heitorlessa changed the title ~~TEST SEMANTIC PR feat(idempotency): adding redis as idempotency backend~~ feat(idempotency): adding redis as idempotency backend Apr 19, 2023

roger-zhangg mentioned this pull request Jun 23, 2023

feat(idempotency): adding redis as idempotency backend #2567

Merged

7 tasks

leandrodamascena removed a link to an issue Aug 15, 2023

Add support for MemoryDB/ElasticCache/Redis as Idempotency backend #1181

Closed

leandrodamascena closed this Sep 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(idempotency): adding redis as idempotency backend #1914

feat(idempotency): adding redis as idempotency backend #1914

Vandita2020 commented Feb 8, 2023 •

edited by heitorlessa

Loading

boring-cyborg bot commented Feb 8, 2023

leandrodamascena commented Feb 17, 2023 •

edited

Loading

codecov-commenter commented Feb 17, 2023 •

edited

Loading

heitorlessa commented Mar 10, 2023

rubenfonseca left a comment

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

rubenfonseca Apr 3, 2023

leandrodamascena commented Sep 1, 2023

feat(idempotency): adding redis as idempotency backend #1914

feat(idempotency): adding redis as idempotency backend #1914

Conversation

Vandita2020 commented Feb 8, 2023 • edited by heitorlessa Loading

Summary

Changes

User experience

Checklist

Acknowledgment

boring-cyborg bot commented Feb 8, 2023

leandrodamascena commented Feb 17, 2023 • edited Loading

codecov-commenter commented Feb 17, 2023 • edited Loading

Codecov Report

heitorlessa commented Mar 10, 2023

rubenfonseca left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandrodamascena commented Sep 1, 2023

Vandita2020 commented Feb 8, 2023 •

edited by heitorlessa

Loading

leandrodamascena commented Feb 17, 2023 •

edited

Loading

codecov-commenter commented Feb 17, 2023 •

edited

Loading