-
Notifications
You must be signed in to change notification settings - Fork 90
powertools-parameters module #90
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
Conversation
Thanks @jeromevdl for the contribution. I will have a look at the PR. |
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.
First of all, thanks for this contribution. It will be a great utility to have.
I have few suggestions/questions which I have tried to explain. Let me know your thoughts and we can discuss further.
Regarding docs, I do agree, once we have the code change finalized, then start with documenting it.(Avoids rework)
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Outdated
Show resolved
Hide resolved
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Outdated
Show resolved
Hide resolved
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Show resolved
Hide resolved
...rs/src/main/java/software/amazon/lambda/powertools/parameters/transform/JsonTransformer.java
Outdated
Show resolved
Hide resolved
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java
Show resolved
Hide resolved
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/ParamManager.java
Show resolved
Hide resolved
...s-parameters/src/test/java/software/amazon/lambda/powertools/parameters/SSMProviderTest.java
Show resolved
Hide resolved
...s-parameters/src/test/java/software/amazon/lambda/powertools/parameters/SSMProviderTest.java
Show resolved
Hide resolved
...rameters/src/test/java/software/amazon/lambda/powertools/parameters/SecretsProviderTest.java
Show resolved
Hide resolved
Could we get an update to an existing example, or a new example to show the developer experience? Thanks. |
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Outdated
Show resolved
Hide resolved
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.
This has started to look nice. Some minor suggestion. Also if we could have a test where we can see how clients will use these API directly, will be great. Like end to end but just mocking the client and CacheManager with Clock
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Outdated
Show resolved
Hide resolved
...ools-parameters/src/main/java/software/amazon/lambda/powertools/parameters/BaseProvider.java
Outdated
Show resolved
Hide resolved
...arameters/src/main/java/software/amazon/lambda/powertools/parameters/cache/CacheManager.java
Outdated
Show resolved
Hide resolved
...s-parameters/src/main/java/software/amazon/lambda/powertools/parameters/cache/DataStore.java
Outdated
Show resolved
Hide resolved
...t/java/software/amazon/lambda/powertools/parameters/transform/TransformationManagerTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/software/amazon/lambda/powertools/parameters/transform/JsonTransformerTest.java
Outdated
Show resolved
Hide resolved
...eters/src/test/java/software/amazon/lambda/powertools/parameters/cache/CacheManagerTest.java
Outdated
Show resolved
Hide resolved
...eters/src/test/java/software/amazon/lambda/powertools/parameters/cache/CacheManagerTest.java
Outdated
Show resolved
Hide resolved
...s-parameters/src/test/java/software/amazon/lambda/powertools/parameters/SSMProviderTest.java
Outdated
Show resolved
Hide resolved
...s-parameters/src/test/java/software/amazon/lambda/powertools/parameters/SSMProviderTest.java
Outdated
Show resolved
Hide resolved
.../src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerIntegrationTest.java
Outdated
Show resolved
Hide resolved
...parameters/src/main/java/software/amazon/lambda/powertools/parameters/cache/NowProvider.java
Outdated
Show resolved
Hide resolved
.../main/java/software/amazon/lambda/powertools/parameters/transform/TransformationManager.java
Outdated
Show resolved
Hide resolved
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.
This looks really nice to me with all the test etc.
We should make sure that we have the docs updated as well to document the utility, either as part of this PR or a separate is ok too.
.../src/test/java/software/amazon/lambda/powertools/parameters/ParamManagerIntegrationTest.java
Outdated
Show resolved
Hide resolved
...arameters/src/main/java/software/amazon/lambda/powertools/parameters/cache/CacheManager.java
Outdated
Show resolved
Hide resolved
3c1c109
to
8b0ac4f
Compare
Will create the doc and a sample once this PR is validated. |
Yeah, lets move onto writing the docs and sample! |
8b0ac4f
to
74ec66e
Compare
3104a94
to
74f95aa
Compare
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.
LGTM 🚀
Description of changes:
Add parameters module, to easily retrieve parameters from SSM Parameter Store and Secrets Manager, put them in cache for further reuse.
Checklist
Breaking change checklist
New module, no impact on other ones, but probably need to increase version accordingly.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.