Skip to content

Allow update time out and memory size in each function - #33

Merged
SeanRoy merged 2 commits into
SeanRoy:masterfrom
glmanhtu:master
Sep 18, 2016
Merged

Allow update time out and memory size in each function#33
SeanRoy merged 2 commits into
SeanRoy:masterfrom
glmanhtu:master

Conversation

@glmanhtu

@glmanhtu glmanhtu commented Sep 14, 2016

Copy link
Copy Markdown
Contributor

Hi SeanRoy,

I found this project when I trying to write a simple python script to upload multi function in same project to AWS lambda. Now, it's not necessary anymore. Thanks you.

However, when I upload my function to AWS, I already set memory size of my function is 256MB
<lambdaFunctionsJSON> [ { "functionName": "GetToken", "description": "Get token from refresh token", "handler": "com.woodenextreme.webservice.GetTokenHandler", "timeout": 60, "memorySize": 256 }, { "functionName": "GetIAP", "description": "Get IAP of current user", "handler": "com.woodenextreme.webservice.GetIAPHandler", "timeout": 60, "memorySize": 256 } ] </lambdaFunctionsJSON>
However, when I check in AWS console, I found this value have been changed to 1024MB.
I check in your code and I think, maybe you forgot to check if user already set time out & memory or not.

Hope to heard from you soon,

Mạnh Tú

.withRole(lambdaRoleArn)
.withTimeout(timeout)
.withMemorySize(memorySize)
.withTimeout(ofNullable(lambdaFunction.getTimeout()).orElse(timeout))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch! .withTimeout(lambdaFunction.getTimeout()) is enough as it is defaulted already in AbstractLambdaMojo:221

.withTimeout(timeout)
.withMemorySize(memorySize)
.withTimeout(ofNullable(lambdaFunction.getTimeout()).orElse(timeout))
.withMemorySize(ofNullable(lambdaFunction.getMemorySize()).orElse(memorySize))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice catch! .withMemorySize(lambdaFunction.getMemorySize()) is enough as it is defaulted already in AbstractLambdaMojo:222

@kgrodzicki

Copy link
Copy Markdown
Contributor

Good catch Mạnh Tú ! I added some comments to commit. Please make changes and it will be good to merge.

We don't need ofNullable because of it is defaulted already in AbstractLambdaMojo (line 221 and 222)
@glmanhtu

Copy link
Copy Markdown
Contributor Author

SeanRoy,
Fixed, waiting for merge :)

@SeanRoy

SeanRoy commented Sep 15, 2016

Copy link
Copy Markdown
Owner

Thanks guys, I'll take a look at it soon

On Wednesday, September 14, 2016, Vũ Mạnh Tú notifications@github.com
wrote:

SeanRoy,
Fixed, waiting for merge :)


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#33 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AIycDrRqnnVuSH20kLcmuOrzn8VISkweks5qqJeZgaJpZM4J8pSc
.

sean@nrby.com brittany.campbell@ping4.com | nrby.com
Twitter https://twitter.com/nrbynow | Facebook
https://www.facebook.com/nrbynow | Instagram
https://instagram.com/nrbynow/ | LinkedIn
http://linkedin.com/company/nrby
http://linkedin.com/company/nrby

This message is intended only for the use of the individual or entity named
above and may contain information which is privileged, confidential, and
not intended for disclosure. If you are not the intended recipient, you are
hereby notified that any disclosure, copying, distribution, dissemination
or use of this message is strictly prohibited. If you have received this
message in error, please accept our apology. We would very much appreciate
your notifying the sender immediately by return email. Thank you.

@SeanRoy
SeanRoy merged commit 601b790 into SeanRoy:master Sep 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants