feat(v2): batch validation with partial failure
Issue #, if available: #1496
Description of changes:
Adding partial failure for validation with SQS and Kinesis. Modification of the ValidationAspect.java to validate each messages of SQS/Kinesis batches and put invalid messages in partial batch failures list of the response. After the handler, we merge with user batch failures.
Checklist
- [x] Meet tenets criteria
- [x] Update tests
- [x] Update docs
- [x] PR title follows conventional commit semantics
Breaking change checklist
RFC issue #:
- [ ] Migration process documented
- [ ] Implement warnings (if it can live side by side)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
:floppy_disk: Artifacts Size Report
| Module | Version | Size (KB) |
|---|---|---|
| powertools-common | 2.0.0-SNAPSHOT | 9.63 |
| powertools-serialization | 2.0.0-SNAPSHOT | 17.23 |
| powertools-logging | 2.0.0-SNAPSHOT | 33.10 |
| powertools-logging-log4j | 2.0.0-SNAPSHOT | 20.70 |
| powertools-logging-logback | 2.0.0-SNAPSHOT | 16.92 |
| powertools-tracing | 2.0.0-SNAPSHOT | 14.00 |
| powertools-metrics | 2.0.0-SNAPSHOT | 13.78 |
| powertools-parameters | 2.0.0-SNAPSHOT | 17.46 |
| powertools-validation | 2.0.0-SNAPSHOT | 21.32 |
| powertools-cloudformation | 2.0.0-SNAPSHOT | 16.54 |
| powertools-idempotency-core | 2.0.0-SNAPSHOT | 34.70 |
| powertools-idempotency-dynamodb | 2.0.0-SNAPSHOT | 12.38 |
| powertools-large-messages | 2.0.0-SNAPSHOT | 17.52 |
| powertools-batch | 2.0.0-SNAPSHOT | 21.51 |
| powertools-parameters-ssm | 2.0.0-SNAPSHOT | 10.75 |
| powertools-parameters-secrets | 2.0.0-SNAPSHOT | 9.97 |
| powertools-parameters-dynamodb | 2.0.0-SNAPSHOT | 12.01 |
| powertools-parameters-appconfig | 2.0.0-SNAPSHOT | 11.51 |
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code
Hey @jeromevdl this is a monster. Do you mind when you have a chance doing a quick writeup of the changes at a high level to start from?
Codecov Report
Attention: Patch coverage is 89.47368% with 12 lines in your changes missing coverage. Please review.
Project coverage is 77.84%. Comparing base (
82d4b30) to head (0923826). Report is 76 commits behind head on v2.
:exclamation: Current head 0923826 differs from pull request most recent head 503170a
Please upload reports for the commit 503170a to get more accurate results.
| Files | Patch % | Lines |
|---|---|---|
| ...wertools/validation/internal/ValidationAspect.java | 89.47% | 7 Missing and 5 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## v2 #1621 +/- ##
=============================================
- Coverage 89.79% 77.84% -11.96%
- Complexity 406 479 +73
=============================================
Files 44 49 +5
Lines 1274 1733 +459
Branches 165 261 +96
=============================================
+ Hits 1144 1349 +205
- Misses 88 295 +207
- Partials 42 89 +47
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@scottgerring this one is ready for review
I should create one (or 2 ? kinesis/sqs) end-to-end tests for this maybe, wdyt @scottgerring ?
I should create one (or 2 ? kinesis/sqs) end-to-end tests for this maybe, wdyt @scottgerring ?
To be honest I feel like it might be better to invest time stabilizing the existing tests. As it stands more tests are just going to lead to more "was that an actual build failure or not", which has already put us in a situation where we don't trust the suite.
Could we get coverage out of integration style tests instead?
Could we get coverage out of integration style tests instead?
Coverage is done with Unit Tests.
And E2E tests are quite stable, sometimes they time out but we rarely troubleshoot failed tests...
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Coverage is done with Unit Tests.
I mean, you are concerned that something in here isn't being covered. Can you get it covered without relying on e2e tests?
but we rarely troubleshoot failed tests...
this is the problem I am referring to :D if we don't bother looking when they break.
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
Dependency Review
The following issues were found:
- ✅ 0 vulnerable package(s)
- ❌ 1 package(s) with incompatible licenses
- ✅ 0 package(s) with invalid SPDX license definitions
- ✅ 0 package(s) with unknown licenses.
We might want to remove checkstyle from the project since we are using PMD now after the workflow refactoring (https://github.com/aws-powertools/powertools-lambda-java/issues/1231) by @sthulb
Eventually, we need to find a formatter that works across IDE specific implementations and can be applied from command line.
See this message: https://github.com/aws-powertools/powertools-lambda-java/issues/1345#issuecomment-1709680061
Quality Gate passed
Issues
0 New issues
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
This PR looks good. I tested it both locally and deployed an example in my AWS account.