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

make runtime configurable #185

Closed
wants to merge 1 commit into from

Conversation

JamesKyburz
Copy link

Make runtime configurable.

  • use runtime from provider as default (is present)
  • allow specific runtime in custom configuration
  • support for custom runtimes

I have deployed a test lambda with the updated code, and warmup worked using a custom runtime.

@juanjoDiaz
Copy link
Owner

Hi @JamesKyburz ,

Thanks for your contribution. A couple of questions:

What's the use case for using a custom runtime or the runtime of the provider?
The warmup lambda is created for a specific node.js runtime. Of course, using a higher node.js runtime should work, but using an older node version or using python, java, or something else would not. And there are many users using such runtimes.
So I can see a lots of room for things not working and people raising tickets and very little benefits, unless I'm missing some obvious benefit...

I can also see that you have sneaked in support for layers. Layers support was discussed on #142 and it was decided that there is no reason to support them since, as mentioned, the warmup lambda is a custom self-contained lambda that shouldn't be tampered with.

@JamesKyburz
Copy link
Author

Thanks for the feedback. You are right that I was not thinking about older versions of node :)

The use case I personally have is the ability to configure the warmup functions to use the same custom javascript runtime as all my other javascript functions.

So this pull request makes it possible to use node12 or a custom runtime (that's why I added layers too)

If the code is designed to run on node10 I could maybe remove reading the runtime from the provider block in serverless.yml?

@juanjoDiaz
Copy link
Owner

I guess that the question is whether there is any benefit on having the warmup lambda using the same runtime as your other lambdas or even a custom runtime. And AFAIK the answer is no. Or is there any?

I'd only see any value on this if, for example, we would allow external warmup function as was requested in #93. At the same time, if you do that you might as well just create the function using normal serverless and skip this plugin completely.

@JamesKyburz
Copy link
Author

Having the ability to use a custom runtime has two advantages imho cost and monitoring.

Switching to a custom runtime can be cheaper than waiting for Amazon to release their official runtime (node 12 lts is soon and we will have to wait a while for Amazon's version)

If a company uses custom runtimes for example nsolid from nodesource then all lambdas including warmup ones can be monitored the same way.

@JamesKyburz
Copy link
Author

Rebased with the latest, Added a few style changes since npm run lint failed with the latest changes in master.

@JamesKyburz
Copy link
Author

I removed the style changes! I was running a different version of eslint locally after the rebase :)

@juanjoDiaz juanjoDiaz force-pushed the master branch 4 times, most recently from 3bc5d91 to 380c4ea Compare February 12, 2021 21:32
@juanjoDiaz
Copy link
Owner

Closing since this seems unnecessary and no one else is requesting to configure the runtime.
We can bring this back if it ever becomes necessary.

@juanjoDiaz juanjoDiaz closed this Jan 11, 2022
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.

2 participants