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

Installing the app in production mode #20

Merged
merged 1 commit into from
Jan 7, 2016

Conversation

4lejandrito
Copy link
Contributor

This avoids installing the development dependencies (devDependencies) from the package.json.

This avoids installing the development dependencies (devDependencies) from the package.json.
@icflorescu
Copy link
Owner

$NODE_ENV is already set to 'production' (see here), so this shouldn't be necessary.

I just tested it on a live cartridge with Node.js 5.1.1 & npm 3.5.2 and both npm i and npm prune behave as expected (dependencies are installed while devDependencies are not). Have you come across a situation where they don't?

@4lejandrito
Copy link
Contributor Author

Now that you said that...

I changed the NODE_ENV variable to a custom value ('openshift') so that my app would grab the proper config file. Since that value is different from production it installs the devDependencies.

I understand if I change the value back to production it will work but... Would it be better to explicitly add the --production flag to allow use cases like mine?

Thanks for the quick answer :)

icflorescu added a commit that referenced this pull request Jan 7, 2016
Specifically set production mode for npm i & prune
@icflorescu icflorescu merged commit 0d5d892 into icflorescu:master Jan 7, 2016
@icflorescu
Copy link
Owner

I see. I imagine some people would also set-up 'test' deployments on OpenShift Online, so it makes total sense.

Thanks!

@4lejandrito
Copy link
Contributor Author

Brilliant!

Thanks a lot.

@ryanblakeley
Copy link

Just for clarity, how would you config or override to install devDependencies in a production push?

@icflorescu
Copy link
Owner

@rblakeley - since the above PR was merged, when you push to OpenShift, it'll install just the "production" dependencies, which I assume is the scenario 99% of the users are after anyway. There's no way to force the automatic installation of devDependencies. If you really need to install devDependencies on the server, you could do so by sshing to your app cartridge and running npm i manually as you see fit.

Of course, I'm always open to discuss. I did merge the above PR, but we can certainly discuss to revert to the original behaviour if it makes better sense.

So, please, let me know your thoughts.

@4lejandrito
Copy link
Contributor Author

I think apps should be installed in production mode by default. In my case it literally saves hundreds of megabytes of space (which costs money). I agree it would be nice to have a way to disable the --production flag for those who need it but it should not be the default.

What do you guys think?

@ryanblakeley
Copy link

I agree that production as a default is fine. To manually install devDependencies I discovered the command npm install --only=dev.
Just as a note, I could never get my simple express starter app to to work on openshift. It was a hair pulling mess so I'm trying AWS now.

@icflorescu
Copy link
Owner

@4lejandrito:

On most PaaS providers, a Node.js app is installed in production mode by default, but that happens based on NODE_ENV=production. Have a look at an in-depth explanation on how things are happening on Heroku here.

So that's how this cartridge now differs from the norm by forcing --production on npm install, regardless of NODE_ENV value.

I think we could (and maybe should) create a mechanism for overriding the default behaviour. Heroku does this by using another env variable NPM_CONFIG_PRODUCTION.

@rblakeley:

I never worked with AWS directly, but if you're looking for the easiest way to deploy/start a simple Express.js application, I think nothing beats Heroku.

What makes OpenShift interesting (and sometimes a bit hairy, I agree ;-), is exactly the ability of fiddling around with custom-made application cartridges such as this one. And price, let's be honest about it :-)

On the other hand, perhaps I should add more How-to examples in the README, maybe even add a simple "Launch on OpenShift" button based on Ryan's excellent Launch Service...

@icflorescu
Copy link
Owner

Have a look at the new README, there's some sort of a "one-click deployment" now ;-)

@ryanblakeley
Copy link

@4lejandrito thanks for the suggestion; you're spot on about Heroku. That is the PaaS I've always used in the past. The NPM_CONFIG_PRODUCTION option has been convenient and might be a good idea for this project.

Unfortunately, I can't use Heroku for my current project because they don't allow port forwarding. My project consists of a frontend app and graphql endpoint; so it uses two ports (and the frontend app proxies the graphql endpoint). It would only work on Heroku if I split them into separate apps. Since they share code and are conceptually paired, I don't want to do that.

Where OpenShift unraveled for me (and where an example might be nice) was doing a deploy with a build step. The build step used devDependencies, so I tried manually installing them via SSH. It just never worked due to a variety of errors: permission corruption, incomplete installations, etc.

I was very quickly out of my depth when trying to launch this boilerplate project just as a starting point.

@4lejandrito
Copy link
Contributor Author

@rblakeley my case was somewhat similar.

I have an isomorphic js app (gulp, babel, browserify, react...) and it was literally impossible to get it built on the openshift servers. That is why I decided to build everything locally and push the final bundles (js and css) to the repository. In this way I could actually deploy to openshift and the deployments are now much faster and have a much smaller memory footprint.

I definitely agree with you @icflorescu but I am not sure how we would cover the original use case that caused me to open the PR in the first place. What if someone wants to run their app in, let's say,
NODE_ENV='stage' mode? Will that mean devDependencies will be installed since it is not production? I understand npm install would still be run under production mode and npm start would do so under stage, am I right?

If that is the case I am perfectly happy with the way you describe.

@icflorescu
Copy link
Owner

Building locally and pushing the resulting bundles is considered best-practice, to say the least. You should always avoid building on the server, especially on shared-resources PaaS. There's no reason for browserify, gulp, grunt npms to exist on the server.

Also, note that Openshift has an outdated gcc at the moment (see discussion here and there's nothing we can do about it except maybe vote. Which means lots of packages with binary deps will fail to build.

About adjusting the deps behaviour on this cartridge, yeah, that's what I was thinking. We could check for two env vars, NPM_ENV and NODE_NPM_ENV before running npm i.

In your particular case, you'd have to specifically set NODE_NPM_ENV to production; would that be acceptable?

For now, it's just a thought. Don't have much time to script this during the next few days anyway (unless RedHat is willing to hire me ;-)...

@ryanblakeley
Copy link

Building locally and pushing the bundle would be great. But some of the files which get bundled contain references to environment variables.

I'm using authorization and Relay. Relay injects the authorization network layer on the graphql endpoint, and needs to know the hostname:port for that endpoint. Since those are environment variables, those files have to be built remotely. I wish I could get around that.

@icflorescu
Copy link
Owner

Couldn't you output them separately, i.e. as data- attrs on an element in your html container, and read them from there in your js app?

@4lejandrito
Copy link
Contributor Author

I had a very similar problem with my app (mine is not relay) but at the end what I did was calling an endpoint from the JS after loading which would retrieve the information I needed from the environment in the server. Maybe not ideal, but worked and it was definitely worth getting rid of the hassle of building everything server side.

@ryanblakeley
Copy link

Thanks, those are interesting ideas. But the environment variables in question are sensitive to security. So having them be available on an endpoint or published in the html would not be safe.

@4lejandrito
Copy link
Contributor Author

Alright makes sense, I thought you were embedding environment variables into the generated client side JS...

@ryanblakeley
Copy link

You know what... I was wrong about them being sensitive. There is an ID but the secret isn't in that part. I might be able to do one of your suggestions.

@icflorescu
Copy link
Owner

I thought so. You're either publishing it or not – it doesn't matter if you're putting the info in HTML or include it in a js application bundle, as long as it's not a private key or something. There's no reason not to build a really "static" bundle that reads the context info from the HTML container...

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