Skip to content
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

RequestStreamHandler functionality added to Amazon Lambda #7866

Merged
merged 1 commit into from
Mar 15, 2020

Conversation

oztimpower
Copy link
Contributor

@oztimpower oztimpower commented Mar 14, 2020

Summary:

Amazon Lambda extension currently only supports the interface RequestHandler.

Enhance the extension, to enable the use of the interface RequestStreamHandler, so that both AWS Lambda interfaces are supported.

#6707 requests this functionality.

The RequestStreamHandler can co-exist in a project with RequestHandler, and the active function toggled via the property quarkus.lambda.handler, as normal. The requestHandler signature for the former has 3 params, the latter 2.

Validated as working for NativeImage, via SAM and AWS.

Specifics:

Deployment:

AmazonLambdaBuildItem adds a flag to indicate whether the scanned requestHandler is of type RequestStreamHandler, since it was the only way I could determine this (tried lots of options not to modify this BuildItem class).

public AmazonLambdaBuildItem(String handlerClass, String name, boolean streamHandler) {

Various changes to AmazonLambdaProcessesor to recognise and process the RequestStreamHandler::requestHandler implementation.

Runtime:

AmazonLambdaRecorder has similar changes to accept the co-existance of the RequestStreamHandler::requestHandler implementation.

Only one lambda handler can be active, per the existing validation in chooseHandlerClass, thus the code is checking which one is present to determine how to branch.

Added a compatible integration test for the AmazonClient.invoke in the test-framework for Amazon Lambda. I used this to test both.

Archetype:

Added a StreamHandler to the archetype, to demonstrate you can have both in your project, and that you must use the @nAmed annotation along with the application.properties to select the active lambda function, per existing functionality.

In addition, I added test functionality to include the integration test for Amazon Lambda into the archetype, as that is an easy way for people to test their functions, and wasn't previously in the archetype. For JVM, there is no need to use SAM when you have the integration test available (see issues with SAM for JVM #6701) which I continue to experience.

Tested by:

  1. mvn archetype:generate -DarchetypeGroupId=io.quarkus -DarchetypeArtifactId=quarkus-amazon-lambda-archetype -DarchetypeVersion=999-SNAPSHOT
  2. mvn package
  3. sh manage.sh create
  4. sh manage.sh invoke
  5. mvn package -Dnative -Dquarkus.native.container-build=true'
  6. sam local invoke --template sam.native.yaml --event payload.json
  7. sh manage.sh native create invoke
    (repeated toggling the application.property to swap handler type quarkus.lambda.handler)

/cc @patriot1burke
/cc @evanchooly
/cc @bnusunny

Copy link
Contributor

@patriot1burke patriot1burke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice PR!. Awesome you added the test harness. The switches you added are useful too.

@oztimpower oztimpower force-pushed the tp/lambda-stream-handler branch 2 times, most recently from 0ebb11b to b9902f3 Compare March 15, 2020 01:01
@oztimpower oztimpower force-pushed the tp/lambda-stream-handler branch from b9902f3 to c4e4b11 Compare March 15, 2020 10:59
@oztimpower
Copy link
Contributor Author

Good news is that with this new RequestStreamHandler functionality I've been able to get Alexa Lambda functions to work seamlessly with JVM, and also with native (lengthy reflection-config.json however to cater for inner builder classes).

@bnusunny
Copy link
Contributor

@oztimpower This is great! Top request from lambda users. Thank you Timothy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants