fix: Do not pass allowCache to java-invoke-local server
#1349
Conversation
This file contains 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
|
Hi @pgrzesik, I'm wondering if anyone could help review this one line change? Thank.🙏 |
|
Hello @AlbertXingZhang - it looks like there are some issues with the change - could you please take a look into the output of CI runs? |
AlbertXingZhang
force-pushed
the
remove-allowCache-from-java-runner
branch
from
dc13d46
to
5050d35
Compare
|
Hi @pgrzesik, I have amended the change. Could you please help check if it looks better now? Thanks. |
|
Hello @AlbertXingZhang - there's some issue with CI builds at the moment - after it will be resolved I will review it again |
|
Hey @AlbertXingZhang - could you please push your changes again (e.g. by amending the commit and pushing it again)? It seems like it has be to triggered once again or otherwise the CI doesn't start |
AlbertXingZhang
force-pushed
the
remove-allowCache-from-java-runner
branch
from
73d0fb8
to
178071a
Compare
AlbertXingZhang
force-pushed
the
remove-allowCache-from-java-runner
branch
from
178071a
to
ef5e02d
Compare
|
I'm sorry I tried it twice since I'm not sure if it is triggered properly. |
pgrzesik
approved these changes
Mar 17, 2022
pgrzesik
changed the title
allowCache to java-invoke-local server
|
Thanks for merging. 🎉 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
We were running "java-invoke-local --server" and "serverless offline" as suggested for faster local invocations to the java lambda functions. But every single request we made ended with the following exception.
groovy.lang.MissingPropertyException: No such property: allowCache for class: serverless.jvm.plugin.InvokeRequestAfter taking a look at both source codes, I think it is because the java runner sends the unsupported property "allowCache" to the java-invoke-local server accidentally.
Motivation and Context
java-invoke-local doesn't support the property "allowCache".
https://github.com/bytekast/serverless-toolkit/blob/44d94008c5f0e7b6953d7d010f26e0490cd3814a/libs/java-invoke-local/src/main/groovy/serverless/jvm/plugin/InvokeRequest.groovy#L9
But JavaRunner.js sends it as a part of the body data to the java-invoke-local server.
serverless-offline/src/lambda/handler-runner/java-runner/JavaRunner.js
Line 79 in 353b6c6
It causes exceptions that prevent us from testing the java lambda functions with faster local invocations. So we made this simple change to just remove "allowCache" from the sending object.
How Has This Been Tested?
We made this change to serverless offline and try it again along with running "java-invoke-local --server" as before. Everything works as expected. We also ran "npm run test:unit" and we don't see it affects any other area.
Screenshots (if appropriate):