openfaas-incubator / node8-express-template

Node.js 8 template for OpenFaaS with HTTP via Express.js
MIT License
15 stars 18 forks source link

Setting NODE_ENV to production by default #11

Open Templum opened 6 years ago

Templum commented 6 years ago

Currently, we already configure the log level of NPM and further only install production (runtime) dependency.
I suggest the default setting of NODE_ENV to production as this will increase the overall performance of the express server, without any additional effort.

alexellis commented 6 years ago

I'm generally for this. Would it benefit the normal node template too? What are the side effects for people relying on this for debugging and how does it impact them?

Templum commented 6 years ago

Regarding the benefit to the normal node template: The thing is, at least for express it will make an impact. Usually, the application needs to make use of the parameter, for example, to reduce unnecessary logging or unnecessary logic. Overall it might, but it is not guaranteed. However, it should not negatively impact it.

Regarding the side effects: I'm not aware of side effects, it might reduce the overall logging verbosity, however, if it is clearly stated you can override it on demand in the stack.yml. And I believe it is more likely that you will end up forgetting to set it to production. So if someone is actively developing, he can make use of the overriding.

telackey commented 6 years ago

I like the idea of it as a default, so long as it could be overridden. For example, we could set NODE_ENV=production in the Dockerfile, but override it with -e on the CLI, stack.yml, docker-compose.yml, etc.

padiazg commented 6 years ago

I like this proposal, also for the regular template. As I debug, it wont have any side effect, besides it can be overriden.