-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Basic support for AWS Lambdas #1042
Conversation
Note: this PR is for running AWS lambdas within Quarkus, not for running Quarkus as a lambda. |
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 OK but really needs to be squashed down to probably one commit.
Yeah. Definitely a squash merge for sure. |
i went ahead and squashed the commits now rather than waiting until the 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.
I added a few comments inline, mostly about the pom files and the structure.
...da/quarkus-lambda-deployment/src/main/java/io/quarkus/lambda/deployment/LambdaProcessor.java
Outdated
Show resolved
Hide resolved
.setLoadOnStartup(1) | ||
.addMapping("/" + mapping)); | ||
|
||
try (final ClassCreator creator = new ClassCreator(classOutput, servletName, null, LambdaServlet.class.getName())) { |
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.
This should really not be necessary, there is some stuff missing from the ServletBuildItem, I have added it at #1045. With this you could either use a template+ custom InstanceFactory to just directly pass the parameter, or specify the class name in the init params and load it in the init method.
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. I'll update this code on #1045 is merged.
...ons/lambda/quarkus-lambda-runtime/src/main/java/io/quarkus/lambda/runtime/LambdaServlet.java
Outdated
Show resolved
Hide resolved
...da/quarkus-lambda-deployment/src/main/java/io/quarkus/lambda/deployment/LambdaProcessor.java
Outdated
Show resolved
Hide resolved
...da/quarkus-lambda-deployment/src/main/java/io/quarkus/lambda/deployment/LambdaProcessor.java
Outdated
Show resolved
Hide resolved
Do we actually want to merge this before it has native support? It would be the only extension in the code base that does not run on native. |
I could go either way but I know there were folks that wanted to see a bit of it in action. If you say you want me to get native working first, though, that's what I'll do. |
native-image support fixed. |
Running in native mode doesn't quite work yet but this is functional enough for folks to start playing with. There are some limitations in terms of input and output (incoming POJOs work but necessarily returning them yet) but these bits can be iterated on depending on how far down this road we want to go.