-
Notifications
You must be signed in to change notification settings - Fork 98
feat: Support CRaC priming of powertools validation #2081
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
feat: Support CRaC priming of powertools validation #2081
Conversation
|
Hey @kjswaruph, thank you so much for contribution. The priming logic looks good to me. I tested it in my AWS account using SNAPSTART enabled for our validation example. I found some points that we need to improve / clarify: (1) Execution of priming logic The priming logic will only run if I explicitly reference the public InboundValidation() {
ValidationConfig.get(); // Ensure ValidationConfig is loaded for SnapStart
}The reason for this is that AspectJ compile-time weaving transforms the Given the fact that the user can use the Validation utility solely using static {
ObjectMapper objectMapper = ValidationConfig.get().getObjectMapper();
// update (de)serializationConfig or other properties
}This will invoke your priming logic since a reference to Can you add a documentation at the end of the documentation about this? You can create a new section "advanced" with a "Snapstart priming" subsection: In the Snapstart priming subsection, can you explain that we need a reference to (2) Unit testing Can you write a unit test that calls the
<Match>
<Bug pattern="RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT"/>
<Class name="software.amazon.lambda.powertools.validation.ValidationConfig"/>
<Method name="beforeCheckpoint"/>
</Match>
Update: I understand now. The return values of the priming calls are ignored which is ok because the rule is scoped to |
- Add org.crac dependency and generate-classesloaded-file profile in pom.xml - Add classesloaded.txt in src/main/resources for automatic class preloading - Update ValidationConfig to register for Crac resource - Update spotbugs-exclude.xml to suppress RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT spotbug error in beforeCheckpoint method
…store hooks do not throw exception
11e6194 to
c095cf5
Compare
|
Hey @phipag , thanks for the review and clarification on the priming logic. I’ve updated the Validation docs with the SnapStart priming subsection and added the unit tests. Please take another look when you have time, tell me if there 's any nedd to change wording in the docs or additional changes. |
|
~~Hey @kjswaruph, thank you so much for getting back to me so quickly. ~~
Done, thanks ✅ I will read the docs in the meantime and suggest changes that you can directly commit in the GitHub UI. |
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.
Thanks @kjswaruph. Just a small suggestion for the docs wording.
Co-authored-by: Philipp Page <[email protected]>
|
|
@phipag Done, applied the recommended suggestions. |
|
Awesome, @kjswaruph! Once the CI passes I will merge this PR. It looks good to me. Congratulations on your first contribution to Powertools for AWS Lambda (Java)!!! 🥳 |
|
Thanks @phipag, appreciate the review and the guidance. Learnt a lot of new things from this, would be happy to pick up more issues if you have suggestions. |
Happy to hear you learnt a lot, @kjswaruph! Absolutely, feel free to checkout our There are a number of more CRaC priming issues if you are interested in working on this. I also meant to ask: What did you think of the Priming documentation for developers? Was it clear to you? Do you have any suggestions for improvement? |
|
The priming documentation was clear to me and it helped me in getting started with this issue. I think overall the documentation is good and clear. |



Summary
Adds support for CRaC priming in the Powertools validation module.
Changes
Issue number: #2005
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.