Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(v2): Split powertools-idempotency module to sub-modules and add redis implementation #1513
chore(v2): Split powertools-idempotency module to sub-modules and add redis implementation #1513
Changes from all commits
0ec7f4a
575e7a9
6382089
914359d
336cab3
dca1f0e
d71f9ac
a9ef2c4
c017948
bcbd956
9f44ebf
6da6aaf
e144be5
30722ba
7ff5708
d2e4efa
d00b5e2
b3e3d7d
0501b8f
0582f24
fce92df
0396be8
1a47aef
df7452f
4975dc9
65318c9
6cce077
0c1cc38
796029d
4d2c716
81417e1
a200e64
a82e4cd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably deserve more explanations: SG of elasticache / SG of the function? what would be the link between the two SGs (port 6379, 6380 ?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
@scottgerring wdyt ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found also this extract here from redis.conf