chore(v2): Split powertools-idempotency module to sub-modules and add redis implementation #1513
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -35,7 +35,7 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| ... | ||||||
| <dependency> | ||||||
| <groupId>software.amazon.lambda</groupId> | ||||||
| <artifactId>powertools-idempotency-dynamodb</artifactId> | ||||||
| <version>{{ powertools.version }}</version> | ||||||
| </dependency> | ||||||
| ... | ||||||
|
|
@@ -56,7 +56,7 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| <aspectLibraries> | ||||||
| <aspectLibrary> | ||||||
| <groupId>software.amazon.lambda</groupId> | ||||||
| <artifactId>powertools-idempotency-dynamodb</artifactId> | ||||||
| </aspectLibrary> | ||||||
| </aspectLibraries> | ||||||
| </configuration> | ||||||
|
|
@@ -80,7 +80,7 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| ... | ||||||
| <dependency> | ||||||
| <groupId>software.amazon.lambda</groupId> | ||||||
| <artifactId>powertools-idempotency-dynamodb</artifactId> | ||||||
| <version>{{ powertools.version }}</version> | ||||||
| </dependency> | ||||||
| ... | ||||||
|
|
@@ -101,7 +101,7 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| <aspectLibraries> | ||||||
| <aspectLibrary> | ||||||
| <groupId>software.amazon.lambda</groupId> | ||||||
| <artifactId>powertools-idempotency-dynamodb</artifactId> | ||||||
| </aspectLibrary> | ||||||
| </aspectLibraries> | ||||||
| </configuration> | ||||||
|
|
@@ -131,7 +131,7 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| } | ||||||
|
|
||||||
| dependencies { | ||||||
| aspect 'software.amazon.lambda:powertools-idempotency-dynamodb:{{ powertools.version }}' | ||||||
| } | ||||||
|
|
||||||
| sourceCompatibility = 11 // or higher | ||||||
|
|
@@ -151,18 +151,20 @@ Depending on your version of Java (either Java 1.8 or 11+), the configuration sl | |||||
| } | ||||||
|
|
||||||
| dependencies { | ||||||
| aspect 'software.amazon.lambda:powertools-idempotency-dynamodb:{{ powertools.version }}' | ||||||
| } | ||||||
|
|
||||||
| sourceCompatibility = 1.8 | ||||||
| targetCompatibility = 1.8 | ||||||
| ``` | ||||||
|
|
||||||
| ### Required resources | ||||||
| Before getting started, you need to create a persistent storage layer where the idempotency utility can store its state - your Lambda functions will need read and write access to it. | ||||||
| As of now, [Amazon DynamoDB](https://aws.amazon.com/dynamodb/) and [Redis](https://redis.io/) are the supported persistnce layers. | ||||||
|
|
||||||
| #### Using Amazon DynamoDB as persistent storage layer | ||||||
|
||||||
|
|
||||||
| If you are using Amazon DynamoDB you'll need to create a table. | ||||||
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
|
|
||||||
| **Default table configuration** | ||||||
|
|
||||||
|
|
@@ -215,6 +217,70 @@ Resources: | |||||
| see 1WCU and 1RCU. Review the [DynamoDB pricing documentation](https://aws.amazon.com/dynamodb/pricing/) to | ||||||
| estimate the cost. | ||||||
|
|
||||||
| #### Using Redis as persistent storage layer | ||||||
|
||||||
|
|
||||||
| ##### Redis resources | ||||||
|
|
||||||
| You need an existing Redis service before setting up Redis as persistent storage layer provider. You can also use Redis compatible services like [Amazon ElastiCache for Redis](https://aws.amazon.com/elasticache/redis/) as persistent storage layer provider. | ||||||
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| !!! tip "Tip:No existing Redis service?" | ||||||
eldimi marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
| If you don't have an existing Redis service, we recommend using DynamoDB as persistent storage layer provider. | ||||||
|
||||||
| If you don't have an existing Redis service, we recommend using DynamoDB as persistent storage layer provider. | |
| If you don't have an existing Redis service, we recommend using DynamoDB as the persistent storage layer provider. DynamoDB does not require network a VPC deployment and is easier to configure and operate. |
.... or some words that go to the "why is this a sensible default"
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
I think this should be configurable; I don't think we should force people to put passwords in env vars. Long term it would be cool if we had IAM integration here, short term enough to be able to provide the password somehow, having retrieved it e.g. using the SecretsManagerProvider ?
(apologies if you can already do this - i haven't got to the code yet!)
There was a problem hiding this comment.
For IAM Integration, I am not sure, this would be an extra possibility, since Amazon ElasticCache is the only possible Redis deployment that people might use.
For SecretsManagerProvider, sure, I agree. I wanted to do that too. Since this is already a lot of implementation details in this PR, I felt I should put it for review, to discuss the approach, list findings and decide which of them are important to be included with this PR and which can be a in a next step.
There was a problem hiding this comment.
I agree, and don't think either of these things needs to happen here. The whole redis rabbit whole we started down for a separate impl to prove the abstraction out; we have that without this other bits!
There was a problem hiding this comment.
yes, having password as an environment variable is not a good practice. It should be something the user passes in the "connection string", so that he can retrieve it from secrets manager
eldimi marked this conversation as resolved.
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
problem in this sentence...
Outdated
There was a problem hiding this comment.
See how Python managed the connection: https://github.com/aws-powertools/powertools-lambda-python/pull/2567/files#diff-9aa1f6d28c9806e244b0bf7a7263e02efb02cf4986da20d1b8fc6bb931097884R48
There was a problem hiding this comment.
Hello Java folks! It's awesome that you're adding support for Redis in Idempotency!!
My 2 cents here. Two widespread use cases when using Redis are:
1 – Customers who want to use SSL connections to encrypt data in transit. I saw Jedis supports SSL parameter when building the client.
2 - Customers who want to store data in a specific dbIndex - dbIndex is a logical database in Redis, the concept is something similar to schema. The default db_index is 0, but customers can store data in another db_index.
Do you think it is possible to add these 2 parameters in this implementation? For cases where the client wants to create a very specific connection with TLS, customizations, and others, we do the same as you: the customer can bring their own client.
Thanks 🚀
There was a problem hiding this comment.
Thanks @leandrodamascena for the feedback. I think it makes sense.
There was a problem hiding this comment.
With 0c1cc38 the option to provide a custom JedisClientConfig is added.
It can be used as described in the documentation
eldimi marked this conversation as resolved.
Show resolved
Hide resolved
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
There was a problem hiding this comment.
not sure it is super useful, or explain how? but better would be to explain the security group config (if any), or any other networking config.
There was a problem hiding this comment.
probably deserve more explanations: SG of elasticache / SG of the function? what would be the link between the two SGs (port 6379, 6380 ?)
eldimi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Outdated
There was a problem hiding this comment.
link at first usage, move it above.
There was a problem hiding this comment.
What do you mean by "default configuration", it is what we use as default ? If that's the case, does it make sense? Not sure anyone is using the user "default" with no password...
I think we should ask for all the required fields to create a JedisClient ourself:
- host (mandatory)
- port (default: 6379)
- user (mandatory)
- password (mandatory)
- ssl (default: false)
- database ?
- something else ?
@scottgerring wdyt ?
There was a problem hiding this comment.
Default here means, as provided by the jedis client default config.
In Redis before version 6, you could only require password for auth .
Also, by default all access is unathenticated and a user with name "default" is created. So, by default (that is the redis default), you can execute commands either without any user/pass or with user:"default" and an empty pass. The user "default" is created even after redis version 6, for backwards compatibility purposes.
See ACL redis doc.
For these reasons, I am not sure we should make them mandatory.
Actually I have to update the powertools documentation, since user/pass, by default, in JedisClient are null.
There was a problem hiding this comment.
Found also this extract here from redis.conf
The special username "default" is used for new connections. If this user
has the "nopass" rule, then new connections will be immediately authenticated
as the "default" user without the need of any password provided via the
AUTH command. Otherwise if the "default" user is not flagged with "nopass"
the connections will start in not authenticated state, and will require
AUTH (or the HELLO command AUTH option) in order to be authenticated and
start to work.
Outdated
There was a problem hiding this comment.
we cannot do this, sorry
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -381,7 +381,6 @@ | |
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <goals> | ||
| <goal>compile</goal> | ||
| <goal>test-compile</goal> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <parent> | ||
| <groupId>software.amazon.lambda</groupId> | ||
| <artifactId>e2e-test-handlers-parent</artifactId> | ||
| <version>1.0.0</version> | ||
| </parent> | ||
|
|
||
| <artifactId>e2e-test-handler-idempotency-dynamodb</artifactId> | ||
| <packaging>jar</packaging> | ||
| <name>A Lambda function using Powertools for AWS Lambda (Java) idempotency</name> | ||
|
|
||
| <dependencies> | ||
| <dependency> | ||
| <groupId>software.amazon.lambda</groupId> | ||
| <artifactId>powertools-idempotency-dynamodb</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>software.amazon.lambda</groupId> | ||
| <artifactId>powertools-logging</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.apache.logging.log4j</groupId> | ||
| <artifactId>log4j-slf4j2-impl</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.amazonaws</groupId> | ||
| <artifactId>aws-lambda-java-events</artifactId> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>org.aspectj</groupId> | ||
| <artifactId>aspectjrt</artifactId> | ||
| </dependency> | ||
| </dependencies> | ||
|
|
||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>dev.aspectj</groupId> | ||
| <artifactId>aspectj-maven-plugin</artifactId> | ||
| <configuration> | ||
| <source>${maven.compiler.source}</source> | ||
| <target>${maven.compiler.target}</target> | ||
| <complianceLevel>${maven.compiler.target}</complianceLevel> | ||
| <aspectLibraries> | ||
| <aspectLibrary> | ||
| <groupId>software.amazon.lambda</groupId> | ||
| <artifactId>powertools-idempotency-core</artifactId> | ||
| </aspectLibrary> | ||
| <aspectLibrary> | ||
| <groupId>software.amazon.lambda</groupId> | ||
| <artifactId>powertools-logging</artifactId> | ||
| </aspectLibrary> | ||
| </aspectLibraries> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <goals> | ||
| <goal>compile</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-shade-plugin</artifactId> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </project> |
Uh oh!
There was an error while loading. Please reload this page.