-
Notifications
You must be signed in to change notification settings - Fork 90
docs: HelloWorldStreamFunction in examples fails with sam #1532
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
Merged
Merged
Changes from all commits
Commits
Show all changes
67 commits
Select commit
Hold shift + click to select a range
e66095e
Setting up Kotlin environment. Converting test to Kotlin.
ea0b496
Deploying via SAM successfully.
615a095
Added Kotlin example.
5199105
Removing unused Gradle build file.
37020f3
Merge branch 'main' into main
scottgerring 6386358
Adding SAM template so can be used as an existing project and Java ta…
19679da
Adding SAM template so can be used as an existing project
17c3a5e
Updating guidance to use SAM for build and deploy
1312c02
Restructuring separate Java and Kotlin examples.
18c7f48
Updating core examples readme to represent new structure for Java and…
d968f3c
Refactoring application code for efficiency, updating build to cover …
5ce71f7
Updating to fix trailing \n
35fbbac
Updating guidance to be more specific for examples
6ffa295
Merge branch 'main' into main
scottgerring d277c5c
Adopting new mechanism for specifying jvm target.
6c4646e
accommodating new project structure
6eecb90
Fixing link typo after refactoring
66a3dc2
Merge branch 'main' into main
scottgerring 1431899
Setting up Kotlin environment. Converting test to Kotlin.
3396766
Deploying via SAM successfully.
baa0e3f
Added Kotlin example.
16380a8
Removing unused Gradle build file.
3f3f4e8
Adding SAM template so can be used as an existing project and Java ta…
9d70b57
Adding SAM template so can be used as an existing project
bcff275
Updating guidance to use SAM for build and deploy
84935c9
Restructuring separate Java and Kotlin examples.
bf6da0d
Updating core examples readme to represent new structure for Java and…
f67f59a
Refactoring application code for efficiency, updating build to cover …
f7288d5
Updating to fix trailing \n
f03b01b
Updating guidance to be more specific for examples
e523554
Adopting new mechanism for specifying jvm target.
a71e263
accommodating new project structure
51fc239
Fixing link typo after refactoring
6cef711
Flattening structure back to original to make merging easier for v2
2444ba4
Merge remote-tracking branch 'origin/main'
c24ee6d
Adding build for Kotlin Gradle
de19b56
Adding build for Kotlin Gradle - Restructuring Java examples to v1 ap…
6e23b26
Correcting paths
480dd8b
Adding SNAPSHOT support and local capability for Maven. Testing using…
7a8dc16
Reviewed and updated against PR comments.
de08cbc
Merge branch 'main' into main
scottgerring bf843a7
Merging with Terraform additions from main branch
9a154dd
Un-commenting examples
e3c1cda
Un-commenting examples
e8ddb45
Adding validation step for IaC SAM
ec17480
Adding Terraform for Java projects IaC validator and linter
7d11fac
Adding additional projects for SAM validation and matrix approach
e932d10
Refactoring stream function to process input logging example with a L…
ab31b09
Demonstrating Java streaming response
d4bed89
Refactoring stream function to process input logging example to return
4322ce9
Update CONTRIBUTING.md
jeromevdl 494d73f
fix: get trace id from system property when env var is not set (#1503)
mriccia e6e2eb0
fix #1500 (#1506)
jeromevdl 7c51a38
feat: Add support for POWERTOOLS_LOGGER_LOG_EVENT (#1510)
AlexeySoshin f814d4d
chore: Addition of Warn Message If Invalid Annotation Key While Traci…
jdoherty 9a3a788
feat: ALC (#1514)
jeromevdl 7fc6ff2
chore:Prep release 1.18.0 (#1515)
jeromevdl 6635b50
chore: update version to next snapshot: 1-19.0-SNAPSHOT (#1516)
jeromevdl 9d8b614
Add some more margin to the test pause (#1518)
scottgerring 223d196
test: e2e tests with java 21 (#1517)
jeromevdl eeaec59
update doc for ALC (#1520)
jeromevdl 150b28f
chore: Testing java21 aspectj pre-release (#1519)
scottgerring 2411a07
chore: Remove build cruft
scottgerring 772b30e
Adding context for using RequestStreamHandler
c69e3dc
replacing current main version of pr_build
2259c66
removing pr_lint
080edc0
Update examples/powertools-examples-core/sam/src/main/java/helloworld…
jasoniharris File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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 think it would be helpful to add some info here about why you would use
RequestStreamHandler
- e.g. to make it clear that it has nothing to do with HTTP streaming.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.
Note added to explain why RequestStreamHandler is preferred here
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 think it's not quite right - I think the use case is more typically "we need to control [de]serialization ourselves".
I find "rather than a simple HTTP request-response model" a little confusing, because you still have exactly the same surrounding HTTP interaction as you would with a 'normal' RequestHandler, you're just choosing to read-and-write the
Body
using java streams rather than letting Lambda handle the serialization for you.@jeromevdl I don't know if you have some super clear wording handy? The core Lambda documentation isn't that prescriptive.
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 agree wording is not clear or even wrong.
RequestStreamHandler
is not linked to Lambda streaming which was announced recently. It is used when you want to use your own serialization: https://docs.aws.amazon.com/lambda/latest/dg/java-handler.html#java-handler-interfaces. I agree the name of the interface is not super well chosen but it has nothing to do with streams...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 tried to rephrase the wording to better reflect what this handler actually does