-
Notifications
You must be signed in to change notification settings - Fork 88
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: Easy Event Deserialization #757
feat: Easy Event Deserialization #757
Conversation
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.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.
Some suggestions.
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
} else if (obj instanceof KinesisAnalyticsStreamsInputPreprocessingEvent) { | ||
KinesisAnalyticsStreamsInputPreprocessingEvent event = (KinesisAnalyticsStreamsInputPreprocessingEvent) obj; | ||
return new EventPart(event.getRecords().stream().map(r -> decode(r.getData())).collect(Collectors.toList())); | ||
} else { |
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 mechanisms can be have to update these when java lib introduces new event type?
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.
I am thinking may be we can have a github action which somehow polls java lib weekly or daily or whatever and create an issue if for powertools if something new is added for events ?
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.
Not all events make sense in here. I didn't add all, only those where there is a message or something meaningful to extract. Maybe I forgot some interesting ones.
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.
We can get an email on each release of the library. Dependabot will also raise a 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.
Ok, may be i am overthinking this, we wait for doing anything more until we see it being a problem
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
...ization/src/test/java/software/amazon/lambda/powertools/utilities/EventDeserializerTest.java
Outdated
Show resolved
Hide resolved
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
...on/src/main/java/software/amazon/lambda/powertools/validation/internal/ValidationAspect.java
Show resolved
Hide resolved
Would anything change post v4 of the events library? It might be worth a quick chat with @andclt. |
@@ -45,6 +45,14 @@ | |||
<groupId>io.burt</groupId> | |||
<artifactId>jmespath-jackson</artifactId> | |||
</dependency> | |||
<dependency> | |||
<groupId>com.amazonaws</groupId> | |||
<artifactId>aws-lambda-java-events</artifactId> |
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.
In v4 of the events lib, there will be a dependency on Jackson, I don't think that changes anything, just worth noting.
...ools-serialization/src/main/java/software/amazon/lambda/powertools/utilities/JsonConfig.java
Show resolved
Hide resolved
…powertools/utilities/EventDeserializerTest.java Co-authored-by: Pankaj Agrawal <[email protected]>
…s#756) Bumps [maven-jar-plugin](https://github.com/apache/maven-jar-plugin) from 3.2.0 to 3.2.2. - [Release notes](https://github.com/apache/maven-jar-plugin/releases) - [Commits](apache/maven-jar-plugin@maven-jar-plugin-3.2.0...maven-jar-plugin-3.2.2) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-jar-plugin dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…rtools#755) Bumps `aws.sdk.version` from 2.17.130 to 2.17.131. Updates `software.amazon.awssdk:bom` from 2.17.130 to 2.17.131 - [Release notes](https://github.com/aws/aws-sdk-java-v2/releases) - [Changelog](https://github.com/aws/aws-sdk-java-v2/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-java-v2@2.17.130...2.17.131) Updates `http-client-spi` from 2.17.130 to 2.17.131 - [Release notes](https://github.com/aws/aws-sdk-java-v2/releases) - [Changelog](https://github.com/aws/aws-sdk-java-v2/blob/master/CHANGELOG.md) - [Commits](aws/aws-sdk-java-v2@2.17.130...2.17.131) Updates `url-connection-client` from 2.17.130 to 2.17.131 --- updated-dependencies: - dependency-name: software.amazon.awssdk:bom dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: software.amazon.awssdk:http-client-spi dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: software.amazon.awssdk:url-connection-client dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
remove trailing slash for multiple params
- javadoc - indentation - better edge cases handling
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.
Minor suggestion, from code perspective, it looks good to me, so if you want to start documenting it.
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
...rialization/src/main/java/software/amazon/lambda/powertools/utilities/EventDeserializer.java
Outdated
Show resolved
Hide resolved
@pankajagrawal16 @msailes ready for further review / merge |
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.
👍🏻
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.
Looks good. Just one minor suggestion on docs.
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.
Great 🎉 ... cc @kozub
Issue #, if available:
Description of changes:
New feature in the serialization module: ability to easily extract the content from an Event (many built-in events).
Ex with API Gateway:
Ex with SQS:
Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.