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.

Sign up for GitHub

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

Jump to bottom

Fix lambda no arg error #13610

Merged
merged 1 commit into from
Mar 30, 2021
Merged

Fix lambda no arg error #13610

merged 1 commit into from
Mar 30, 2021

Conversation

matejvasek
Copy link
Contributor

@matejvasek matejvasek commented Dec 1, 2020 •
edited
Loading

resolves #13595

@ghost ghost added the area/amazon-lambda label Dec 1, 2020
@matejvasek
Copy link
Contributor Author

@patriot1burke pleas look at this.

matejvasek
matejvasek commented Dec 1, 2020
View reviewed changes
integration-tests/funqy-amazon-lambda/src/test/java/io/quarkus/funqy/test/NoArgFunTest.java Outdated
}, 10_000);

try {
LambdaClient.invoke(String.class, null);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we could add overload with timeout?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@patriot1burke
Copy link
Contributor

This looks good. Just need CI to pass, although I don't think its your code.

@patriot1burke
Copy link
Contributor

Need to squash commits too.

gsmet
gsmet previously requested changes Dec 2, 2020
View reviewed changes
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Requested a small style change.

Also as mentioned by @patriot1burke we need a squash.

...da/common-runtime/src/main/java/io/quarkus/amazon/lambda/runtime/AbstractLambdaPollLoop.java Outdated Show resolved Hide resolved
@patriot1burke
Copy link
Contributor

This will need to be tested with lambda directly too.

@ghost ghost added the area/testing label Dec 2, 2020
@matejvasek
Copy link
Contributor Author

This will need to be tested with lambda directly too.

@patriot1burke Do you mean integration test for plain amazon-lambda? amazon-lambda has always input (well you can use java.lang.Void but that doesn't change anything).

@matejvasek matejvasek force-pushed the fix-lambda-no-arg-error branch from d7b9188 to cabf8d9 Compare December 2, 2020 19:34
@matejvasek
Copy link
Contributor Author

rebased @patriot1burke PTAL

@matejvasek matejvasek force-pushed the fix-lambda-no-arg-error branch from cabf8d9 to 09e645f Compare December 2, 2020 19:49
@patriot1burke
Copy link
Contributor

This will need to be tested with lambda directly too.

@patriot1burke Do you mean integration test for plain amazon-lambda? amazon-lambda has always input (well you can use java.lang.Void but that doesn't change anything).

I mean you need to test it with the real AWS Lambda. It will probably be fine, but i usually do that to make sure.

@gsmet gsmet dismissed their stale review December 21, 2020 11:26

Comments addressed.

@gsmet
Copy link
Member

gsmet commented Dec 21, 2020

@matejvasek I let you confirm you have done the test Bill asked for? Thanks!

@matejvasek
Copy link
Contributor Author

@gsmet sorry, I have not.

@matejvasek
Fix lambda no arg error
c90bd8c 
Signed-off-by: Matej Vasek <[email protected]>
@matejvasek matejvasek force-pushed the fix-lambda-no-arg-error branch from 09e645f to c90bd8c Compare March 9, 2021 17:25
@matejvasek
Copy link
Contributor Author

@gsmet I tested this with real AWS Lambda and it works.

@matejvasek
Copy link
Contributor Author

@gsmet PTAL

Base automatically changed from master to main March 12, 2021 15:55
@matejvasek
Copy link
Contributor Author

@patriot1burke PTAL

@matejvasek
Copy link
Contributor Author

@gsmet @patriot1burke ping

@patriot1burke patriot1burke merged commit 06e7f62 into quarkusio:main Mar 30, 2021
@quarkus-bot quarkus-bot bot added this to the 1.14 - main milestone Mar 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@patriot1burke patriot1burke patriot1burke approved these changes

@gsmet gsmet gsmet left review comments

Assignees
No one assigned
Labels
area/amazon-lambda area/testing
Projects
None yet
Milestone
2.0.0.Alpha1
Development

Successfully merging this pull request may close these issues.

AWS Funqy Lambda native image Timeout
3 participants
@matejvasek @patriot1burke @gsmet